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.

Sign up for GitHub

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

Jump to bottom

[EuiDataGrid] Fix/improve screen reader accessibility for column headers #6034

Merged
merged 8 commits into from
Jul 19, 2022
Merged

[EuiDataGrid] Fix/improve screen reader accessibility for column headers #6034

merged 8 commits into from
Jul 19, 2022

Commits on Jul 7, 2022

  1. Improve 'Header actions' button label 

    - Switch to hint text that an action (clicking, aka enter/space key) is necessary

- move to `aria-describedby` over `aria-label` for 2 reasons:

1. to give more pause before announcing the new hint

2. in combo with the `hidden` attribute, this prevents the extra hint text from being announced as part of the column header name when screen reader navigating between cells

- requires adding fragment wrapper (turn off whitespace diffs)
    @cee-chen
    cee-chen committed Jul 7, 2022
    Configuration menu
    Copy the full SHA
    ee6f20f View commit details
    Browse the repository at this point in the history

  2. [Setup] Move all sorting-related logic to a separate util 

    - for easier testing and dev readability purposes

+ perf improvement - DRY out unnecessary new Set() in favor of sortedColumn & a useMemo() around .find()

- update unit tests to use hook helper instead of having to wrap context provider
    @cee-chen
    cee-chen committed Jul 7, 2022
    Configuration menu
    Copy the full SHA
    8dfb15c View commit details
    Browse the repository at this point in the history

  3. Move sorting text to aria-describedby as well 

    - for better pause/screen reader behavior

- for header cells without actions, this can't be in an aria-describedby because VO reads the describedby BEFORE the header, not after :| moving it to a SR only item after the column text appears to work
    @cee-chen
    cee-chen committed Jul 7, 2022
    Configuration menu
    Copy the full SHA
    11ac2b9 View commit details
    Browse the repository at this point in the history

  4. Simplify aria-sort prop 

    - now that we no longer need to set an aria-describedby on the topmost grid parent, this can be just ariaSort instead of ariaProps

- add comment documentation for somewhat unintuitive aria-sort behavior

- simplify to a nested ternary (fight me irl)

- add easier to read boolean flags (which will be used more later)
    @cee-chen
    cee-chen committed Jul 7, 2022
    Configuration menu
    Copy the full SHA
    f913b0e View commit details
    Browse the repository at this point in the history

  5. Improve sorting screen reader text 

    Minor
- Make var name a little more explicit
- useMemo for perf

Major
- Fix header cells w/ actions not reading out `aria-sort` (because focus is on the child button, not on the grid parent) by adding a new branching path
- Fix sort text not being i18n'ed correctly (requires switching to `EuiI18n`, since hooks can't be called in map callbacks or conditionally :T)
- Fix copy to read out 'ascending' and 'descending' fully as words, + add ending period for better screen reader flow
    @cee-chen
    cee-chen committed Jul 7, 2022
    Configuration menu
    Copy the full SHA
    2eb051c View commit details
    Browse the repository at this point in the history

  6. Unit tests 

    - Remove `data_grid.test.tsx` mounted unit tests in favor of more fleshed out `data_grid_header_cell.test.tsx` tests
    @cee-chen
    cee-chen committed Jul 7, 2022
    Configuration menu
    Copy the full SHA
    5e89fa9 View commit details
    Browse the repository at this point in the history

  7. Changelog

    @cee-chen
    cee-chen committed Jul 7, 2022
    Configuration menu
    Copy the full SHA
    43d6ad0 View commit details
    Browse the repository at this point in the history

  8. Fix data_grid.spec.tsx failure 

    - textContent now includes hidden describedby text, using a querySelector to specify the text content we want fixes it

- typescript was complaining about `data`, not sure why
    @cee-chen
    cee-chen committed Jul 7, 2022
    Configuration menu
    Copy the full SHA
    a5d2efc View commit details
    Browse the repository at this point in the history