lexical icon indicating copy to clipboard operation
lexical copied to clipboard

fix: removed window.pageXOffset and window.pageYOffset from TableActionMenu position calculation

Open 58bits opened this issue 1 year ago • 3 comments
trafficstars

Apologies for the YOLO PR, however, based on a recent discord discussion here https://discord.com/channels/953974421008293909/953974421486436393/1219603905198161920 - it appears we are not the only ones having a problem with the TableActionMenu plugin dropdown position when there are multiple editors on the page, or when the editor position requires scrolling down (i.e. the editor appears lower down on the page or 'below the fold'). In these cases, the dropdown menu does not appear over the cell that requested it (it may not even appear in the browser viewport).

This PR contains the fix we applied (and re-apply if we ever take an update from the Playground).

It works for us without any problem on all editor instances, irrespective of how many editors there are or whether you have to 'scroll down' to use the editor. See the screenshot below.

However, I have NOT tested this in the Playground demo site yet, and there may be an 'adjustment' like 'position: relative' required in the editor container there (honestly not sure).

See the comments in the commit as well.

Hope at least that this is of some help.

image

58bits avatar Mar 21 '24 22:03 58bits

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 21, 2024 10:12pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 21, 2024 10:12pm

vercel[bot] avatar Mar 21 '24 22:03 vercel[bot]

I've just seen this... https://github.com/facebook/lexical/pull/5614 - although it refers to the same issue, I'm not clear on whether that commit was meant to resolve it or was reverted.

58bits avatar Mar 25 '24 22:03 58bits

hi, not a reviewer, but just wanted to give my thoughts since I'm running into this issue also. I think previously the menu "dropdown" css class used position: absolute instead of position: fixed: https://github.com/facebook/lexical/pull/2297

And TableActionMenu was written with position: absolute in mind: https://github.com/facebook/lexical/pull/957 hence the usage of window.pageXOffset and window.pageYOffset. Your solution works well for fixed position, but in my use case I need table menu to be absolutely positioned, and this is where removing window.pageYOffset doesn't work anymore (if you scroll down and create a table, the menu positions will all be completely off).

Hoping that we can have a solution that also accommodates absolute positioning, thanks!

colin-jiang avatar Apr 22 '24 02:04 colin-jiang