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

Fix splitting in the middle of a text block #453

Merged
merged 1 commit into from
Apr 19, 2017
Merged

Conversation

nylen
Copy link
Member

@nylen nylen commented Apr 19, 2017

As a follow-up to #409, this fixes an issue with splitting a text block in the middle of its content:

  • Create a text block with two paragraphs of content
  • Move the cursor to the end of the first paragraph
  • Press Enter. The text block will be split into two.

Instead, I'd expect that two Enter presses would be required to split the text block in the middle of its content, like when we edit at the end of a block.

The reason for this issue was that TinyMCE inserts a <br data-mce-bogus=1> element inside of our new second paragraph, and this causes the splitIndex to be -1 because we should really be looking for the parent p element of the br. The solution I found is to walk the DOM tree until we find the currently selected top-level element of the block (in this case the p element).

@nylen nylen requested a review from youknowriad April 19, 2017 16:59
@nylen
Copy link
Member Author

nylen commented Apr 19, 2017

Also, how can we add tests for this fairly tricky bit of logic?

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

This works great! Thanks for the fix (I had some trouble understanding the issue at first)

Yeah this code is tricky and hard to test. Maybe we could just extract to a splitNode( contentNode, selectedNode ) which returns { before, after } or false.

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.

2 participants