Repository navigation
[lexical-selection] Bug Fix: Wrap a block leaf in an element that is not a block container on its own - #9664
Merged
Merged
Conversation
…not a block container on its own ## Description Closes #9638. `$wrapNodes` groups the selected leaves by the block that holds them and moves each block's content into a new element. A block leaf (a block decorator, such as a horizontal rule) in an element that is not a block container (a box that declares no structure, without `NodeStructureExtension`) took that element for its block: all the box's children moved into one new element, those before the range included, and the new elements were then inserted next to one of them (the paragraph before the range), inside that new element. The tree had a cycle and the update never returned. Such a leaf is now wrapped on its own, as it already is in a block container, and where the range starts at it the new elements go where it is. Block containers and inline content are unchanged. ## Test plan `WrapNodesBlockLeaf.test.ts` (a guard turns a parent cycle into an error, so a hang fails the test instead of the run): - the issue's shape: a range from inside "bb" to the end of "cc" across a rule in a box keeps "aa", "bb", the rule and "cc" in order, wraps "bb", the rule and "cc", and the rule stays in the box; - a range that starts at the rule; - controls: the same range without the rule; a rule at the root; - a property over 300 random documents of paragraphs, rules and boxes (nested up to twice) and random ranges between text and element points: `$wrapNodes` returns and keeps every text, in order. ### Before main (c0c7e42): the issue's repro never returns. With the guard: ``` Tests 3 failed | 2 passed (5) × a range across a horizontal rule in a box keeps every paragraph × a range that starts at the rule × property: random box, rule and paragraph mixes ``` ### After ``` $ vitest --run --project unit packages/lexical-selection/src/__tests__/unit/WrapNodesBlockLeaf.test.ts Tests 5 passed (5) ``` All unit tests: 24,722 passed, 421 expected fail, 0 failed. `pnpm run ci-check` passes.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
zurfyx
requested review from
acywatson,
etrepum,
ivailop7 and
potatowagon
as code owners
October 31, 2026 05:44
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
…its parent holds blocks only ## Description Review of the first commit found a new hang: a box holding text directly, two rules and more text, with the range from the second rule to the end. The second rule became its own unit and the anchor for the new elements was the first rule, but the text's unit (the box, the block of its direct text) then moved all the box's children, the first rule included, into a new element: the elements were inserted next to a node inside that element. A block leaf is now its own unit only where its parent holds no inline content directly (a box of blocks, the shape of #9638). Where the parent holds text directly, the parent is the block of that text and of the leaf, as before. ## Test plan `WrapNodesBlockLeaf.test.ts` adds the review's case (forward and backward; both hang on the previous head, pass here), and the property generates adjacent rules. With text directly in a box, `$wrapNodes` reorders text or never returns in some documents on main too: that property is pinned as `test.fails` and filed as #9665 (main fails 8 of its 500 documents, this change 7, all among main's). All unit tests pass (`DisposeMemory.test.ts`'s heap bound failed once under 16 workers and passes alone). `pnpm run ci-check` passes.
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks, this looks good. $wrapNodes now treats a horizontal rule in a box that holds only blocks the same way it already treats one at the root, and the new tests, including the seeded property, fail on main and pass here. The suggestions below are optional and non-blocking.
- Optional: also check that the parent is a block (
packages/lexical-selection/src/range-selection.ts:304-312).$isBlockLeafInElementchecks only that the parent is not a block container and has no inline children. Take a block decorator inside an inline element:p("x", link(<hr>)), p("cc"), with the range starting at the link. The rule becomes the insertion point, and both quotes land inside the paragraph after"x", givingp("x", quote(<hr>), quote("cc")). Onmainthe same range givesquote("x", link(<hr>)), quote("cc"). A block inside an inline element isn't a valid structure to begin with, but addingINTERNAL_isInlineContent(parent)to the early return would keep such a leaf on the existing$wrappedBlockpath and leave the issue's shape unchanged. - Optional nit: a paragraph holding only a block decorator loses its format and indent.
p[center, indent 2](<hr>)now takes the run path, which skips the format and indent copy (range-selection.ts:642-645). The new quote therefore no longer carries the paragraph's alignment and indent, as it did onmain. - Optional: changeset wording (
.changeset/selection-wrap-block-leaf.md). The entry is valid under.changeset/README.md, which asks each entry to describe the change from the user's side. Callers whose box shape didn't hang will still see two differences: the rule is now wrapped in its own element, and siblings after the range stay in the box. A clause such as "the rule is wrapped on its own, as at the root" in the summary would tell changelog readers about it.
| parent === null || | ||
| !$isLeafNode(node) || | ||
| INTERNAL_isInlineContent(node) || | ||
| $isBlockContainer(parent) |
There was a problem hiding this comment.
Optional: this never checks that parent is a block. For a block decorator inside an inline element (p("x", link(<hr>)), with the range starting at the link), the rule becomes the insertion point, and the quotes land inside the paragraph after "x". Adding INTERNAL_isInlineContent(parent) || here would keep that case on the existing $wrappedBlock path without affecting #9638's shape.
| '@lexical/selection': patch | ||
| --- | ||
|
|
||
| [fix] `$wrapNodes` over a range across a block decorator (a horizontal rule) in an element that is not a block container returns, and keeps every paragraph |
There was a problem hiding this comment.
Optional: callers whose box shape didn't hang will still see that the rule is now wrapped in its own element and that siblings after the range stay in the box. A clause such as "the rule is wrapped on its own, as at the root" would tell them about it.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes #9638.
$wrapNodesgroups the selected leaves by the block that holds them andmoves each block's content into a new element. A block leaf (a block
decorator, such as a horizontal rule) in an element that is not a block
container (a box that declares no structure, without
NodeStructureExtension) took that element for its block: all the box'schildren moved into one new element, those before the range included, and
the new elements were then inserted next to one of them (the paragraph
before the range), inside that new element. The tree had a cycle and the
update never returned.
Such a leaf is now wrapped on its own, as it already is in a block
container, where its parent holds blocks only (the shape of this issue),
and where the range starts at it the new elements go where it is. Where the
parent also holds text directly, the parent stays the block of that text and
of the leaf, as before (doing otherwise anchored the new elements on a rule
the text's unit then moved). Block containers and inline content are
unchanged.
With text directly in a box next to its blocks,
$wrapNodescan reordertext or never return on main too: filed as #9665, and pinned here as a
test.failsproperty (main fails 8 of its 500 documents, this change 7, allamong main's).
Changeset
.changeset/selection-wrap-block-leaf.md(@lexical/selectionpatch,[fix]with aVerify:paragraph).Test plan
WrapNodesBlockLeaf.test.ts(a guard turns a parent cycle into an error,so a hang fails the test instead of the run):
rule in a box keeps "aa", "bb", the rule and "cc" in order, wraps "bb",
the rule and "cc", and the rule stays in the box;
backward): every text and rule stays;
too) and boxes (nested up to twice) and random ranges between text and
element points:
$wrapNodesreturns and keeps every text, in order; thesame with text directly in a box is pinned (Bug: [lexical-selection] $wrapNodes over a box that holds text directly next to its blocks can reorder text or never return #9665).
Before
main (c0c7e42): the issue's repro never returns. With the guard:
After
All unit tests: 24,722 passed, 421 expected fail, 0 failed.
pnpm run ci-checkpasses.