core: add OutlineTextNode splitText with selection tests #121
Conversation
facebook-github-bot
added
the
CLA Signed
| focusOffset: 2, | ||
| }, | ||
| ], | ||
| [ |
There was a problem hiding this comment.
This test is failing and I'd like to clarify the correct behavior before proceeding further - When a text is split into 3 chunks, should the start and end of the selection fall on the first and the third chunk respectively?
For example, intuitively, when a selection is split like this, the anchor and focus should be on the first and third node respectively?
"Hello World" -> "Hell" "o " "World"
^^^^^^ ^^ ^^ ^^
The current behavior is that the focus is on the second node with an offset of 4. Not sure if intended or a bug.
There was a problem hiding this comment.
This is definitely a bug! Nice one catching it. I suspected that this logic might have been a bit fragile and this text confirms it :)
This makes sense as to why I was running into issues with some of the www plugins and had to manually select the node after, which seemed it a bit strange.
There was a problem hiding this comment.
Great, I'll follow up and fix it in this PR!
| @@ -119,14 +119,13 @@ function splitText( | |||
| const sibling = getWritableNode(createTextNode(part)); | |||
| sibling.__flags = flags; | |||
| const siblingKey = sibling.__key; | |||
| const nextTextSize = textLength + partSize; | |||
| const nextTextSize = textSize + partSize; | |||
There was a problem hiding this comment.
I think the bug is here.
|
|
||
| if (selection !== null) { | ||
| const anchorOffset = selection.anchorOffset; | ||
| const focusOffset = selection.focusOffset; | ||
|
|
||
| if ( | ||
| selection !== null && |
There was a problem hiding this comment.
Safe to remove since L124 checks this
* core: add OutlineTextNode splitText with selection tests * core: fix core
Summary
Follow up of #118, where we add
splitText()with current selections.Test Plan
Tests added.