[lexical][lexical-table] Bug Fix: Delete empty paragraph before table#8669
[lexical][lexical-table] Bug Fix: Delete empty paragraph before table#8669Rohithmatham12 wants to merge 2 commits into
Conversation
|
Hi @Rohithmatham12! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
potatowagon
left a comment
There was a problem hiding this comment.
Reviewed by Navi (Tater Thoughts Bobblehead) on behalf of @potatowagon.
Assessment: LGTM ✅
This is a clean, well-scoped fix for #8075 (forward delete swallowed by shadow-root guard when an empty paragraph precedes a table).
What I checked:
- Logic correctness: The new guard only fires when
anchor.type === "element"ANDanchorNode.isEmpty()— it won't affect paragraphs with content. The shadow-root protection for the table itself is preserved (thereturn truestill fires after removing the empty node). - Caret handling:
$normalizeCaret($getChildCaret(nextSibling, 'next'))correctly positions selection at the start of the table boundary.$getCaretRange(caret, caret)creates a collapsed selection, which is the expected state after forward-deleting an empty paragraph into a table. - Edge cases: The
anchorNode.remove()is safe because we've confirmed it's empty. No risk of data loss. - Test coverage: The regression test properly reproduces the exact scenario (empty paragraph → table, forward delete from paragraph) and asserts the paragraph is gone while the table remains.
- www backwards compatibility: No API changes, no signature changes, no export changes. This is a pure behavioral fix for a previously-broken interaction. Safe for www consumers (MLCComposer, XDSRichText, EPS).
Minor note: CLA is unsigned (likely first-time contributor) — that'll need to be resolved before merge.
Ready to approve once CLA is signed and full CI matrix runs.
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
ee411a8 to
e37d040
Compare
|
Title and description doesn't match the pull request template |
Description
Forward delete from an empty paragraph immediately before a table was swallowed by the shadow-root protection in
RangeSelection.forwardDeletion. That guard correctly prevents deleting the table itself, but it also returned before the empty paragraph could be removed.This change preserves the protected table behavior while removing the empty anchor paragraph and moving the selection to the table boundary.
Closes #8075
Test plan
Before
Forward delete from an empty paragraph directly before a table left the empty paragraph in place, so the Delete key appeared to do nothing.
After
Forward delete removes the empty paragraph and keeps the table protected. Added regression coverage for the paragraph-before-table case.
Automated tests:
node node_modules/vitest/vitest.mjs --project unit packages/lexical-table/src/__tests__/unit/LexicalTableSelection.test.tsx --runnode node_modules/vitest/vitest.mjs --project unit packages/lexical/src/__tests__/unit/LexicalSelection.test.ts --runnode node_modules/prettier/bin/prettier.cjs --check packages/lexical/src/LexicalSelection.ts packages/lexical-table/src/__tests__/unit/LexicalTableSelection.test.tsxnode node_modules/eslint/bin/eslint.js packages/lexical/src/LexicalSelection.ts packages/lexical-table/src/__tests__/unit/LexicalTableSelection.test.tsx