-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug Fix: Shift+down selects an extra subsequent element for Table selection #6679
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
focusNode, | ||
(n) => $isElementNode(n) && !n.isInline(), | ||
); | ||
if ($isTableCellNode(focusParentNode)) { | ||
focusParentNode = getParentTable(focusParentNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just $findMatchingParent(focusParentNode, $isTableNode), but needing that extra helper method, it will return the table node or null, just like your method.
Cursor should stop inside first/last cell of table not on the paragraph before/after the table
focusOffset: 0, | ||
focusPath: [2], | ||
focusOffset: 1, | ||
focusPath: [1, 2, 1], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Focus will now be inside the last cell of the table, not on the next element after table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still seems possible to get "stuck" if you remove any elements between the tables
For example, using this document with two tables and a paragraph, if you focus the last paragraph and then press shift up it works to select the first table but not the second
the third part takes care of changing selection already. This part also has a bug: the case of selecting table using shift+up (not down) is not handled which makes selection stuck
Addressed your comment in 0a47177 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work great now! Very nice improvement
Description
Only the table should be selected when shift+down. Also fixes scenario when selection is stuck when trying to unselect a table with arrow keys.
Closes #6527 for Tables
Test plan
Before
Paragraph after table is selected
After
Only table is selected
Before
Selection stuck
After
Can unselect tables one by one