Repository navigation
[lexical-mdast][lexical-rich-text] Bug Fix: Make the > shortcut match Markdown import, and leave a quote line by line with Enter and Backspace - #9486
Conversation
…e the same quote as Markdown import With MdastShadowRootQuoteExtension, a loaded Markdown quote is a shadow root QuoteNode that holds paragraphs. The > shortcut still made a quote that holds text directly. The two look the same, but Enter left the typed quote at once and never left the loaded one. The > shortcut now creates its quote with the registry's import handlers, so it makes the same kind of quote as import, and the typed text goes into a paragraph in it. Without the extension nothing changes. In @lexical/rich-text, Enter on an empty paragraph of a shadow root quote moves the paragraph out of the quote, the way Enter on an empty list item leaves the list. In the middle of the quote it splits the quote in two. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
…quote takes out only the first line Backspace at the start of the first line of a shadow root quote lifted every block out of the quote, so `> a` and `> b` both became plain paragraphs although the caret was only on `a`. Now QuoteNode.collapseAtStart moves only the first block out, the way Backspace at the start of a list item takes only that item out of the list. In a nested quote the line moves to the outer quote. The Enter tests move to a new QuoteNodeShadowRootKeys.test.ts next to the Backspace ones, because Backspace needs a root element (jsdom). Co-Authored-By: Claude Opus 5.5 <[email protected]>
conghuydever
changed the title
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for the careful write-up. The typed-versus-loaded quote mismatch is well explained, and the Before/After output in the test plan shows the new tests failing on main and passing with the fix. Leaving a shadow root quote line by line, the way you leave a list, reads as the right behavior, and quotes that don't opt in to shadow root behavior are untouched.
Needed before merge
Add a changeset. This changes what two published packages do: Enter on an empty line of a shadow root quote, Backspace at the start of its first line (and QuoteNode.collapseAtStart(), which now lifts only the first block), and the > shortcut with MdastShadowRootQuoteExtension. CONTRIBUTING.md ("Changesets") requires a changeset for every user-facing change, and AGENTS.md asks for a Verify: paragraph when a fix changes observable behavior. The description is also missing the pull request template's ## Changeset section. Something like .changeset/shadow-root-quote-keys.md:
---
'@lexical/rich-text': patch
'@lexical/mdast': patch
---
[fix] Leave a shadow root quote line by line with Enter and Backspace, and make the `> ` shortcut create the same quote as Markdown import
Verify: with `MdastShadowRootQuoteExtension`, typing `> a` makes a shadow root quote that holds a paragraph. Enter on an empty line of a shadow root quote moves the line out of the quote (after it, before it, or between the two halves of a split quote). Backspace at the start of the first line, and `QuoteNode.collapseAtStart()`, move only that block out instead of every block.pnpm run check-changesets validates it locally.
Suggestions
$createShortcutQuote falls back when a blockquote rule seeds an empty paragraph (packages/lexical-mdast/src/MdastShortcuts.ts:109). The imported quote is kept only if quote.isEmpty(). A rule that gives an empty quote a paragraph for the caret fails that check, and the repository's own dev-examples/mdast-editor alert rule does exactly that (src/extensions/MdastAlertExtension.ts:150-152). In that editor > still makes a quote that holds text, and Enter leaves it at once, which is the mismatch this PR fixes. Consider also accepting a shadow root quote whose only children are empty paragraphs, and putting the typed content into its first paragraph.
A browser test for Backspace. The Backspace cases in QuoteNodeShadowRootKeys.test.ts run under jsdom, which has no Selection.modify. So they skip the native measurement deleteCharacter makes in a browser, and every fixture has the quote as the first block. AGENTS.md ("Testing Strategy") puts behavior that depends on a real selection engine under packages/**/__tests__/browser/**. One case with a paragraph before the quote would guard it. Browser tests clean up with onTestFinished(() => editor.dispose()) rather than using.
| ): QuoteNode { | ||
| const imported = scanner.importBlock(match.node); | ||
| const quote = imported.length === 1 ? imported[0] : null; | ||
| return $isQuoteNode(quote) && quote.isEmpty() ? quote : $createQuoteNode(); |
There was a problem hiding this comment.
A blockquote import rule that seeds an empty paragraph in an empty quote (so the caret has somewhere to land) fails quote.isEmpty(), and the shortcut then falls back to a quote that holds text. dev-examples/mdast-editor's $importAlertBlockquote does exactly that (src/extensions/MdastAlertExtension.ts:150-152), so in that editor typed and loaded quotes still differ. Consider also accepting a shadow root quote whose only children are empty paragraphs, and moving remaining into its first paragraph.
There was a problem hiding this comment.
Done in 3ab8e40. $createShortcutQuote now also keeps a shadow root quote whose only children are empty paragraphs, and $applyBlock puts the typed text into its first paragraph (any other empty paragraphs are removed, so the quote holds one line, as the import of > a gives).
A new test in MdastShortcuts.test.ts ("a quote whose import rule gives it an empty paragraph") uses a blockquote rule shaped like $importAlertBlockquote: it types > a and compares the tree with the imported > a, and checks the caret is in that line. It fails without the change.
| if (key === 'Enter') { | ||
| editor.dispatchCommand(KEY_ENTER_COMMAND, null); | ||
| } else { | ||
| editor.dispatchCommand(DELETE_CHARACTER_COMMAND, true); |
There was a problem hiding this comment.
Under jsdom there is no Selection.modify, so this skips the native measurement deleteCharacter makes in a browser before it reaches $collapseAtStart, and every Backspace fixture puts the quote first. AGENTS.md ("Testing Strategy") puts behavior that depends on a real selection engine in packages/**/__tests__/browser/**. A browser case with a paragraph before the quote would guard this path. Use onTestFinished(() => editor.dispose()) there instead of using.
There was a problem hiding this comment.
Added in 3ab8e40: packages/lexical-rich-text/src/__tests__/browser/QuoteNodeShadowRootBackspace.test.ts. It mounts a paragraph p followed by a shadow root quote holding a and b, puts the caret at the start of a, presses Backspace with userEvent.keyboard('{Backspace}'), and expects p, a, then the quote with b, with the caret at the start of a. It cleans up with onTestFinished.
It passes in Chromium, Firefox and WebKit, and fails in Chromium with packages/lexical-rich-text/src/index.ts from main.
…port rule seeds for the > shortcut, and add a changeset and a browser test ## Description - The > shortcut kept the quote that the import makes only when it was empty. An import rule may give the empty quote an empty paragraph for the caret, as the mdast-editor example's alert rule does. Then the shortcut fell back to a quote that holds text, and Enter left that quote at once. Now a shadow root quote that holds only empty paragraphs is kept too, and the typed text goes into its first paragraph. - Add a changeset for @lexical/rich-text and @lexical/mdast. - Add a browser test for Backspace at the start of the first line of a shadow root quote that comes after a paragraph. The unit tests run Backspace under jsdom, which has no Selection.modify. ## Changeset .changeset/shadow-root-quote-keys.md ## Test plan ### Before $ pnpm exec vitest run packages/lexical-mdast/src/__tests__/unit/MdastShortcuts.test.ts × a quote whose import rule gives it an empty paragraph Tests 1 failed | 34 passed (35) The browser test fails with packages/lexical-rich-text/src/index.ts from main (e28f4dd). ### After $ pnpm exec vitest run packages/lexical-mdast/src/__tests__/unit/MdastShortcuts.test.ts Tests 35 passed (35) $ VITEST_BROWSER=chromium,firefox,webkit pnpm exec vitest run --project "browser*" packages/lexical-rich-text/src/__tests__/browser Test Files 15 passed (15) Tests 99 passed (99) Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Thanks for the review. All points are handled in 3ab8e40:
The description's Before and After now show the current output: 14 failed and 36 passed with the source from |
There was a problem hiding this comment.
Thanks — this looks good. Typed and loaded quotes now match with MdastShadowRootQuoteExtension, and leaving a shadow root quote line by line follows the list behavior closely. The changeset and the browser test from the earlier round are both in, and the new unit and browser tests fail against main and pass with this change. The suggestions below are optional and non-blocking.
Optional: >> and >- still make a quote that holds text (packages/lexical-mdast/src/MdastShortcuts.ts:111-118). CommonMark §5.1 allows a block quote marker without a following space, so >> a is a quote inside a quote and >- a a quote holding a list, which is what import gives with the extension. Typing >, >, Space keeps only the imported quote when it is empty or holds only empty paragraphs, so it falls back to $createQuoteNode(): the result exports as > a and leaves on the first Enter. This is unchanged from main, so it is not a regression; accepting a shadow root quote whose descendants are all empty blocks, and putting the typed text in its innermost first block, would close the gap, or a JSDoc sentence could note it.
Optional: a line holding only spaces does not leave on Enter (packages/lexical-rich-text/src/index.ts:365). $handleQuoteInsertParagraph requires paragraph.isEmpty(), while $handleListInsertParagraph in packages/lexical-list/src/formatList.ts, the behavior the comment cites, treats a whitespace-only item as empty.
Optional: the mdast-editor example's own Enter handler (dev-examples/mdast-editor/src/extensions/MdastEditorExtension.ts, the INSERT_PARAGRAPH_COMMAND handler at COMMAND_PRIORITY_BEFORE_EDITOR) still runs before the new rule for an empty last line. When that line is the quote's only line, which is what > now creates there through the alert rule's seeded paragraph, it leaves an empty quote behind: > , Enter, x exports > then x. Dropping its $isQuoteNode(parent) branch would let @lexical/rich-text remove the emptied quote. Examples need no changeset, per CONTRIBUTING.md ("Changesets").
| return $isQuoteNode(quote) && | ||
| (quote.isEmpty() || | ||
| (quote.isShadowRoot() && | ||
| quote | ||
| .getChildren() | ||
| .every(child => $isParagraphNode(child) && child.isEmpty()))) | ||
| ? quote | ||
| : $createQuoteNode(); |
There was a problem hiding this comment.
Optional: a marker typed without a space after > (>> , >- ) imports as a quote holding a nested quote or a list, which fails this check, so the shortcut falls back to a quote that holds text: typing >, >, Space, a exports > a, while loading >> a gives > > a. Unchanged from main; accepting a shadow root quote whose descendants are all empty blocks, and putting the typed text in its innermost first block, would close it.
| const quote = paragraph.getParent(); | ||
| if ( | ||
| !$isParagraphNode(paragraph) || | ||
| !paragraph.isEmpty() || |
There was a problem hiding this comment.
Optional: $handleListInsertParagraph treats a whitespace-only item as empty, so Enter on a list item holding ' ' leaves the list; here a line holding ' ' adds another line instead. Accepting a paragraph whose children are all blank text nodes would match the list behavior the comment above cites.
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
…rtcut-shadow-root # Conflicts: # packages/lexical-mdast/src/MdastShortcuts.ts # packages/lexical-mdast/src/__tests__/unit/MdastShortcuts.test.ts
There was a problem hiding this comment.
Thanks, this looks good. A typed > quote now matches the imported one with MdastShadowRootQuoteExtension, and leaving a shadow root quote line by line follows the list behavior closely. The changeset and the Backspace browser test from the earlier round are both in, and the new unit and browser tests fail against main and pass with this change. The suggestions below are optional and non-blocking.
Optional: >> and >- still make a quote that holds text (packages/lexical-mdast/src/MdastShortcuts.ts, $createShortcutQuote). CommonMark 0.31.2 §5.1 allows a block quote marker with no space after >, so >> a is a quote inside a quote and >- a a quote holding a list, which is what import gives with the extension. Typing >, >, Space keeps the imported quote only when it is empty or holds only empty paragraphs, so it falls back to $createQuoteNode(); the result exports as > a and leaves on the first Enter. This is unchanged from main, so it is not a regression. Accepting a shadow root quote whose descendants are all empty blocks, and putting the typed text into its innermost first block, would close the gap; a JSDoc sentence would at least set expectations.
Optional: a line holding only spaces does not leave on Enter (packages/lexical-rich-text/src/index.ts, $handleQuoteInsertParagraph). It requires paragraph.isEmpty(), while $handleListInsertParagraph in packages/lexical-list/src/formatList.ts, the list behavior the new comment points to, treats a whitespace-only item as empty.
Optional: the mdast-editor example's own Enter handler (dev-examples/mdast-editor/src/extensions/MdastEditorExtension.ts, the INSERT_PARAGRAPH_COMMAND handler at COMMAND_PRIORITY_BEFORE_EDITOR) still runs before the new rule on an empty last line. When that line is the quote's only line, which is what > now creates there through the alert rule's seeded paragraph, it leaves an empty quote behind: > , Enter, x exports > above x. Dropping its $isQuoteNode(parent) branch would let @lexical/rich-text remove the emptied quote. Examples need no changeset (CONTRIBUTING.md, "Changesets").
| const quote = paragraph.getParent(); | ||
| if ( | ||
| !$isParagraphNode(paragraph) || | ||
| !paragraph.isEmpty() || |
There was a problem hiding this comment.
Optional: isEmpty() makes a line holding only spaces add another line instead of leaving the quote. $handleListInsertParagraph in packages/lexical-list/src/formatList.ts, the list behavior the comment above points to, treats a whitespace-only item as empty; accepting a paragraph whose children are all blank text nodes would match it.
| ): QuoteNode { | ||
| const imported = scanner.importBlock(match.node); | ||
| const quote = imported.length === 1 ? imported[0] : null; | ||
| return $isQuoteNode(quote) && |
There was a problem hiding this comment.
Optional: >> and >- (a > marker with no space, CommonMark 0.31.2 §5.1) import with the extension as a nested shadow root quote or a quote holding a list, so this check falls back to $createQuoteNode() for them and the typed text lands in a quote that holds text, as on main. Accepting a shadow root quote whose descendants are all empty blocks, and filling its innermost first block, would make those match import too.
Description
The problem
Set up an editor with
MdastShortcutsExtension,MdastShadowRootQuoteExtensionandRichTextExtension.> a, put the caret aftera, and press Enter. You get a new line in the quote. Press Enter on that empty line: another empty line in the quote. And again. Enter never leaves the quote. If the quote is the last block of the document, the caret can't get below it.>, Space,a. It looks exactly like case 1. Press Enter: the caret is already outside the quote. You can't add a second line to it with Enter.> a/> bquote, put the caret at the start ofaand press Backspace. Both lines leave the quote, although the caret was only ona.Same keys (Enter,
b, Enter, Enter,c) onmain:>Spacea> a\n\nb\n\n\n\nc(bis already outside)> a> a\n>\n> b\n>\n>\n>\n> c(cis still inside)Why
There are two kinds of quote. With
MdastShadowRootQuoteExtension, import makes a shadow rootQuoteNodethat holds paragraphs. The>shortcut still makes a quote that holds its text directly. Enter ends a quote that holds text. In a shadow root quote, Enter only ever adds paragraphs, because@lexical/rich-texthas no Enter rule for it. AndQuoteNode.collapseAtStartlifts every block of a shadow root quote out at once.The fix
With this PR, a typed and a loaded quote are the same kind, and you leave a shadow root quote line by line, the way you leave a list. The same keys give
> a\n>\n> b\n\ncfor both.@lexical/mdast:>shortcut creates its quote with the registry's import handlers (newMarkdownStreamScanner.importBlock, next toimportInline), so it makes the same kind of quote as import. WithMdastShadowRootQuoteExtensionthat is a shadow root quote, and the typed text goes into a paragraph in it. Without the extension nothing changes.dev-examples/mdast-editoralert rule does. A shadow root quote that holds only empty paragraphs is kept too, and the typed text goes into its first paragraph.QuoteNodeas before.>typed at the start of a shadow root quote makes a nested shadow root quote, the same tree as> > a.@lexical/rich-text, for shadow root quotes only:$copyNodeof the quote, so it keeps the shadow root state, format and direction. A quote left empty is removed.QuoteNode.collapseAtStart) moves only that block out of the quote, the way Backspace at the start of a list item takes only that item out. Before, it lifted every block. The existingcollapseAtStart()test is updated for this.INSERT_PARAGRAPH_COMMANDhandler, so Shift+Enter (INSERT_LINE_BREAK_COMMAND) is not touched.Not in this PR: a
<blockquote>pasted as HTML still becomes a quote that holds text, because the defaulttext/htmlpaste uses$generateNodesFromDOMandQuoteNode.importDOM. Changing that default would change paste for every app. Apps can already opt in withShadowRootQuoteRuleandClipboardDOMImportExtension.AI disclosure: the code, tests and this description were prepared with AI assistance.
Changeset
.changeset/shadow-root-quote-keys.md:@lexical/rich-textand@lexical/mdastpatch,[fix], with aVerify:paragraph.Test plan
New
packages/lexical-rich-text/src/__tests__/unit/QuoteNodeShadowRootKeys.test.ts:New
packages/lexical-rich-text/src/__tests__/browser/QuoteNodeShadowRootBackspace.test.ts: Backspace in a real browser at the start of the first line of a shadow root quote that comes after a paragraph. The unit tests run Backspace under jsdom, which has noSelection.modify, and their quote is always the first block.New
describe('> makes the same quote as import')inpackages/lexical-mdast/src/__tests__/unit/MdastShortcuts.test.ts. It compares the typed tree as JSON with the imported one for> a(with and without the extension),> **a**with the text already after the marker,> ## H2,> > a, and a quote whose import rule gives it an empty paragraph. It also checks that>+ Enter puts the caret on the empty line in the quote, and that the Enter steps above give the same Markdown and tree in a typed and a loaded quote.Before
The new and changed unit tests with
MdastShortcuts.ts,MdastStream.ts,MdastExtension.tsandpackages/lexical-rich-text/src/index.tsfrommain(e28f4dd), test file paths left out:Five of the new tests pass on
maintoo and guard against regressions: Enter on a line with text, Backspace on the only line and on the second line, the quote without the extension, and> ## H2. The browser test fails withpackages/lexical-rich-text/src/index.tsfrommain.After
The one
test-unitfailure is inscripts/__tests__/unit/changelog.test.ts, which fails on and off in my checkout, a different case each run (anENOTEMPTYwhile removing its own temp folder, or a failedgitcall in its temp repository). This PR changes no file underscripts/.pnpm run tscis clean, andpnpm exec eslintandpnpm exec prettier --checkonpackages/lexical-mdast/srcandpackages/lexical-rich-text/src, andpnpm run check-changesets, are clean.Tested on macOS with Node 24.21.0, browser tests in Chromium, Firefox and WebKit through Playwright. E2E and the playground were not run. The playground does not use shadow root quotes.
🤖 Generated with Claude Code