refactor: use getDOMSlot to build headings with anchors#3117
Conversation
fixes: - prevent quote node vertical scrollbar this was surfaced by headings on mobile due to the different line height
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e1ec893. Configure here.
| if (!node.hasAttribute(HEADING_LINK_ATTRIBUTE)) return null | ||
| return { | ||
| conversion: () => ({ node: null, forChild: () => null }), | ||
| conversion: () => ({ node: null }), |
There was a problem hiding this comment.
Legacy heading import duplicates text
Medium Severity
Removing forChild: () => null from the decorative heading-link import means text inside the permalink a is imported as well as sibling heading content. HTML produced by the previous sibling-anchor export (anchor text plus reconciled spans) can round-trip into duplicated heading text when pasted or imported.
Reviewed by Cursor Bugbot for commit e1ec893. Configure here.
There was a problem hiding this comment.
We render the lexical state and the HTML dynamically, this can't happen.
1 participant
Description
Before, to create headings with anchors, we would render both the heading and the anchor as siblings (Lexical reconciles this way) and hide the anchor behind the heading text via CSS. This required careful orchestration with the HTML placeholder and the live
readervariant.This PR, instead, uses the new
getDOMSlotAPI to create heading nodes with anchor support, wrapping the childrenTextNodesdirectly into the<a>anchor.other fixes:
this was surfaced by headings on mobile due to the different line height
Screenshots
Left is before, right is after
The left side shows the current behavior, anchor and
<span>as siblings controlled via CSS; the right side instead shows the right HTML without any kind of CSS hacks.Checklist
Are your changes backward compatible? Please answer below:
yes
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
7, can export html, create the right ssr html and works perfectly in editor and in reader.
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
Yes
Did you introduce any new environment variables? If so, call them out explicitly here:
No
Did you use AI for this? If so, how much did it assist you?
Yes,
getDOMSlotusageNote
Medium Risk
Changes reader DOM structure, SSR export, and HTML import for headings, but scope is limited to
SNHeadingNodeand related styles with plain-text-only permalink wrapping.Overview
Read-only heading permalinks no longer rely on a hidden, absolutely positioned anchor sibling. In the reader, plain-text headings (
SNHeadingNode) get an empty decorative<a>and LexicalgetDOMSlotreconciles text/line breaks inside that link so the visible title is the real permalink. Headings with rich inline content skip the wrapper and keep only theid.exportDOMmirrors that structure for SSR via anappendhook that drops exported children into the anchor.importDOMstill stripsdata-lexical-heading-linkanchors without duplicating link nodes on round-trip.Styles drop the overlay/
position: relativehacks;.sn-heading__linkis styled like normal heading text..sn-quoteaddsoverflow-y: hiddento avoid an unwanted vertical scrollbar (notably on mobile).Reviewed by Cursor Bugbot for commit e1ec893. Bugbot is set up for automated code reviews on this repo. Configure here.