Skip to content
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: Selection of table cell is accompanied by selection of adjacent cell too #6973

Open
jvithlani opened this issue Dec 17, 2024 · 9 comments · May be fixed by #7005
Open

Bug: Selection of table cell is accompanied by selection of adjacent cell too #6973

jvithlani opened this issue Dec 17, 2024 · 9 comments · May be fixed by #7005
Labels
good first issue Good for newcomers selection tables Relates to Lexical Tables

Comments

@jvithlani
Copy link

jvithlani commented Dec 17, 2024

Currently in the table if we triple click a cell, it will select 2 cells which is a very weird behaviour. If the cell in the last column which is being triple clicked then 2 rows will be selected

Screen.Recording.2024-12-18.at.2.08.37.AM.mov

Lexical version: 0.21.0

Steps To Reproduce

  1. Insert a table of any size
  2. Triple click any cell

The current behavior

Random selection of table cells on triple click

The expected behavior

Some consistent behaviour which behaves same for all table cells

Impact of fix

The ux around table will be great and consistent

@etrepum etrepum added tables Relates to Lexical Tables selection good first issue Good for newcomers labels Dec 17, 2024
@etrepum
Copy link
Collaborator

etrepum commented Dec 17, 2024

There's a workaround for this with RangeSelection (#4512) but the TableObserver code needs to handle it separately. Basically it's a matter of checking the detail property in the event and handling that selection update manually.

@jvithlani
Copy link
Author

@etrepum when i try the playground link in (#4512) its fixed, but on the current playground its still broken, how do i get to use these changes for my editor?

@etrepum
Copy link
Collaborator

etrepum commented Dec 18, 2024

The table code back then worked differently, someone has to create a PR with a new fix for this issue.

@Parasaran-Python
Copy link
Contributor

Parasaran-Python commented Dec 27, 2024

Hello @etrepum

I see that the logic to select the table cells is this,

image

upon triple clicking a cell we select all the nodes between that cell (anchor node) and its just next cell (focus node),
if a cell is the last in a row the path to its just next cell (which is the first cell in the next row) goes through the cell just below it (the logic is this way most likely to handle the click and drag action accurately), hence all the cells in the next row get selected.

Let me know if you have any questions with this.

the cell on which the triple click happened is our anchor node,
the immediate next cell is our focus node.

Approach: If we could just prevent the focus from going to the immediate next cell on triple click we can very likely fix the issue.

My questions is: Are we by any chance shifting the focus to the immediate next cell through some code?

@etrepum
Copy link
Collaborator

etrepum commented Dec 27, 2024

All of the nodes between the anchor and focus of a range selection are selected, just by the definition of what a range selection is. The code you're pointing at is not relevant here.

@Parasaran-Python
Copy link
Contributor

All of the nodes between the anchor and focus of a range selection are selected, just by the definition of what a range selection is. The code you're pointing at is not relevant here.

The above code gets all the nodes between the anchor and focus and adds it to _cachedNodes which is eventually used by another code in a loop to select nodes.

On triple clicking the anchor is our current cell and focus is the next cell, and the range selection selects all the nodes in between.

This is my understanding till now.

Please correct me if you see a gap.

Coming to the expected behavior,

On triple click we expect only the current cell to get selected right? Please correct me if I'm wrong.

@etrepum
Copy link
Collaborator

etrepum commented Dec 27, 2024

Nodes don't "get selected", a selection is created and then set on the editor, nothing happens to the nodes, this code tells you which nodes are in the selection based on the definition of a range selection.

When you call selection methods on a node, it creates a selection with points on that node and sets it on the editor. There is no method to go the other way, nothing happens to the nodes when they enter or leave the editor's selection.

I'm only commenting on your understanding of the code here. You're looking in the wrong place. If you expect a different set of nodes to be in the selection then the selection needs to be created with different points.

@jvithlani
Copy link
Author

@etrepum is there any chance if someone from the team can raise a fix for this?

@etrepum
Copy link
Collaborator

etrepum commented Dec 30, 2024

@jvithlani it's not a priority of mine to work on this but I posted a draft PR that demonstrates the technique that I suggested earlier #7005 - you should be able to add this sort of handler to your own code separate from the table plugin as this is purely additive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers selection tables Relates to Lexical Tables
Projects
None yet
3 participants