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

[DataGrid] The checkbox shouldn't be tabbable #1430

Closed
2 tasks done
oliviertassinari opened this issue Apr 17, 2021 · 4 comments
Closed
2 tasks done

[DataGrid] The checkbox shouldn't be tabbable #1430

oliviertassinari opened this issue Apr 17, 2021 · 4 comments
Labels
accessibility a11y bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 17, 2021

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The select all checkbox is in the tab order

Screenshot 2021-04-18 at 01 21 05

Expected Behavior 🤔

The select all checkbox is not in the tab order

Steps to Reproduce 🕹

  1. Open https://material-ui.com/components/data-grid/#mit-version
  2. Use Tab to reach a data grid

Context

This is a follow-up on #1289. We missed a tabIndex={-1} on the checkbox and test cases.

Your Environment 🌎

v4.0.0-alpha.25

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work accessibility a11y component: data grid This is the name of the generic UI component, not the React module! labels Apr 17, 2021
@dtassone
Copy link
Member

Why shouldn't it be in tabbable? It's a control with a button behind it 🤔

@DanailH
Copy link
Member

DanailH commented Apr 19, 2021

We removed the tabIndex from all other checkboxes as it hijacks the keyboard control. basically using the keyboard to navigate to a cell with a checkbox and hitting enter or space should select that checkbox. At least that is how I understood it.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Apr 19, 2021

Why shouldn't it be in tabbable?

@dtassone Because this is described in the WAI-ARIA authoring practices. The grid is a composite widget. They reiterate what this notion means:

A grid is a composite widget so it:

  • Always contains multiple focusable elements.
  • Only one of the focusable elements contained by the grid is included in the page tab sequence (tabbable).
  • Requires the author to provide code that manages focus movement inside it.

https://www.w3.org/TR/wai-aria-practices/#grid

Now, the cell > checkbox case falls under this section:

When the above grid navigation keys move focus, whether the focus is set on an element inside the cell or the grid cell depends on cell content. See Whether to Focus on a Cell or an Element Inside It.

In theory, we should have the checkbox tabbable and focused, not the cell. This is because a checkbox doesn't have any key handler that conflicts with the grid arrow navigation. In practice, I'm not sure it's manageable.

We also don't remove the tab index from all the cells developers are providing. IMHO, we should take shortcuts and document how developers can implement it correctly, using our API, but not have the data grid bring an implementation fully compliant by default. For instance, in a cell, the developer could listen to the change focus event, and do the heavy lifting (move focus, remote tabIndex on their side). We could start with this column:

Screenshot 2021-04-20 at 00 48 12

@joserodolfofreitas
Copy link
Member

The expected behavior is already implemented on the latest version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

4 participants