Rich Text: Remove restoreContentAndSplit#7618
Conversation
|
Funny enough, this will incidentally fix the issue described at #5095 (comment), which is still technically an issue for Some related discussion today in Slack with @tofumatt |
|
The end-to-end test failure is a legitimate bug where the caret is in the first paragraph wrongly when splitting in two from the middle: But in fact it's the same bug observed at #5095 (comment) and proposed to be fixed by #7620 . You can observe that it becomes fixed again if you cherry-pick 29bcba5 into your local branch. Thus, considering #7620 as a blocker for this. |
| const beforeFragment = beforeRange.extractContents(); | ||
| const afterFragment = afterRange.extractContents(); | ||
| const beforeFragment = beforeRange.cloneContents(); | ||
| const afterFragment = afterRange.cloneContents(); |
There was a problem hiding this comment.
Could the afterFragment still be extracted, so we can avoid having to set it afterwards? I think this this happens in the block edit component. Or maybe I'm misunderstanding this.
There was a problem hiding this comment.
so we can avoid having to set it afterwards?
What do you mean by set?
There was a problem hiding this comment.
By means of setAttributes in the block edit function.
There was a problem hiding this comment.
Still not totally sure I understand, but we should want the explicit setAttributes and avoid leveraging any of these more implicit DOM mutations.
1085589 to
03edbac
Compare
03edbac to
f3d3852
Compare
f3d3852 to
7a61eb0
Compare
Avoid destroying original content incurred by extractContents
Call `onSplit` directly
7a61eb0 to
03ded61
Compare
|
Rebased and force pushed. Running locally, all tests / functionality appears to work as expected now. |
tofumatt
left a comment
There was a problem hiding this comment.
I tested this locally, split a bunch of blocks, paragraphs, lists, headings, including paragraphs that had bold characters and such–split them inside and outside the bold characters.
All seemed to work well and I encountered no bugs.
Writing a few e2e tests for block splitting would be good, I dunno if we have any. Either way I suppose that's not just related to this PR 😄
🚢
|
|
There's quite a few split across |
Related: https://github.com/WordPress/gutenberg/pull/5100/files#r198953121
This pull request seeks to simplify the
RichTextcomponent, removing itsrestoreContentAndSplitfunction which had been used to preserve content through a block split. In my testing, I had found that this was not a behavior caused by TinyMCE, as indicated by the function's comment, but rather in our use ofRange#extractContentswhich moves (i.e. destroys) contents of a range. Updating this code to useRange#cloneContentsinstead avoids the need to re-set theRichTextcontent which, in turn, should be a boon to performance and avoid the need for this pass-through function.Testing instructions:
Verify that there are no regressions in block splitting. Particularly, ensure that a paragraphs contents are not destroyed after pressing Enter within.