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

Send selection change event when typing #1867

Merged

Conversation

hypest
Copy link
Contributor

@hypest hypest commented Feb 7, 2020

Fixes #1865

Experimental fix by dispatching a ReactAztecSelectionChangeEvent when text is changed in (Android) Aztec's manager.

This fix needs some extensive testing since the extra selection changed event could have side effects in other features.

Thing is, selection changed events should be sent out anyway by this code but it looks like the listener is suspended because of this code on the Aztec side.

@khaykov , I know it's been a while now since that code was written but, do you remember who/when should re-enable the selection change listener after the ZeroIndexContentWatcher has disabled it?

Edit: latest fix on this PR is on the Aztec side. PR here: wordpress-mobile/AztecEditor-Android#892

To test:

  1. Add a pararaph block and add some text to it
  2. Click in the middle of the text and hit enter to move the right half of the text to a new block
  3. Hit backspace and watch the new block merge back correctly with the previous block
  4. Hit enter from the middle of the text again to create a new block
  5. This time delete one or more characters from the beginning of the new text block
  6. Continue to hit the backspace key => blocks should get merged

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@hypest hypest added this to the 1.22 milestone Feb 7, 2020
Copy link
Contributor

@cameronvoell cameronvoell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good!

@mchowning mchowning merged commit 1d0df4d into release/1.22.0 Feb 7, 2020
@mchowning mchowning deleted the issue/1865-merge-on-backspace-in-edited-paragraph branch February 7, 2020 21:10
@hypest hypest mentioned this pull request Feb 10, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants