[lexical-playground] Bug Fix: Table action menu visibility with cell overflow

[kirandash]

Description

Current behavior:
Currently, the table action menu button remains visible even when a cell is partially visible due to overflow, as reported in issue #7331. This creates a confusing user experience since the menu appears for cells that are not fully in view.

Changes being added:
This PR improves the table action menu visibility check by properly handling cell overflow in the scrollable container. The menu is now correctly hidden when a cell is partially visible and reappears when scrolled into full view.

Technical Changes:

1. Created checkTableCellOverflow in TableActionMenuPlugin to:

   • Check direct cell overflow (width/height)
   • Verify cell is fully visible within its scrollable container
   • Handle both single cell and table selection cases

2. Added E2E test to verify:

   • Menu visibility when cell has no overflow
   • Menu hidden when cell overflows

Closes #7331

Test plan

Before

• Table action menu button remains visible for partially visible cells
• No E2E test coverage for overflow behavior

lexical-bug-7331.mp4

After

• Table action menu correctly hides when cells overflow
• Menu appears when cells are fully visible
• Added E2E test Table action menu is hidden when cell overflows

lexical-7331-fix.mov

[vercel]

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	Mar 19, 2025 11:55am
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 19, 2025 11:55am

[etrepum]

This does generally solve the problem but there is a small issue in that we don't update this position when scrolling (not a regression, it occurs before this PR as well). You can reproduce by selecting a fully visible cell and then scrolling the table (the menu stays in place but the cell moves) or selecting an overflowed cell and then scrolling it to be fully visible (no menu appears).

Since this PR strictly makes forward progress I'm inclined to merge as-is, and handling scrolling or revisiting the popover strategy in a more systematic way would make sense.

Generally speaking we have a lot of popover related issues in the playground because none of them are quite implemented correctly popover Issues related to popover/menu code in the playground

It would make sense to finish up #6923 or do something equivalent to that eventually.

Alternatively it might also be possible to do something in the internal getDOMSlot API (#6759) to allow managed portals to be attached to the end of ElementNode and then ignored by the reconciler and mutation observer… but that would require a lot more thought since it has to tie in with the createDOM/updateDOM lifecycle as well (elements can be destroyed and re-created at any time).

Looks like some e2e test need to be addressed

[riley-pearce-airteam]

Something I hadn't considered when I created this issue is that the action button can also overflow on the left hand side when a small portion of the cell is still visible on the left.

[kirandash]

Something I hadn't considered when I created this issue is that the action button can also overflow on the left hand side when a small portion of the cell is still visible on the left.

Thanks for catching this! I've updated the checkTableCellOverflow function to handle overflow on both sides. Now the action menu will be hidden when a cell is partially visible on either the left or right side of the scrollable container.

Let me know if you notice any other edge cases that need to be addressed!

[kirandash]

@etrepum Thank you for the detailed feedback. I understand there are still underlying issues with popover positioning during scrolling that need to be addressed more systematically. And I agree that completing #6923 would be the right long-term fix.

For now, I've focused on fixing the immediate e2e test failures to unblock the pipeline, but I'll look into contributing to the systematic popover positioning solution which is WIP as well.

I've fixed the failing e2e test for the table action menu visibility in two ways:

1. For collab mode: Added test.skip(isPlainText || isCollab)
2. For Firefox: Added test.fixme(browserName === 'firefox') since the clicks work manually but are unreliable in automated tests

[rilrom]

Something I hadn't considered when I created this issue is that the action button can also overflow on the left hand side when a small portion of the cell is still visible on the left.

Thanks for catching this! I've updated the checkTableCellOverflow function to handle overflow on both sides. Now the action menu will be hidden when a cell is partially visible on either the left or right side of the scrollable container.

Let me know if you notice any other edge cases that need to be addressed!

Thanks for looking into it.

Ideally we'd only hide the action button on left overflow if the actual button is overflowing. Currently it sounds like the button won't get displayed if even the slightest bit of cell is overflowing from the left which wouldn't be ideal since the action button is placed on the right.

[kirandash]

Something I hadn't considered when I created this issue is that the action button can also overflow on the left hand side when a small portion of the cell is still visible on the left.

Thanks for catching this! I've updated the checkTableCellOverflow function to handle overflow on both sides. Now the action menu will be hidden when a cell is partially visible on either the left or right side of the scrollable container.
Let me know if you notice any other edge cases that need to be addressed!

Thanks for looking into it.

Ideally we'd only hide the action button on left overflow if the actual button is overflowing. Currently it sounds like the button won't get displayed if even the slightest bit of cell is overflowing from the left which wouldn't be ideal since the action button is placed on the right.

Thanks for the feedback! I've updated the logic to be more precise about when to hide the action button. Now it only hides when the button itself would be inaccessible, rather than when any part of the cell overflows. The button's position (5px from right edge), width (20px), table cell padding (8px) are taken into account to ensure it remains visible whenever there's enough space for it to be fully displayed.

When there is enough space:

When there is no more space:

[etrepum]

This looks great! I think eventually we will be able to come up with a better solution for this sort of thing (that puts this sort of UI inside the document DOM) with some combination of getDOMSlot and/or setDOMUnmanaged but those are not really public APIs yet.