Skip to content

[lexical-playground] Bug Fix: Allow deleting empty column layouts via backspace #7636

Merged
ivailop7 merged 5 commits intofacebook:mainfrom
Lakshmanshankar:delete-empty-column-layout
Jun 26, 2025
Merged

[lexical-playground] Bug Fix: Allow deleting empty column layouts via backspace #7636
ivailop7 merged 5 commits intofacebook:mainfrom
Lakshmanshankar:delete-empty-column-layout

Conversation

@Lakshmanshankar
Copy link
Copy Markdown
Contributor

Description

Previously, column layouts could not be deleted using only the keyboard — even if they were empty.
This PR enables backspace to delete an empty column layout only when:

  • Cursor is at offset 0 in the first column
  • All columns are empty

Closes #7615

Test plan

Before

  • Pressing backspace at the start of an empty column layout did nothing.

After

Screencast.from.21-06-25.08.47.44.AM.IST.webm

@vercel
Copy link
Copy Markdown

vercel bot commented Jun 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 25, 2025 6:57pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 25, 2025 6:57pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 21, 2025
Copy link
Copy Markdown
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The big thing missing here is a test that shows that this works, but I think it can be cleaned up a bit more by using collapseAtStart instead of handling backspace specifically

Comment on lines +124 to +125
const firstLayoutItemKey = container.getFirstChild()?.__key;
if (firstLayoutItemKey !== currentLayoutItem?.__key) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const firstLayoutItemKey = container.getFirstChild()?.__key;
if (firstLayoutItemKey !== currentLayoutItem?.__key) {
const firstLayoutItemKey = container.getFirstChild()?.getKey();
if (firstLayoutItemKey !== currentLayoutItem?.getKey()) {

Comment on lines +200 to +204
editor.registerCommand(
KEY_BACKSPACE_COMMAND,
() => $deleteEmptyLayoutOnBackspace(),
COMMAND_PRIORITY_LOW,
),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be more easily implemented by having LayoutItemNode implement collapseAtStart with a similar implementation, e.g.

function $isEmptyLayoutItemNode(node: LexicalNode): boolean {
  return $isLayoutItemNode(node) && !node.getFirstDescendant();
}
export class LayoutItemNode extends ElementNode {
  // …
  collapseAtStart(): boolean {
    const parent = this.getParentNodeOrThrow();
    if (
      this.is(parent.getFirstChild()) &&
      parent.getChildren().every($isEmptyLayoutItemNode)
    ) {
      this.parent.replace($createParagraphNode()).select();
      return true;
    }
    return false;
  }
}

@Lakshmanshankar
Copy link
Copy Markdown
Contributor Author

Hi etrepum,

I've pushed the changes to use collapseAtStart() as suggested, and removed the manual Backspace handling logic. I’ve also verified the behavior in the playground, and it works as expected.

Just wanted to confirm — when you mentioned "The big thing missing here is a test that shows that this works," were you referring to adding a unit test under something like "/lexical-playground/unit/LayoutItem.test.tsx"?

Happy to add that if you'd like. Thanks again for the helpful feedback!

@etrepum
Copy link
Copy Markdown
Collaborator

etrepum commented Jun 23, 2025

Yes, a unit or e2e test is missing

@ivailop7 ivailop7 added this pull request to the merge queue Jun 26, 2025
Merged via the queue into facebook:main with commit d96c8e7 Jun 26, 2025
36 checks passed
@etrepum etrepum mentioned this pull request Jul 3, 2025
fantactuka pushed a commit that referenced this pull request Aug 11, 2025
… backspace (#7636)

Co-authored-by: Ivaylo Pavlov <ivailo90@gmail.com>
Co-authored-by: Bob Ippolito <bob@redivi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Columns Layout delete

4 participants