[Breaking Change][lexical] Bug Fix: Backspace at block start preserves the current block#8493
Conversation
|
I think this does make sense to scope down, but since it's more of a behavior change than a bug fix we should call it out with a |
|
Updated — |
|
For some reason the deployments aren't showing up for this branch, I don't have access to introspect vercel but I can't merge without some final QA just in case |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
The Vercel deployment seems to have gotten dropped somewhere between the title/body edit on the day I opened this and the breaking-change update — not sure why; happy to do whatever helps if you have a way to retrigger. In the meantime, ran through manual QA locally (Chromium and Safari) on top of the e2e CI runs that already covered chromium/firefox/webkit on macOS. Six scenarios:
All matched expected. Just pushed a no-op commit to nudge Vercel into rebuilding the preview. |
|
By the way, this PR reminded me of a problem: when the cursor is at the beginning of a heading and there are no sibling elements before it, pressing Backspace causes the heading to recreate. This doesn't happen with other elements Screen.Recording.2026-05-11.at.13.55.21.movI'm not sure if it's appropriate to fix this bug as part of this PR, but it seems to me that you're pretty close to a solution |
580ed62 to
8b6944e
Compare
|
@levensta — thanks for spotting this. Confirmed and folded in ( Root cause: QuoteNode and ListItemNode Tests added in
While addressing this, a closer look at the merge-block exception surfaced a related cascade gap: Updated PR title/body to cover both scopes. |
etrepum
left a comment
There was a problem hiding this comment.
On closer inspection I'm not sure we want this to happen when the preceding block is a ListNode, at least if we're mirroring what google docs does. I think the ListNode should not be considered empty because it has a ListItemNode
…s the current block
8b6944e to
01f7e49
Compare
|
@etrepum — agreed on the ListNode framing. Narrowed the exception to siblings at the same nesting level ( Pinned that behavior with a |
Description
When a HeadingNode (or any non-empty block ElementNode) sits directly after an empty ParagraphNode, pressing Backspace at the heading's start merges the heading's children into the empty paragraph and removes the heading. The heading's type gets discarded as part of the merge. Separately, when a non-empty HeadingNode is the first block in the document (no previous sibling), pressing Backspace at its start unconditionally replaces it with a brand-new HeadingNode of the same tag — the visible markup is unchanged but the node identity (
__key) churns on every Backspace. The Google Docs convention many users expect is the opposite: in the first case, remove the empty previous block and preserve the heading; in the second, leave the heading untouched.Breaking Change
Two scenarios change at Backspace at a block start:
removeText()calls and selections spanning non-empty blocks are unchanged.HeadingNode.collapseAtStartself-replaced with a fresh HeadingNode of the same tag every time Backspace was pressed, churning the node__keyand firing a phantomdestroyed+createdmutation pair even though the visible markup was unchanged. Now the heading is left in place: no__keychurn, no mutation pair, no resulting history entry. Mutation listeners onHeadingNode(e.g.LexicalTableOfContentsPlugin) and any external code tracking heading keys stop seeing these phantom events; the visible DOM is unchanged in both old and new behavior.Fix
Two narrow exceptions, both at the start of a block on Backspace.
Heading after an empty paragraph.
RangeSelection.deleteCharacterreaches themerge-blockbranch withstate.caret.originpointing at the empty adjacent block andstate.blockpointing at the (non-empty) anchor block. The branch then callsremoveText(), and the cross-block merge in$removeTextFromCaretRangealways wins on the earlier block's type — that's where the heading is lost. A narrow exception is added at the top of themerge-blockbranch: when the adjacent block is empty and the anchor block is not, remove just that empty adjacent block and leave the selection where it is.removeText()isn't called, so the cross-block merge never runs and the anchor block's type is preserved. Type-agnostic: applies to Heading, Quote, list items, custom subclasses without per-type branching. The exception is also limited to adjacent blocks at the same nesting level (caret.origin.getParent() === block.getParent()), so a structural wrapper like aListNodewhose only child is an emptyListItemNodefalls through to the default cross-block merge — theListNodeis not treated as empty just because its only child is. This mirrors Google Docs' Backspace-at-heading behavior next to an empty bullet (the heading text is pulled into the list item).Non-empty heading with no previous block.
RangeSelection.deleteCharacterfalls through the merge logic (no previous block to merge with) and ultimately invokes$collapseAtStart, which walks ancestors callingcollapseAtStartuntil one returnstrue.HeadingNode.collapseAtStartpreviously self-replaced with a freshHeadingNodeof the same tag (and moved children over) even when non-empty — semantically a no-op but observable as__keychurn. The non-empty branch now just signals "handled" by returningtruewithout replacing. The empty-heading branch (heading → paragraph) is unchanged. QuoteNode and ListItemNodecollapseAtStartare intentionally untouched: they implement escape semantics ("Backspace at start escapes to paragraph" / list outdent), so the second exception stays Heading-specific, matching levensta's report.Scope
Two adjacent cases are still out of scope:
removeTextmerge policy. Changing the cross-block merge itself (e.g. "later block wins" or "non-empty wins") would also affect directremoveText()calls and user-initiated selections that span non-empty blocks. That's a broader observable behavior change for any caller relying on the current "earlier block's type wins" rule.LineBreakNodecase. levensta noted on the issue thread that a non-empty paragraph ending in aLineBreakNodetriggers the same merge. Treating that as "empty enough" to qualify for the exception is a judgment call about what counts as empty, and seems worth its own discussion.If a broader scope is preferred, drop a comment and I'll fold either (or both) into this PR.
Refs #4359
Test plan
pnpm vitest run --project unit— 113 files / 2493 tests pass. NewBackspace at start of heading (#4359)describe block inLexicalHeadingNode.test.ts:test.forcases for the merge-block exception: heading preserved after empty paragraph; quote preserved after empty paragraph (type-agnostic); only the empty paragraph is removed between two non-empty siblings; legacy cross-block merge still applies when previous block is non-empty (out-of-scope marker); aListNodewhose only child is an emptyListItemNodefalls through to the default cross-block merge (heading text is pulled into the list item), pinning the same-nesting-level limit on the exception.HeadingNode.collapseAtStartcases for the second exception: non-empty heading preserved with samegetKey()(catches the recreate regression — the previous code self-replaced and the test fails without the fix); preserved when the heading is wrapped inside another ElementNode; preserved when the heading is followed by a non-paragraph sibling (e.g. a list); empty heading still converts to paragraph (existing behavior).pnpm tscclean.Headings/on chromium — 4/4 pass. The existing'... stays as a heading when you backspace at the start of a heading with no previous sibling nodes present'test covers the DOM-level behavior of the second exception; thegetKey()check sits in the unit suite because the regression is invisible at the DOM level. Firefox/webkit weren't re-run this revision — no e2e files changed.Note: the new direct
HeadingNode.collapseAtStarttests invoke the method directly because jsdom doesn't implementSelection.modify, whichdeleteCharacterfalls through to when no previous block exists. The end-to-end keyboard path is covered by the existing e2e test in theHeadings/suite.