Fixes grid selection to be aware of nested tables

[icrosil]

Why

As pointed in #4647 Table selection behaved with issue on moving selection between nested tables. What happened is that we expect nodes anchor/focus (from/to) to be in the same grid by default, but it isn't true for nested tables.

What

Here after finding focus and anchor cells we check if grids for both of them are equal - that would mean we are on the same level, if not we are moving one selection up. This change makes selection behaviour as next:

• If we move focus from higher level to lower - same higher cell to be selected.
• If we move focus from nested level to level above (any cell) that would mean we make whole original cell to be focused.
• If we move focus from higher to same level including some of nested tables - they will be skipped to focus higher level cells.

Before:

lexical.selection.before.mov

After:

lexical.selection.after.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	Nov 15, 2023 4:10pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 15, 2023 4:10pm

[github-actions]

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
packages/lexical/dist/Lexical.js	37.22 KB (+0.21% 🔺)	745 ms (+0.21% 🔺)	125 ms (+65.83% 🔺)	869 ms
packages/lexical-rich-text/dist/LexicalRichText.js	40.04 KB (+0.19% 🔺)	801 ms (+0.19% 🔺)	108 ms (+1.73% 🔺)	909 ms
packages/lexical-plain-text/dist/LexicalPlainText.js	40.01 KB (+0.2% 🔺)	801 ms (+0.2% 🔺)	118 ms (-43.02% 🔽)	918 ms

[ivailop7]

A bit on the nit picking side, but if I do 3-level table in table nesting, selection still breaks.

[icrosil]

A bit on the nit picking side, but if I do 3-level table in table nesting, selection still breaks.

yeah, wasn't sure if that's something lexical want to support or not, let me see if could generalize it to cover it.

[icrosil]

A bit on the nit picking side, but if I do 3-level table in table nesting, selection still breaks.

Actually just tested and works as expected, what it does it basically tries to find common table between 2 cells:
if cells are not on the same level, it moves anchor / focus to the level up, and repeats recursively until matched so it covers any level of nested tables.

lexical.3.tables.in.mov

What it doesn't do - it doesn't handle focus of cells from different tables, so if you have 2 tables next to each other and try to focus beyond single table that would still crash. I wouldn't try to fix it here since it would need more of a product decision, and generally would be more problem of general selection, not table selection.

What I'd propose is to add similar "level matching" selection implementation as I've added inside table but for all page to make sure that we are selecting text / paragrpahs / tables as elements, but that would be a product decision maybe closer to Notion selection rather than current Lexical if I'm getting this right.

[ivailop7]

Great, however, when I use the / menu and select '5x5 table' and then in any cell I do it again '/' and then '5x5 table' it crashes the selection directly even when using 2 tables nested within each other.

[icrosil]

Great, however, when I use the / menu and select '5x5 table' and then in any cell I do it again '/' and then '5x5 table' it crashes the selection directly even when using 2 tables nested within each other.

I cannot reproduce it locally on this branch, could you please share video steps?

[ivailop7]

broken_tables.mp4

So I managed to replicate the issue, it appears when I use the keyboard arrows to navigate, it happens 50% of the time. If I do point and click, it works fine, but if I navigate with the arrows upon inserting the second table, selection freezes.

[icrosil]

broken_tables.mp4
So I managed to replicate the issue, it appears when I use the keyboard arrows to navigate, it happens 50% of the time. If I do point and click, it works fine, but if I navigate with the arrows upon inserting the second table, selection freezes.

Thanks for the details, was able to replicate now, it seems that selection and grid actions are located in multiple places and not the same for mouse and keyboard events, I'm putting down all possible use cases with keyboard and going to update PR with fixes for all edge cases I could find there.

[ivailop7]

Hey @icrosil , is this good for review? I see it working well in the playground preview, but the tests you've added fail often, so unsure if you are planning on more change for this PR.

[icrosil]

Hey @icrosil , is this good for review? I see it working well in the playground preview, but the tests you've added fail often, so unsure if you are planning on more change for this PR.

Hey I wanted to fix tests first, to make sure I'm not breaking anything, but if that would take too long I'd probably slim them down as functionality now is in better state.

I wouldn't claim that it fixed all possible scenarios because of the fragmented selection control I think that would be extremely difficult, but at least we fixed highlighted use cases. Let me try update fixes first, will ping you here on thread when ready for a review. Thanks!

[icrosil]

Hey @icrosil , is this good for review? I see it working well in the playground preview, but the tests you've added fail often, so unsure if you are planning on more change for this PR.

I've decided to remove flaky tests from diff to see if CI will pass on this, doing more manual checks locally please feel free to review code, especially if CI is green :)

[ivailop7]

LGTM