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 accessibility issues #606

Closed
jchen527 opened this issue Mar 4, 2017 · 5 comments
Closed

Table accessibility issues #606

jchen527 opened this issue Mar 4, 2017 · 5 comments

Comments

@jchen527
Copy link
Contributor

jchen527 commented Mar 4, 2017

There are a few issues with the aria roles on the current table implementation:

  • role="grid" should be an attribute on the table element instead of the inner grid
  • all table rows (including the header row) should have role="row" even if they don't have event handlers like onRowClick

See aria documentation for more details. Elements with role="grid" require descendants with role="row", and elements with role="rowheader" require an ancestor with role="row" which requires an ancestor with role="grid".

@bvaughn
Copy link
Owner

bvaughn commented Mar 4, 2017 via email

jchen527 added a commit to jchen527/react-virtualized that referenced this issue Mar 4, 2017
Move role="grid" from the inner grid to the table and add
role="row" to all table rows. Fixes bvaughn#606.
@bvaughn
Copy link
Owner

bvaughn commented Mar 4, 2017

Elements with role="grid" require descendants with role="row"

I think you mean to say that elements with role="row" require an ancestor with role="grid" ? Grid is, by default, a role="grid" but it does not contain role="row" children (because there is no wrapper "row" element in multi-column Grid).

I also wonder if it would be appropriate to set role for a Grid inside of a Table to "rowgroup" (instead of null).

@bvaughn
Copy link
Owner

bvaughn commented Mar 4, 2017

Thanks for taking the time to educate me on the incorrect use of roles in this case. I really appreciate the PR! I just merged it, with the small addition of adding role="rowgroup" to the Grid inside of a Table. 😄

@jchen527
Copy link
Contributor Author

jchen527 commented Mar 5, 2017

Re: your comment about Grid having no role="row" children, I believe elements with role="grid" do require descendants with role="row" or role="rowgroup" because they are "Required Owned Elements". I've been using the Accessibility Developer Tools chrome extension and the audit complains when there is only an element with role="grid".

Thanks for the quick review and role="rowgroup" change!

@thiemeljiri
Copy link

Is there a way how to not add these attributes? You have it wrong and even if that was implemented correctly it's not always what you want, which is our case.

Role grid must be sued only when it's a data grid with implemented keyboard navigation. Using such roles without implementing the rest hurts the accessibility.

I strongly suggest removal of the role grid and I would like to know the motivation behind adding it. This is how to use it properly and believe me, that's not what you wan't in many cases.
https://www.w3.org/TR/wai-aria-practices/examples/grid/dataGrids.html
https://www.w3.org/TR/wai-aria-practices/examples/grid/dataGrids.html

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

No branches or pull requests

3 participants