Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backspace does not outdent nested list item on android #1027

Closed
mkevins opened this issue May 24, 2019 · 4 comments · May be fixed by wordpress-mobile/AztecEditor-Android#821
Closed

Backspace does not outdent nested list item on android #1027

mkevins opened this issue May 24, 2019 · 4 comments · May be fixed by wordpress-mobile/AztecEditor-Android#821

Comments

@mkevins
Copy link
Contributor

mkevins commented May 24, 2019

Description:
Pressing backspace when the caret is at the beginning of a nested list item does not "outdent" the list item. Rather, it merges the list item to the preceding list item.

Steps to reproduce:

  1. Create a nested list
  2. Place caret at the start of an indented list item
  3. Press [Backspace]

Expected behavior:
The indented list item should outdent one level with each press of [Backspace], and only merge with the preceding item after it is fully outdented.

Screencast:

Actual behavior:
The indented list item immediately merges with the preceding list item.

Screencast:

@mkevins
Copy link
Contributor Author

mkevins commented May 24, 2019

One approach to implement outdent behavior for Android is to follow the implementation used for iOS which emits the [Backspace] event to JS, delegating the nested list item behavior to gutenberg code.

I began implementing this on Android, however, the changes are non-trivial to implement. On Android, the logic determining whether or not to emit a [Backspace] event to JS is conditioned in several places, across several projects / repositories. Here is the line that emits the event to JS.

Here are the places which currently condition on this:

When the caret is at the beginning of the block with no selection:
https://github.com/wordpress-mobile/gutenberg-mobile/blob/develop/react-native-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecText.java#L393

When the onBackspace property is truthy (across the bridge) and the isTextChangedListenerDisabled flag is false:
https://github.com/wordpress-mobile/gutenberg-mobile/blob/develop/react-native-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecText.java#L105
onBackspace property:
https://github.com/WordPress/gutenberg/blob/1a20e15a615ecb344cfb0dd8b614982910240909/packages/block-editor/src/components/rich-text/index.native.js#L859

and:

When the content is either empty, or equal to the empty html tag for the current block type:

Also, the interface method will not be called from Aztec unless several conditions are met within the InputFilter:
https://github.com/wordpress-mobile/AztecEditor-Android/blob/develop/aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt#L501

It would be nice to have many (or all) of these conditions in one place, however, refactoring to accomplish that may require a good deal of time / care to ensure that the changes do not cause regressions in other areas.

I have pushed some experimental branches to begin the process of exploring what changes would be necessary:

https://github.com/wordpress-mobile/AztecEditor-Android/tree/experimental/fix-backspace-outdent-on-nested-lists
https://github.com/wordpress-mobile/gutenberg-mobile/tree/experimental/fix-backspace-outdent-on-nested-lists

The current state of these branches does fix the outdent issue, however, may result in other regressions. With some effort, there may be a way to refactor the conditions to selectively alter the behavior only for nested lists, and I suspect this should also involve extensive testing to prevent regressions in Aztec. 👋 @daniloercoli @hypest wdyt?

@daniloercoli daniloercoli self-assigned this May 24, 2019
@daniloercoli
Copy link
Contributor

daniloercoli commented Jun 3, 2019

One approach to implement outdent behavior for Android is to follow the implementation used for iOS which emits the [Backspace] event to JS, delegating the nested list item behavior to gutenberg code.

I've the fear that the current Android behaviour will bite us in the near future, when other blocks will be ported to mobile gb. I think we can migrate much of the GB.mobile logic to the wrapper, and keep in Aztec the bare minimum of it. (Consider that Backspace.key detection is also used in Aztec to remove styles).

I can take a deeper look tomorrow, and assign this issue to me.

@SergioEstevao
Copy link
Contributor

@mchowning when you work on Aztec for mentions to support the @ detection you probably can also check if it's possible to send the backspace press on other situations?

@SiobhyB
Copy link
Contributor

SiobhyB commented Jan 29, 2024

Confirmed that this was fixed somewhere down the line! 🎉 I tested using a Pixel 6 and the latest version (24.1) and a backspace worked to outdent a nested list item.

@SiobhyB SiobhyB closed this as completed Jan 29, 2024
Mobile Gutenberg automation moved this from Backlog (unscheduled/unassigned) to Done (keep clean, manually) Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Mobile Gutenberg
  
Done (keep clean, manually)
4 participants