Rich Text: Remove updateContent function#7620
Conversation
|
|
||
| this.savedContent = this.props.value; | ||
| this.setContent( this.savedContent ); | ||
| this.editor.selection.moveToBookmark( bookmark ); |
There was a problem hiding this comment.
-
Dropping the focus management breaks the "merge". The focus is not restored at the right position when merging two paragraphs.
-
I believe the undo behavior is also necessary in this case or in cases the
valueof a RichText is updated programmatically (the change is not triggered by the RichText element itself)
There was a problem hiding this comment.
But all tests have passed successfully!
😉
There was a problem hiding this comment.
hahaha :) we definitely need a test for that. I guess we only test the content and not the cursor position.
|
I added back the selection setting, this time with the condition that the editor has focus when content is being set. This is in combination with a new end-to-end test which would fail without the condition (as it does in master as well, even including a ZWSP in the saved content). See 01e0501. |
ellatrix
left a comment
There was a problem hiding this comment.
Looks good to me! The only thing I could find is weird undo records on split, but that's in master too.
This pull request seeks to simplify
RichTextto have a single function for setting content, vs. the currentupdateContentandsetContent, by removingupdateContent.It's current implementation includes three notable differences from just calling
setContentdirectly, addressed below:updateContentis called on a block split.this.savedContentto the current value. This should take place insetContentif we're assuming that this value is always to be kept in sync with the current content of the editor instance.Testing instructions:
Verify that there are no regressions in the behavior of the editor, particularly when splitting content, which until the merge of #7482 causes
updateContentto be called.