Fix(Page Break) : Children of root nodes must be elements or decorators" invariant isn't enforced #7703
Conversation
Jynx2004
requested review from
zurfyx,
fantactuka,
acywatson,
Fetz,
ivailop7,
Sahejkm,
potatowagon,
lilshady and
takuyakanbr
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
meta-cla
bot
added
the
CLA Signed
There was a problem hiding this comment.
This is probably a reasonable idea but it doesn't really make sense to change @lexical/table to support nodes that are only in the playground. There are two approaches that would make more sense to merge:
- Figure out a more general heuristic to use, it's likely that the issue you're trying to solve happens any time the last node of the cell is a DecoratorNode that is not inline, for example (
$isDecoratorNode(node) && !node.isInline()). I don't think page break nodes are generally very special, so it doesn't make sense to solve one specific manifestation of the problem when you can solve the entire class of problem. - Implement workarounds for playground nodes in the playground itself (not sure what the best solution for this would be, would require auditing more of the relevant code, and it's likely much harder than the above approach).
It's also important to implement at least one test that demonstrates that this addition actually does what it intends to, and the test will prevent that behavior from regressing as the code around it changes.
| ); | ||
| const lastChild = tableCellNode.getLastChild(); | ||
| const isLastChildPageBreak = | ||
| lastChild && lastChild.getType() === 'page-break'; // Adjust 'page-break' to the actual node type for page breaks |
There was a problem hiding this comment.
page-break is currently a playground-only node that is not distributed with lexical so it doesn't really make sense to encode specific behavior for it in the @lexical/table package. This method of type checking is also not compatible with subclasses which would have a different type (this is important for node replacement).
| // Optionally, move the selection to the new paragraph | ||
| newParagraph.selectEnd(); |
There was a problem hiding this comment.
optional isn't a very accurate description since this always happens and there's no way to control it
Summary:
Updated the CONTROLLED_TEXT_INSERTION_COMMAND handler in tableObserver.js to improve paragraph insertion behavior within table cells when the last child is a page-break node.
Changes Made:
Modified the logic in the CONTROLLED_TEXT_INSERTION_COMMAND handler to insert a new paragraph node containing the text payload directly after the last child of a TableCellNode when:
The last child is a page-break node.
No edge position is defined (edgePosition === undefined).
Replaced the existing $insertParagraphAtTableEdge function call with direct node manipulation using the insertAfter method to append the new paragraph immediately after the page-break node.
Added selectEnd to the new paragraph node to position the cursor at the end of the inserted paragraph, enhancing user experience.
Impact:
Ensures precise placement of new content within table cells following a page-break node.
Improves editor usability by correctly positioning the cursor after insertion.
Closes : #7625