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

Add arrow key and tab/enter table editor navigation #1965

Merged

Conversation

deundrewilliams
Copy link
Member

@deundrewilliams deundrewilliams commented Jan 31, 2022

  • When text is selected, use arrow keys and Tab/Enter to move between cells
  • If no text is selected (still editing cell content), use Tab for normal tab webpage navigation
  • To deselect text, use Command/Ctrl + left or right arrow to move the cursor to the beginning or end of the cell's content
  • When pressing the up arrow on the top row, the cursor will navigate to the node above the table, it will also navigate to the node below the table when pressing the down arrow on the bottom row
  • Use Tab/Enter or the arrow keys to open, navigate, and close the cell dropdown menu
  • Use Shift+Tab to move back cell positions, move up the dropdown menu, or move back into the cell from the dropdown menu
  • When pressing the up arrow from any node in Firefox, the cursor will now go to the node directly above it, instead of to the top of the page

fixes #726

Copy link
Contributor

@SJJacques SJJacques left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works as described, but conventionally Shift + Tab allows for backwards navigation and the combo still moves to the next cell.

@deundrewilliams
Copy link
Member Author

Good point, I'll try to add that functionality

@maufcost
Copy link
Contributor

maufcost commented Feb 1, 2022

This is working pretty good to me! Amazing work @deundrewilliams. The only concern I have is with the Tab/Enter command. Is it supposed to navigate the cursor from cell to cell? Because every time I use the Tab/Enter command, it selects the current cell's table menu. If that's the expected behavior, I will go ahead and approve this :)

@deundrewilliams
Copy link
Member Author

Yes @maufcost , at first I had Tab navigating the cursor from cell to cell, but this eliminated the normal Tab order of navigating from cell content -> cell menu -> table options (toggle header, cell width) -> cell content. So using Tab to go right should work only if text is selected.

@SJJacques
Copy link
Contributor

When using tab navigation to go right, the rightmost cell takes you to the next row. The left navigation works but a shift+tab in the leftmost cell doesn't take you to the previous row, otherwise everything looks great to me.

@deundrewilliams deundrewilliams changed the title Add arrow key and tab/enter table editor navigation Draft: Add arrow key and tab/enter table editor navigation Feb 1, 2022
@deundrewilliams deundrewilliams marked this pull request as draft February 1, 2022 21:46
@deundrewilliams deundrewilliams marked this pull request as ready for review February 3, 2022 16:36
@deundrewilliams deundrewilliams changed the title Draft: Add arrow key and tab/enter table editor navigation Add arrow key and tab/enter table editor navigation Feb 3, 2022
Copy link
Contributor

@SJJacques SJJacques left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When text is highlighted and direction is 'left' in the leftmost column it moves up a row rather than to the final column of the previous row. I dropped console.log("totalCols:", totalCols) below line 48 and it returns 1 in that case for some reason. I think it might not be updating after the initial column is made.

@deundrewilliams
Copy link
Member Author

Ok, everything should be working now. The totalCols reference was incorrect because it would just refer to the first node on the page instead of wherever the table node was.

SJJacques
SJJacques previously approved these changes Feb 3, 2022
Copy link
Contributor

@SJJacques SJJacques left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

jpeterson976
jpeterson976 previously approved these changes Feb 8, 2022
Copy link
Contributor

@jpeterson976 jpeterson976 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well!

Copy link
Contributor

@FrenjaminBanklin FrenjaminBanklin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on this so far!

A few thoughts:
It's possible to use the arrow keys to enter the table, but not to leave the table. If the cursor is currently in any of the top row/header cells and the Up key is pressed, it should move up in the document. Likewise if the cursor is currently in any of the bottom cells and the Down key is pressed, it should move down in the document.

Things get a little weird when tabbing is involved:

  • Safari seems to ignore the tab order entirely where cell/table controls are concerned, but that's probably outside the scope of this issue.
  • Firefox and Chrome seem to have the same issues with regards to tabbing:
    • The 'Switch to auto width cells' button doesn't seem to be reachable at all. Repeated presses of the Tab key will move between the cell controls, the 'Toggle Header' button, and then back to the cell text. I think this was already happening so it might not be within the scope of this branch to fix.
    • When the cell controls are active/visible they can be navigated with Tab/Shift-Tab easily, but the arrow keys seem to cause weird/possibly unpredictable behavior. Not only will the Up/Down keys scroll the page, but they will also change which cell control option is highlighted. However, this doesn't change which option is actually active, only Tab/Shift-Tab do. Also, focusing on a cell control option with Tab/Shift-Tab does not highlight that option as it would when hovering over the option with the mouse. This also doesn't seem to happen consistently. To illustrate all of this:
      2022-02-08 11 14 22
  • When cell text is highlighted, Tab/Shift-Tab can be used to navigate cells in addition to the arrow keys. While this is helpful in a certain context, it also means that it is no longer possible to reach the cell control options until the cell text is no longer highlighted. However, there isn't a quick way of doing this short of clicking off the text or overwriting it. I can see two approaches here:
  1. Do not automatically select text when using the keyboard to navigate the table, and/or automatically deselect text when using the keyboard to navigate the table. This could potentially make it difficult to navigate the table using Tab/Shift-Tab.
  2. Make it possible to deselect text when pressing the Escape (or some other?) key. This will let everything continue to behave as it currently does.

Also not really within the scope of this issue, but I noticed while experimenting: The header row doesn't appear to have borders between cells in Firefox.

@FrenjaminBanklin FrenjaminBanklin changed the base branch from dev/27-pyrite to dev/28-jadeite February 8, 2022 18:36
@FrenjaminBanklin FrenjaminBanklin dismissed stale reviews from jpeterson976 and SJJacques February 8, 2022 18:36

The base branch was changed.

@deundrewilliams
Copy link
Member Author

Updates:

  • The 'Switch to auto width cells' button is now reachable by Tab, it comes after the 'Toggle headers' button
  • Arrow keys (up and down) can be used to navigate the cell controls along with Tab/Shift+Tab
  • When navigating the cell controls with the keyboard, the currently selected control will be highlighted, just as it does with a mouseover
  • When cell text is highlighted, you can deselect by pressing Cmd (on Mac, Ctrl on PC) and either the left or right arrow key. The left key will take the cursor to the beginning of the word, and the right will take it to the end. With this, the cell control menu can be accessed afterwards with the Tab key

For the header issue with Firefox, it seems like this has been a long standing bug that the developers haven't fixed yet. The various suggested CSS rules to add didn't seem to work either.

Copy link
Contributor

@FrenjaminBanklin FrenjaminBanklin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safari is still a bit weird about the Tab key - it doesn't seem like table cell controls or the 'Toggle Header' and cell width buttons are reachable in the tab order at all. If this is somehow just not possible to fix, we can create another issue and maybe look into addressing it in the future.

Firefox and Chrome seem to be just about there - the only other thing I can see is making it possible to navigate out of the table with the arrow keys: pressing the 'Up' key while in the top row should move the cursor further up into the content, pressing the 'Down' key while in the bottom row should move the cursor further down into the content.

@deundrewilliams
Copy link
Member Author

I got the navigation out of the table with the down/up arrow keys working.

In Safari, it seems to only respect the tab order when clicking on the 'Toggle Header' button first and then tabbing through from there - starting from anything else doesn't work as expected. There are other issues though:

  • The input field cursor continues to blink during the tabbing, regardless of if the input field is supposed to be focused. When starting to type, it places one character at the end of the text, and then the rest in reverse order.
  • An option is outlined in the cell control dropdown for seemingly no reason.

All of this is shown below, and also happens on the dev branch so I don't think it's directly related to the changes I made. I'm not sure why all of this works fine in Chrome and Firefox but breaks in Safari though, so I'll keep looking into it

recording

@deundrewilliams deundrewilliams linked an issue Feb 28, 2022 that may be closed by this pull request
@deundrewilliams
Copy link
Member Author

The main issue seems to be the fact that Safari still allows user input while also focusing other elements like the dropdown menu or the two cell control buttons at the bottom. This is what's causing most of the other unexpected behavior, so the goal is to find someway to disable input while another option is focused, which is what Chrome and Firefox do by default. I haven't been able to figure out any solution to this short of setting the entire editor's editable property to false when Tab is pressed inside a cell, which doesn't fully work since there doesn't seem to be any reliable way to know when to set it back to true. Because of this, I think it could be best to move all of that to a different Safari-focused issue.

@FrenjaminBanklin
Copy link
Contributor

I concur, go for it. Will do another pass, assuming the Safari-only weirdery is the only unsolved mystery I'm thinking this will be good to go.

Copy link
Contributor

@FrenjaminBanklin FrenjaminBanklin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignoring how Safari plays havoc with tab order, I noticed two more pretty minor things.

The first, possibly more readily solvable - it looks like using the keyboard to enter the table in Firefox is weird when moving upwards in the document. For example, if there's a table node preceded by a header node and followed by a text node, and pressing the Up key while the cursor is in the text node - the expected behavior is that the cursor should move into the bottom row of the table, but for some reason in Firefox (not Chrome or Safari) it'll skip the table entirely and move directly into the header node.

The second thing (which is honestly more like a nit pick) is that the cursor doesn't seem to be showing up when shift-tabbing out of the cell options menu to return to a cell in the table. It seems like it moves the cursor back into the cell, so typing will add to the cell's contents, but the cursor doesn't start blinking so it's not clear that you can do so.

@deundrewilliams
Copy link
Member Author

I'm still working on tests for the first issue you mentioned plus some other bugs I'm finding, but I can't figure out how to reproduce the second bug you mentioned. I'm shift tabbing out of the menu and the cursor appears. This recording is from Firefox but it does the same in Chrome. Is this what you meant @FrenjaminBanklin, or something else?

shift-tab

@FrenjaminBanklin
Copy link
Contributor

After reviewing the video I took to demonstrate that issue, it occurs to me that it might have been due to my mouse cursor sitting on top of the buttons as I was trying to use them, which was probably triggering the onHover behavior hence the weird highlight mismatch. Now that you've got the items in that list highlighting on focus, I think everything should be fine.

@FrenjaminBanklin
Copy link
Contributor

Then I realized you were talking about something else entirely.

I guess it's actually more specific than how I originally described it. It seems like it only occurs after adding or removing a row. For example, notice how the cursor is not visible after adding a row then shift-tabbing back into the cell to type:
2022-03-03 14 28 32

@deundrewilliams deundrewilliams dismissed FrenjaminBanklin’s stale review March 21, 2022 12:25

Added fix for up arrow from nodes in Firefox

@deundrewilliams deundrewilliams force-pushed the issue/726-table-nav branch 2 times, most recently from f5ce0e2 to 57ebde4 Compare August 22, 2022 15:46
@FrenjaminBanklin FrenjaminBanklin changed the base branch from dev/28-jadeite to dev/29-sodalite August 26, 2022 20:15
@FrenjaminBanklin FrenjaminBanklin changed the base branch from dev/29-sodalite to dev/30-howlite January 10, 2023 19:32
Copy link
Contributor

@FrenjaminBanklin FrenjaminBanklin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the exception of Safari's tab order being generally uncooperative, this looks like it's basically ready to go.

Additional review to see if there's any other unwanted behavior that I may have missed would be nice.

@RachelDau RachelDau self-requested a review January 11, 2023 13:53
Copy link
Contributor

@RachelDau RachelDau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this out and it looks like everything works on firefox and chrome so I approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tab and Enter defaults for table
6 participants