Preserve whitespace around custom attachment.#1102
Open
arthurwozniak wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds regression coverage and adjusts HTML import conversion to correctly preserve whitespace adjacent to custom action text attachments, including handling NBSP without introducing extra “phantom” spaces.
Changes:
- Added browser tests to validate whitespace preservation around custom attachments (including NBSP cases).
- Updated DOM conversion logic to detect ASCII whitespace adjacent to attachments without matching NBSP.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| test/browser/tests/attachments/custom_attachment_whitespace.test.js | Adds Playwright coverage for whitespace behavior around custom attachments. |
| src/nodes/custom_action_text_attachment_node.js | Tweaks whitespace detection regex to avoid treating NBSP as whitespace while preserving adjacent ASCII whitespace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+30
to
36
| // Preserve the space Lexical strips next to the attachment. ASCII only: | ||
| // \s also matches NBSP, which Lexical keeps, so it would add a phantom space. | ||
| const nodes = [] | ||
| const previousSibling = attachment.previousSibling | ||
| if (previousSibling && previousSibling.nodeType === Node.TEXT_NODE && /\s$/.test(previousSibling.textContent)) { | ||
| if (previousSibling && /[ \t\r\n]$/.test(previousSibling.textContent)) { | ||
| nodes.push($createTextNode(" ")) | ||
| } |
Comment on lines
+48
to
50
| if (nextSibling && /^[ \t\r\n]/.test(nextSibling.textContent)) { | ||
| nodes.push($createTextNode(" ")) | ||
| } |
| const nodes = [] | ||
| const previousSibling = attachment.previousSibling | ||
| if (previousSibling && previousSibling.nodeType === Node.TEXT_NODE && /\s$/.test(previousSibling.textContent)) { | ||
| if (previousSibling && /[ \t\r\n]$/.test(previousSibling.textContent)) { |
Author
There was a problem hiding this comment.
PR preserves original codestyle.
|
|
||
| const nextSibling = attachment.nextSibling | ||
| if (nextSibling && nextSibling.nodeType === Node.TEXT_NODE && /^\s/.test(nextSibling.textContent)) { | ||
| if (nextSibling && /^[ \t\r\n]/.test(nextSibling.textContent)) { |
Author
There was a problem hiding this comment.
PR preserves original codestyle.
Comment on lines
+34
to
36
| if (previousSibling && /[ \t\r\n]$/.test(previousSibling.textContent)) { | ||
| nodes.push($createTextNode(" ")) | ||
| } |
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.
Summary
Importing HTML into the editor dropped the space between formatted text and an inline custom attachment (mention, placeholder):
<strong>Link: </strong><action-text-attachment>came back asLink:glued to the attachment. The space survived in the saved value but vanished on re-edit.Lexical strips the ASCII whitespace bordering an inline decorator on import, and
CustomActionTextAttachmentNodere-adds it from the neighbouring sibling. That restore only ran for bare text nodes, so when the bordering text was wrapped in an inline format (<strong>,<mark>, etc) the sibling was an element and the space was lost.Changes
src/nodes/custom_action_text_attachment_node.js- read the sibling'stextContentinstead of requiring a bare text node, so the space counts inside inline formats. Match ASCII whitespace ([ \t\r\n]) rather than\s, which also matches NBSP - whitespace Lexical keeps - and would inject a phantom space.