[lexical][lexical-rich-text][lexical-utils] Bug Fix: copy and insert keep the original node's state - #9059
Conversation
…insert keep the original node's state ## Description Five bugs, one shape. An operation that splits, copies or inserts a block builds the new node by hand instead of deriving it from the node it came from, so the original's mode, alignment, format or style is silently dropped — or the split leaves an extra empty block behind. This is the same defect class already seen in the `insertNewAfter` paths that construct a node from scratch rather than reusing the original's state. - `TextNode.splitText` (`packages/lexical/src/nodes/LexicalTextNode.ts`) copied format, style, detail and NodeState onto the parts it creates but not `__mode`. Splitting a token TextNode left a token head followed by normal-mode tails, which are editable and can be merged into neighbouring text by normalization. Every part now keeps the mode of the node that was split; segmented nodes keep their existing behaviour of being downgraded to normal. (facebook#8924) - `ParagraphNode.extractWithChild` (`packages/lexical/src/nodes/LexicalParagraphNode.ts`) did not exist, so `ParagraphNode` inherited the default of `false` and copying a single, fully selected paragraph dropped the paragraph itself from both clipboard payloads — its alignment, indent and style had nowhere to live and the paste came out left aligned. `HeadingNode` and `QuoteNode` already override this. `ParagraphNode` now does too, but only when the selection covers the whole paragraph and that paragraph actually carries block-level state, so a plain paragraph still copies as the inline-only content existing clipboard consumers expect. (facebook#8944, fixes facebook#8101) - `ParagraphNode.collapseAtStart` (same file) decided a paragraph was "empty" from `children[0]` alone. A first paragraph whose first text node is blank but whose later children carry the content — a leading space followed by a bold run — was treated as empty, so Backspace at its start removed the whole paragraph and lost the content; a non-text child such as a line break was discarded the same way. It now requires every child to be a text node and the paragraph's whole text content to be blank. Blank and empty first paragraphs still collapse exactly as before. (facebook#8975) - `$insertNodeToNearestRoot` (`packages/lexical-utils/src/index.ts`) splits the enclosing blocks at the selection focus, so a focus at the end of a block left an empty right-hand half — a spurious empty paragraph or list item after the inserted node. That empty block is only wanted when the insertion point is at the very end of its nearest root, where it is the only place left for the selection to go, so a `$shouldSplit` now declines the empty `'last'` split when the new `$hasContentAfter` helper reports content still following the caret. (facebook#8976, fixes facebook#5433) - `QuoteNode.insertNewAfter` (`packages/lexical-rich-text/src/index.ts`) built the continuation block by hand, named its `RangeSelection` argument `_` and carried across only the direction, so Enter at the end of a centred quote produced a left-aligned paragraph while the identical keystroke in a centred paragraph did not. It now carries the block format and style and the caret's pending text format and style, matching core `ParagraphNode.insertNewAfter`. (facebook#9028) ## Test plan Each fix brings its own unit tests: `splitText keeps the token mode on every part` in `LexicalTextNode.test.tsx`, the new `Issue8101Repro.test.ts` and `ParagraphCollapseAtStart.test.ts` under `packages/lexical`, two new cases in `LexicalUtilsInsertNodeToNearestRoot.test.tsx`, and the new `QuoteInsertNewAfter.test.ts` under `packages/lexical-rich-text`. The quote tests run the same helper against a paragraph and against a quote and require them to agree, so they are pinned to core's behaviour rather than to a hardcoded expectation. facebook#8975 additionally adds a Playwright case to `packages/lexical-playground/__tests__/e2e/Selection.spec.mjs`; that spec needs a built playground and was not run locally, so it is covered by CI's e2e job rather than by the output below. ### Before Source fixes reverted, new tests kept: ``` $ npx vitest run --project unit packages/lexical/src packages/lexical-utils packages/lexical-rich-text × splitText keeps the token mode on every part × keeps a first paragraph whose content follows a blank text node × keeps a first paragraph whose only other child is a line break × copying a whole aligned paragraph keeps the alignment in the Lexical clipboard payload × copying a whole aligned paragraph keeps the alignment in the HTML clipboard payload × copy a centered paragraph and paste it into an empty paragraph × keeps the block alignment and style, as ParagraphNode does × seeds the continuation block with the pending text format × insert in the end of paragraph that is followed by another block × insert in the end of the last nested list item Test Files 5 failed | 83 passed (88) Tests 10 failed | 1622 passed (1632) ``` ### After ``` $ npx vitest run --project unit packages/lexical/src packages/lexical-utils packages/lexical-rich-text Test Files 88 passed (88) Tests 1632 passed (1632) $ npx tsc --noEmit -p . (clean) ``` Supersedes facebook#8924, facebook#8944, facebook#8975, facebook#8976, facebook#9028, consolidated per the review feedback on facebook#9027 and facebook#9035.
|
@LeSingh1 is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
I don't think all of these are the correct approach, I don't think the intended behavior is for splitting a token to create more tokens for example. mode should only be applied to the original node and anything else created from that should be normal unless the caller explicitly wants them to keep the mode. I had made a similar comment on the original #8924 |
Removes the change that carried a TextNode's mode onto every part of a split, along with its test. The mode downgrade is intentional, so this belonged in a separate discussion rather than in this PR.
|
You're right, and I should have taken the #8924 answer the first time. Dropped the What's left is the paragraph/quote/ |
|
A lint rule changed since this PR was opened so you'll need to apply |
…cal/utils` to `lexical`
Description
Four bugs, one shape. An operation that copies or inserts a block
builds the new node by hand instead of deriving it from the node it came
from, so the original's mode, alignment, format or style is silently
dropped — or the split leaves an extra empty block behind. This is the same
defect class already seen in the
insertNewAfterpaths that construct anode from scratch rather than reusing the original's state.
ParagraphNode.extractWithChild(
packages/lexical/src/nodes/LexicalParagraphNode.ts) did not exist, soParagraphNodeinherited the default offalseand copying a single,fully selected paragraph dropped the paragraph itself from both clipboard
payloads — its alignment, indent and style had nowhere to live and the
paste came out left aligned.
HeadingNodeandQuoteNodealreadyoverride this.
ParagraphNodenow does too, but only when the selectioncovers the whole paragraph and that paragraph actually carries
block-level state, so a plain paragraph still copies as the inline-only
content existing clipboard consumers expect. ([lexical] Bug Fix: keep paragraph alignment when copying a whole paragraph #8944, fixes Bug: Text alignment is lost on copy-paste #8101)
ParagraphNode.collapseAtStart(same file) decided a paragraph was"empty" from
children[0]alone. A first paragraph whose first text nodeis blank but whose later children carry the content — a leading space
followed by a bold run — was treated as empty, so Backspace at its start
removed the whole paragraph and lost the content; a non-text child such
as a line break was discarded the same way. It now requires every child
to be a text node and the paragraph's whole text content to be blank.
Blank and empty first paragraphs still collapse exactly as before. ([lexical] Bug Fix: collapseAtStart no longer discards a first paragraph that only starts blank #8975)
$insertNodeToNearestRoot(packages/lexical-utils/src/index.ts) splitsthe enclosing blocks at the selection focus, so a focus at the end of a
block left an empty right-hand half — a spurious empty paragraph or list
item after the inserted node. That empty block is only wanted when the
insertion point is at the very end of its nearest root, where it is the
only place left for the selection to go, so a
$shouldSplitnow declinesthe empty
'last'split when the new$hasContentAfterhelper reportscontent still following the caret. ([lexical-utils] Bug Fix: no extra paragraph when $insertNodeToNearestRoot splits at the end of a block #8976, fixes Bug: $insertNodeToNearestRoot inserts extra paragraph node if layout node is inserted at the end of text node. #5433)
QuoteNode.insertNewAfter(packages/lexical-rich-text/src/index.ts)built the continuation block by hand, named its
RangeSelectionargument_and carried across only the direction, so Enter at the end of acentred quote produced a left-aligned paragraph while the identical
keystroke in a centred paragraph did not. It now carries the block format
and style and the caret's pending text format and style, matching core
ParagraphNode.insertNewAfter. ([lexical-rich-text] Bug Fix: Enter at the end of a quote keeps the block alignment and style #9028)Test plan
Each fix brings its own unit tests: the new
Issue8101Repro.test.tsand
ParagraphCollapseAtStart.test.tsunderpackages/lexical, two newcases in
LexicalUtilsInsertNodeToNearestRoot.test.tsx, and the newQuoteInsertNewAfter.test.tsunderpackages/lexical-rich-text. Thequote tests run the same helper against a paragraph and against a quote and
require them to agree, so they are pinned to core's behaviour rather than to
a hardcoded expectation. #8975 additionally adds a Playwright case to
packages/lexical-playground/__tests__/e2e/Selection.spec.mjs; that specneeds a built playground and was not run locally, so it is covered by CI's
e2e job rather than by the output below.
Supersedes #8924, #8944, #8975, #8976, #9028, consolidated per the review
feedback on #9027 and #9035.