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

Table keyboard interaction #1137

Closed
9 tasks done
jattasNI opened this issue Mar 27, 2023 · 2 comments · Fixed by #2172
Closed
9 tasks done

Table keyboard interaction #1137

jattasNI opened this issue Mar 27, 2023 · 2 comments · Fixed by #2172
Assignees
Labels
enhancement New feature or request

Comments

@jattasNI
Copy link
Contributor

jattasNI commented Mar 27, 2023

😯 Problem to Solve

We want users to be able to perform common table interactions via keyboard as well as via mouse.

💁 Proposed Solution

Earlier resources: #877 ; interaction spec - currently has password error (UXDesign1).

📋 Tasks

  • Demo prototype visuals to Brandon / get design feedback
  • Header/row/cell navigation via Up/Down/Left/RightArrow keys
  • Navigation within current row / header row with Tab/Shift-Tab
  • Return to navigation mode with Esc after action menu / cell contents are focused
  • Ensure keyboard navigation behaves sanely with mouse / other table interactions
  • Finalize impact to table / cell view APIs, if any
  • Get/address feedback on implementation via draft PR
  • Additional keys (Home/End/PgUp/PgDn/CtrlEnter/F2/etc)
  • Additional work identified in HLD
@jattasNI jattasNI added enhancement New feature or request triage New issue that needs to be reviewed labels Mar 27, 2023
@jattasNI jattasNI added this to the Table Milestone 4 milestone Mar 27, 2023
@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Mar 28, 2023
@atmgrifter00 atmgrifter00 self-assigned this Jul 25, 2023
@atmgrifter00 atmgrifter00 moved this from Backlog to In progress in Nimble Design System Priorities Aug 28, 2023
@m-akinc m-akinc moved this from In progress to Defined/Ready to Pickup in Nimble Design System Priorities Sep 26, 2023
@atmgrifter00
Copy link
Contributor

As part of the work in this PR, we are hiding the expand-collapse button for both group rows and parent rows from the accessibility tree. For parent rows this is meant to be a temporary state until we provide keyboard interactions for the table, at which point the expand-collapse button for parent rows should be focusable (but not for group rows).

It's also worth noting that by hiding the expand-collapse button the accessibility score actually drops, seemingly because it expects the wrapped button element to have its tabindex set to -1, the same as the FAST component, but tabindex is not passed through in the template (while aria-hidden is).

@m-akinc m-akinc moved this from Defined/Ready to Pickup to Backlog in Nimble Design System Priorities Jan 9, 2024
@jattasNI jattasNI mentioned this issue Jan 10, 2024
1 task
@m-akinc m-akinc moved this from Backlog to Current Iteration in Nimble Design System Priorities Feb 12, 2024
@m-akinc
Copy link
Contributor

m-akinc commented Feb 12, 2024

HLD is in PR, but needs some attention to keep moving forward.

@msmithNI msmithNI self-assigned this Mar 19, 2024
@msmithNI msmithNI moved this from Current Iteration to In progress in Nimble Design System Priorities Mar 25, 2024
msmithNI added a commit that referenced this issue Apr 9, 2024
# Pull Request

## 🤨 Rationale

HLD for #1137 

## 👩‍💻 Implementation

Started with the HLD document from #1506 , and updated it to reflect our
decision to standardize on the ARIA treegrid role (moved the info about
the grid role/ previous IxD design to the Alternatives section), and
added an explicit section with the specific interactions as they'll
apply to the Nimble table.

Note: Currently there isn't a prototype that fully reflects the
interactions detailed in this document (along with the latest table
component), but one is in progress.
msmithNI added a commit that referenced this issue Jun 25, 2024
# Pull Request

## 🤨 Rationale

Resolves #1137 

## 👩‍💻 Implementation

See #2173 for the most up-to-date expected behavior of keyboard
navigation that this PR implements. ([Direct link to updated
HLD](https://github.com/ni/nimble/blob/96e1725fc7cc8b3fdf5f5197b090e8f95f86cd5f/packages/nimble-components/src/table/specs/table-keyboard-navigation-hld.md))

The `KeyboardNavigationManager` class does the bulk of the work:
- Tracks what is currently focused / should be focused (via properties
`focusType`, and `row/column/cellContent/headerActionIndex`)
- Note: If the user clicks in the table away from the current keyboard
focus, the focus state would end up out-of-date. There's code in
`handleFocus()` to handle this (see additional comments there)
- Listens for table keypresses to handle arrow key navigation,
Tab/Shift-Tab, PgUp/PgDn/etc key presses
- If the user unfocuses and refocuses the table, re-focuses the
appropriate elements in the table (`handleFocus()` again)
- Tab/Shift-Tab behavior should match the HLD / expectations for ARIA
treegrid. (Basically, Tab/Shift-Tab go through the tabbable elements of
the header row / current row, not including cells/column headers, until
the end is reached, then the table blurs and elements before/after it
will be focused with additional Tab presses.)

The overall approach of only setting `tabindex=0` on the single element
in the table we want focused is called [roving
tabindex](https://www.freecodecamp.org/news/html-roving-tabindex-attribute-explained-with-examples/).

To ensure that we control what elements in the table should get focused,
this PR updates `tabindex` on many elements in the table:
- The table itself is `tabindex=0` to ensure that it can be focused via
`Tab`
- Elements that are focusable by default such as buttons/anchors, are
explicitly set to `tabindex=-1` to start with, until the keyboard
navigation code wants to focus them (then they're set to `tabindex=0`
just before being focused).

## 🧪 Testing

- Manual testing (mostly Chrome, some Firefox).
- Earlier builds had some Safari testing, but we should explicitly
re-test there before submission.
- Added autotests

Keyboard navigation can be used on any/all of the [existing table
Storybook
pages](https://60e89457a987cf003efc0a5b-gjoayfsquj.chromatic.com/?path=/story/components-table--table),
once the table is focused.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
- Added keyboard nav docs to [table Storybook docs
page](https://60e89457a987cf003efc0a5b-gjoayfsquj.chromatic.com/?path=/docs/components-table--docs)
(in Accessibility section)
@m-akinc m-akinc moved this from In progress to Done in Nimble Design System Priorities Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

4 participants