useSortBy should toggle`aria-sort` on column headers and relevant demos should encourage a11y best practices

[nicklemmon]

Describe the bug (required)

1. useSortBy should leverage the aria-sort attribute which describes the current sort direction for a particular column to screen reader only users
2. The demo for useSortBy is not keyboard accessible - fortunately this can be overcome easily via manually leveraging column.toggleSortBy! Since this library is fairly popular, it would be good to encourage best practices where possible.

Provide an example via Codesandbox! (required)
Codesandbox Demo <-- here is a demo demonstrating the lack of aria-sort on the <th> elements with a keyboard accessible implementation using column.toggleSortBy

Steps To Reproduce (required)

1. Go to the linked demo in the documentation for useSortBy
2. Try to control the sorting using a keyboard only - unfortunately you can't due to some HTML semantics problems
3. When clicking on a <th> to sort, inspect the DOM for the aria-sort attribute - it isn't set at all.

Expected behavior (Recommended)
Here is a great demo of how this should work from a reliable source:
https://dequeuniversity.com/library/aria/table-sortable

1. All sortable <th> elements have a nested <button> inside to make them keyboard accessible
2. All sortable <th> elements have a valid aria-sort attribute (but only if they are sortable!)
3. Note they do not have the title attribute that is currently provided by useSortBy - this shouldn't be necessary with the aria-sort attribute in place.

Screenshots
N/A

Desktop (please complete the following information):

• OS: macOS Big Sur
• Browser: All browsers
• Version: 7.6.3

Smartphone (please complete the following information):

N/A

Additional context
Seriously, thanks to all of the maintainers for your work. The headless approach this library takes serves the React community well.

Depending on the complexity of the code, I could potentially jump in and help out with this one. The aria-sort problem in particular seems like low-hanging fruit. Updating a CodeSandbox demo should be quite easy as well, though I'm not sure of the best way to go about doing that in this case.

[hakubo]

In addition to that I have noticed that it sets title on header cell to "Toggle SortBy". I think name of the column should be preserved here like: 'Toggle SortBy, column: ${name}"

[nicklemmon]

In addition to that I have noticed that it sets title on header cell to "Toggle SortBy". I think name of the column should be preserved here like: 'Toggle SortBy, column: ${name}"

Right - definitely not the right content. I think when paired with aria-sort, the regular heading content should do just fine for screen reader users.

[github-actions]

This issue is being marked as stale (no activity in the last 14 days)

[github-actions]

This issue has been detected as stale and automatically closed. It's very likely that your issue has remained here this long because it would require breaking changes to v7. React Table v8 is currently in alpha (soon beta!) and already contains bug fixes, performance improvements and architectural changes that likely address this issue.

• If your v7 issue has been previously labeled as requiring breaking changes, please try the new v8 alpha/beta releases
• If your v7 issue has not already been labeled as a breaking change, open a new issue.
• If this was a v8 issue and was closed by mistake, please reopen or leave a comment below.

[esetnik]

@tannerlinsley I believe this should be reopened as it's still an issue with react-table v8. It makes it hard to target sortable headers in cypress tests in an accessible way.

[tannerlinsley]

It’s headless, so you should be able to put your own area attribute in your market depending on the table state

[esetnik]

For anyone else that comes across this. I made it work using the following:

                <TableHeaderColumn
                  {...column.getHeaderProps(column.getSortByToggleProps())}
                  aria-sort={
                    column.isSorted
                      ? column.isSortedDesc
                        ? 'descending'
                        : 'ascending'
                      : 'none'
                  }
                >
                  <TableHeaderCell role="button">
                    {column.render('Header')}
                    {column.canSort && (
                      <SortIcon
                        size={IconSize.extraSmall}
                        direction={
                          column.isSorted
                            ? column.isSortedDesc
                              ? Direction.down
                              : Direction.up
                            : null
                        }
                      />
                    )}
                  </TableHeaderCell>
                </TableHeaderColumn>

[nicklemmon]

Nice, @esetnik!

A couple of things to be sure to address too (your component might do this under the hood):

• Make sure the rendered th does not have role="button" - that would wipe out the semantic meaning of the th and make aria-sort meaningless
• The rendered markup should render like so at the end of the day:

<th aria-sort="descending">
  <button>Table header content here</button>
</th>

Check out this example for more:
https://dequeuniversity.com/library/aria/table-sortable

[nicklemmon]

And @tannerlinsley in response to your comment here:

It’s headless, so you should be able to put your own area attribute in your market depending on the table state

It is headless though it does help render relevant HTML attributes for some features of the table but not others. col-span and row-span, for example:
https://tanstack.com/table/v8/docs/api/core/header#colspan

I suppose it depends on what "headless" means exactly! For some headless libraries (React ARIA, Downshift, Radix) they are all about adding the right HTML attributes for handling state changes (including ARIA). Is this library only concerned with data? If it's concerned at all with producing the right HTML to reflect table state, then ARIA attributes should be part of the mix I would think.

(And thanks for all your hard work on this and so many other libraries 🙏 - big fan of your work)

[esetnik]

Nice, @esetnik!

A couple of things to be sure to address too (your component might do this under the hood):

• Make sure the rendered th does not have role="button" - that would wipe out the semantic meaning of the th and make aria-sort meaningless
• The rendered markup should render like so at the end of the day:

<th aria-sort="descending">
  <button>Table header content here</button>
</th>

Check out this example for more: https://dequeuniversity.com/library/aria/table-sortable

I believe that my implementation is now correct. Thank you for your help @nicklemmon

<th colspan="1" role="columnheader" title="Toggle SortBy" aria-sort="ascending" class="Table__TableHeaderColumn-sc-13lnlsj-5 jtfJwa" style="cursor: pointer;"><div role="button" aria-pressed="true" class="Table__TableHeaderCell-sc-13lnlsj-0 ejdZOg">Class Name<div size="1" direction="0" color="black" class="Icons__BaseIcon-sc-11pp0d5-0 Icons__Arrow-sc-11pp0d5-54 Icons__SortIcon-sc-11pp0d5-56 hQXkhp ilQtJx kiMnuM"></div></div></th>

[esetnik]

I suppose it depends on what "headless" means exactly! For some headless libraries (React ARIA, Downshift, Radix) they are all about adding the right HTML attributes for handling state changes (including ARIA). Is this library only concerned with data? If it's concerned at all with producing the right HTML to reflect table state, then ARIA attributes should be part of the mix I would think.

I agree. And most devs don't know too much about aria and often don't have the business justification to bother supporting it. In my opinion it should be baked in whenever possible. If adding too many properties by default was a concern there could be some configuration one could set to decide whether or not to generate aria attributes.

[tannerlinsley]

I’m happy to accept any PRs for better aria attrs!