Skip to content

[lexical-table] Chore: Fix test for nested table pasting#8088

Merged
etrepum merged 3 commits intofacebook:mainfrom
randal-atticus:fix-table-nested-pasting-test
Jan 21, 2026
Merged

[lexical-table] Chore: Fix test for nested table pasting#8088
etrepum merged 3 commits intofacebook:mainfrom
randal-atticus:fix-table-nested-pasting-test

Conversation

@randal-atticus
Copy link
Copy Markdown
Contributor

Description

When hasNestedTables: true, and you paste a table into another table, it flattens it. I'm working on adding a way to allow a table to be pasted into another table, and realised the test assumes we can already do that.

Before changing anything I want to make sure the test obeys the current behaviour.

Test plan

Before

Screen.Recording.2026-01-20.at.4.19.51.pm.mov

After

Behaviour unchanged:
https://github.com/user-attachments/assets/14e47f23-801a-4e7e-b5c1-4aad40507d0d

@meta-cla meta-cla 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 Jan 20, 2026
@vercel
Copy link
Copy Markdown

vercel bot commented Jan 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
lexical Ready Ready Preview, Comment Jan 20, 2026 8:36pm
lexical-playground Ready Ready Preview, Comment Jan 20, 2026 8:36pm

Request Review

),
);

tableObserver.listenersToRemove.add(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've hoisted this entire command handler up to LexicalTablePluginHelper, the only change is where editor != dispatchEditor is checked.

I had to move this up because none of these command handlers were being registered in tests.
Separately, I was wondering if you think all of the command handlers should be lifted out applyTableHandlers, because:

  • These handlers are not registered in tests (unless wrapped in a certain way)
  • This function is a mix of both DOM event handlers and lexical command handlers, perhaps should be registered separately
  • The command handlers are registered somewhat redundantly for each table (e.g. if you have 100 tables in the doc, there will be 100 CUT_COMMAND handlers, 100 FORMAT_TEXT_COMMAND handlers etc).

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.

Commands being registered per-table doesn't really make a lot of sense. I guess whoever did that originally was thinking that it would make sense to only register the commands when at least one table is in the document, and just didn't optimize for >1 table? These are the kinds of problems that extensions are much better at.

@randal-atticus
Copy link
Copy Markdown
Contributor Author

Sorry, fixing tests...

editor.registerCommand(
SELECTION_INSERT_CLIPBOARD_NODES_COMMAND,
({nodes, selection}, dispatchEditor) => {
(selectionPayload, dispatchEditor) => {
Copy link
Copy Markdown
Contributor Author

@randal-atticus randal-atticus Jan 20, 2026

Choose a reason for hiding this comment

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

this is structured a bit oddly but is effectively mirroring the previous implementation 1:1 (because the old SELECTION_INSERT_CLIPBOARD_NODES_COMMAND had COMMAND_PRIORITY_HIGH and fell through to this handler afterwards).

@etrepum etrepum added the extended-tests Run extended e2e tests on a PR label Jan 20, 2026
@etrepum etrepum added this pull request to the merge queue Jan 21, 2026
Merged via the queue into facebook:main with commit 30ada84 Jan 21, 2026
36 checks passed
@etrepum etrepum mentioned this pull request Jan 31, 2026
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.

2 participants