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

RichText: fix caret position when deleting a selected word #11965

Merged
merged 4 commits into from
Nov 16, 2018

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Nov 16, 2018

Description

Fixes #11964 and adds e2e tests.
Also fixes an issue where CTRL+A wouldn't select all blocks if you first select all text in a rich text field by triple clicking and then pressing CTRL+A.

In #11627 I retained deletion handling for uncollapsed selections (meaning text is selected) because I couldn't figure out the problem fast enough with letting it be handled natively again. It turns out TinyMCE is handling the case where an instance is entirely selected, and the React onKeyDown event is only fired after the TinyMCE event, causing TinyMCE to delete the contents first, and then us thinking the field is empty and merging it with the neighbouring block. The solution is to prevent the TinyMCE behaviour in that case, and handle it ourselves.

@notnownikki I also updated the tests you have added, to check if the space is actually there before retyping.

How has this been tested?

See #11964 and above.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@ellatrix ellatrix force-pushed the fix/delete-selected-word branch from ee1a592 to e6a4094 Compare November 16, 2018 09:39
@ellatrix ellatrix added [Status] In Progress Tracking issues with work in progress [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable labels Nov 16, 2018
@ellatrix ellatrix removed the [Status] In Progress Tracking issues with work in progress label Nov 16, 2018
@ellatrix ellatrix requested review from notnownikki and a team November 16, 2018 10:46
@ellatrix ellatrix added this to the 4.5 milestone Nov 16, 2018
@ellatrix ellatrix added the [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... label Nov 16, 2018
Copy link
Member

@notnownikki notnownikki left a comment

Choose a reason for hiding this comment

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

Thanks for the test fixes and improvements :)

I've gone through all the steps manually and everything works as expected, 👍 from me!

@notnownikki
Copy link
Member

Just an aside: is there already an issue open for the caret placement after undoing deletion?

@ellatrix
Copy link
Member Author

Just an aside: is there already an issue open for the caret placement after undoing deletion?

I'm not sure. Caret position in general is never set after undoing changes. It's an old issue that probably needs a bit of work.

@ellatrix
Copy link
Member Author

Thanks for testing @notnownikki!

@ellatrix ellatrix merged commit f635145 into master Nov 16, 2018
@ellatrix ellatrix deleted the fix/delete-selected-word branch November 16, 2018 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RichText: incorrect caret position after deleting selected word
2 participants