[lexical][lexical-clipboard] Bug Fix: Preserve alignment when pasting selected text#8671
[lexical][lexical-clipboard] Bug Fix: Preserve alignment when pasting selected text#8671Rohithmatham12 wants to merge 3 commits into
Conversation
|
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 with minor note ✅
What this does: Fixes #8101 — when copying text from an aligned paragraph within the same Lexical editor, the application/x-lexical-editor clipboard JSON only contained the TextNode, losing block-level formatting (alignment, indent). The fix adds $shouldIncludeElementWithSelectedChildren() which includes the parent element in the clipboard payload when the selected children cover the full element text content.
What I checked:
- Logic correctness: The function checks: (a) is range selection, (b) node is non-inline ElementNode, (c) not root, (d) selectedNodes content under this parent matches the parent's full text content. This correctly identifies "entire paragraph selected" without false positives on partial selections.
- Edge cases considered:
- Partial text selection →
selectedTextContent !== node.getTextContent()→ returns false ✅ - Empty paragraph →
selectedTextContent !== ""guard → returns false ✅ - Inline elements (links, marks) →
node.isInline()guard → skipped ✅ - Root node →
$isRootNode(node)guard → skipped ✅ - Multiple children in paragraph (e.g. bold+plain) → uses
$sliceSelectedTextNodeContentper node → correct ✅
- Partial text selection →
- Performance note: The function iterates selectedNodes and calls
isParentOf()for each. For very large selections this is O(n×depth), but clipboard operations are user-initiated and typically small. Acceptable. - Test coverage: Good test — creates a centered paragraph, copies via
$getClipboardDataFromSelection, pastes into another paragraph, asserts alignment is preserved in DOM output. - www compat: No API signature changes.
$shouldIncludeElementWithSelectedChildrenis module-private. The behavioral change (richer clipboard payload) is strictly additive — paste targets that ignore element wrappers will work as before.
One observation: The text-content comparison (selectedTextContent === node.getTextContent()) is a heuristic — it could theoretically match if a partial selection happens to produce the same string as the full element (e.g., repeated characters). This is an extremely unlikely edge case and the consequence (including alignment where it shouldn't) is benign.
CI: CLA ✅, Vercel ✅. Full test matrix pending (PR is ~30min old). Recommend waiting for unit green.
Ready to approve once full CI passes.
|
Title and description doesn't match the pull request template |
|
The approach makes sense, but Details and a suggested fix inline. Also noted how |
| if (selectedNodes.length === 0) { | ||
| return false; | ||
| } | ||
| const selectedTextContent = selectedNodes |
There was a problem hiding this comment.
selection.getNodes() returns intermediate element nodes too — e.g. for "Hello " + <a>world</a>, it returns [TextNode, LinkNode, TextNode]. The join then produces "Hello worldworld" (LinkNode's getTextContent() + child TextNode), which never matches the paragraph's "Hello world". So this function returns false for any paragraph containing links, marks, or other inline elements.
Filtering to leaf nodes (e.g. .filter(n => !$isElementNode(n))) before the comparison would fix it.
|
|
||
| if (shouldInclude && !shouldExclude) { | ||
| targetArray.push(serializedNode); | ||
| } else if ( |
There was a problem hiding this comment.
For context — HeadingNode already overrides extractWithChild to return true, so heading alignment is preserved on copy without needing this new function. ParagraphNode doesn't override it (defaults to false), which is why paragraph formatting gets stripped.
The full-text check makes sense if you only want full-paragraph selections to carry alignment. Just noting the existing mechanism since it covers a related case.
|
|
||
| target.select(0, target.getChildrenSize()); | ||
|
|
||
| const selection = $getSelection(); |
There was a problem hiding this comment.
target.select() returns RangeSelection directly — the $getSelection() + invariant($isRangeSelection) round-trip can be replaced with const selection = target.select(0, target.getChildrenSize()).
There was a problem hiding this comment.
const selection = target.select(0) is also sufficient, the default for both select arguments is the end (target.select() and target.selectEnd() are equivalent)
|
Addressed the review feedback in
Re-ran: |
Description
When copying selected text from an aligned paragraph inside the same Lexical namespace, the
application/x-lexical-editorclipboard payload took priority overtext/html. That JSON payload only contained the selectedTextNode, so pasting into another paragraph reused the destination paragraph's default alignment and discarded the source paragraph alignment.This updates clipboard JSON export to retain the non-inline parent element when the selected child content covers the full element text. That preserves block-level formatting such as paragraph alignment for full-paragraph text selections while partial text selections remain text-only.
Closes #8101
Test plan
Before
Copying all visible text from an aligned paragraph and pasting it elsewhere lost the source paragraph alignment.
After
Full-paragraph text selections preserve block-level alignment on paste, including paragraphs that contain inline elements such as links, while partial text selections remain text-only.
Automated tests:
node node_modules/vitest/vitest.mjs --project unit packages/lexical/src/__tests__/unit/HTMLCopyAndPaste.test.ts --runnode node_modules/vitest/vitest.mjs --project unit packages/lexical-clipboard/src/__tests__/unit/GetClipboardDataExtension.test.ts packages/lexical-clipboard/src/__tests__/unit/ClipboardImportExtension.test.ts --runnode node_modules/prettier/bin/prettier.cjs --check packages/lexical-clipboard/src/clipboard.ts packages/lexical/src/__tests__/unit/HTMLCopyAndPaste.test.tsnode node_modules/eslint/bin/eslint.js packages/lexical-clipboard/src/clipboard.ts packages/lexical/src/__tests__/unit/HTMLCopyAndPaste.test.ts