[EuiDataGrid] Improve `aria` row counts and indices for screen reader users #6033

cee-chen · 2022-07-06T20:20:50Z

Summary

Please see inline code comments below that highlight changes & include screen reader before/after screencaps.

Checklist

Checked in Chrome, Safari, Edge, and Firefox
Added or updated jest and cypress tests
Checked for accessibility including keyboard-only and screenreader modes
A changelog entry exists and is marked appropriately

- this enables row counts in virtualized grids to be correctly announced (instead of only counting the rendered rows) - NOTE: This works in VO+Safari but not VO+FF

- which requires passing `pagination` down to `EuiDataGridCell` and adding # of paginated rows to the visible row index

- add column header name/ID in parenthesis after column index - fix row index - was not correct on sort, and should now do so - add `-` for extra spacing between cell content and cell data, remove `:` for more natural speech cadence

kibanamachine · 2022-07-06T20:46:44Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6033/

cee-chen · 2022-07-06T20:24:14Z

src/components/datagrid/data_grid.tsx

                  data-test-subj="euiDataGridBody"
                  className="euiDataGrid__content"
                  role="grid"
+                  aria-rowcount={rowCount}


Without aria-rowcount, screen readers are reporting the wrong total rows for virtualized grids (because they're only counting the actual number of DOM nodes present, which is inaccurate due to virtualization).

Before

After

Also note that VO+Firefox does not appear to respect/announce aria-rowcount, while Safari and Chrome appear to.

The Firefox + VO not honoring aria-rowcount seems like a 🐞 worth filing with Mozilla. I'll do that this afternoon.

It'd be awesome to mention that aria-sort on FF/VO also does nothing!

cee-chen · 2022-07-06T20:37:46Z

src/components/datagrid/body/data_grid_cell.tsx

    const content = (
      <div
        role="gridcell"
+        aria-rowindex={ariaRowIndex}


Without aria-rowindex and aria-rowcount, the automatic table/gridcell announcement that occurs when using screen reader navigation* (ctrl+opt+arrow keys VS. just arrow keys) is off.

Before

After

*Caveat

While this is a relatively minor and low-cost addition, @1Copenut and I discussed screen reader browsing/navigation behavior on data grids and came to the conclusion they will never be optimized vs. using regular arrow keys, for a few key reasons:

Screen reader navigation does not trigger focus (unless navigating to a focusable child within a grid cell) on a grid cell and as such the 'current cell' falls out of date. This is reproducible on W3's own datagrid example: https://www.w3.org/WAI/ARIA/apg/example-index/grid/dataGrids

Since screen reader navigation doesn't scroll grids, it will never work properly with scrolling virtualized grids

cee-chen · 2022-07-06T21:06:22Z

src/components/datagrid/body/data_grid_cell.tsx

+          <p>
+            {'- '}
+            <EuiI18n
+              token="euiDataGridCell.position"
+              default="Row {row}, Column {col} ({columnId})"
+              values={{
+                row: ariaRowIndex,
+                col: colIndex + 1,
+                columnId: column?.displayAsText || rest.columnId,
+              }}
+            />
+          </p>


Adding the (column Name/ID) in parenthesis helps contextualize the cell more, similar to default aria table behavior. I changed the - and : punctuation for a more natural sounding voice flow - let me know what you think!

Before:

(Note: GH's video player appears to start muted, be sure to unmute it)

before.mp4

After

(Note: GH's video player appears to start muted, be sure to unmute it)

after.mp4

I've been listening to this read back all morning and thinking of it in terms of data importance priority. (I totally made that up). I swapped the Column Name/ID ahead of the row and column number combo. Putting myself in the place of a user digging through a column of data, I wanted to hear the data first, then the label, then my position in the grid. What do you think about swapping line 99 to be

! data_grid_cell#L99 <EuiScreenReaderOnly> {'- '} <EuiI18n token="euiDataGridCell.position" - default="Row {row}, Column {col} ({columnId})" + default="{columnId}. Row {row}, Column {col}." values={{ row: ariaRowIndex, col: colIndex + 1, columnId: column?.displayAsText || rest.columnId, }} /> </EuiScreenReaderOnly>

I don't disagree on column label/title feeling like it should come before position, but I also simultaneously feel like it makes sense to keep the column name and column index together since they're related 🤔 how about something like

{Column Title}, column {number}, row {number} e.g., Amount, Column 2, Row 8

Not sure if this matters also, but it might be worth nothing that some column titles are extremely technical and long in Kibana and not nice and human-readable, e.g. destination.ip_address or similar

@constancecchen Good call. Let's keep the column name and index together.

Your point about some Kibana column titles being technical and long is spot on. It matters, but how much is a sliding scale. I'm floating the idea elsewhere, and feel the benefits outweigh the increased cognitive load. If research proves out otherwise, we can always revisit this solution.

Gotcha! So just to confirm, do you want to stay with the current copy, or do you want to change it to:

Suggested change



{'- '}

<EuiI18n

token="euiDataGridCell.position"

default="Row {row}, Column {col} ({columnId})"

values={{

row: ariaRowIndex,

col: colIndex + 1,

columnId: column?.displayAsText || rest.columnId,

}}

/>





{'- '}

<EuiI18n

token="euiDataGridCell.position"

default="{columnId}, column {col}, row {row}"

values={{

row: ariaRowIndex,

col: colIndex + 1,

columnId: column?.displayAsText || rest.columnId,

}}

/>



?

Let's change it to the default="{columnId}, column {col}, row {row}" you proposed in the green addition.

My apologies, comment clarity is a work in progress.

cee-chen · 2022-07-06T21:11:25Z

One more thing worth noting - it appears we do not need aria-colcount or aria-colindex as we do not virtualize our sticky header cells with role="columnheader", and as such / because they're always present, the column count is always correct. Feel free to test/confirm this however Trevor, once staging is up

kibanamachine · 2022-07-06T21:52:41Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6033/

1Copenut

These aria additions are significant improvements for virtualized and paginated tables. It gives a proper context and informs users of the potential cognitive load involved in parsing data. Much appreciated!

I have just one change request, noted in its own comment.

The changes drove really well in Safari + VO, and I'll give them a second pass in Safari and Firefox later today.

1Copenut · 2022-07-07T15:31:05Z

src/components/datagrid/body/data_grid_cell.tsx

+          <p>
+            {'- '}
+            <EuiI18n
+              token="euiDataGridCell.position"
+              default="Row {row}, Column {col} ({columnId})"
+              values={{
+                row: ariaRowIndex,
+                col: colIndex + 1,
+                columnId: column?.displayAsText || rest.columnId,
+              }}
+            />
+          </p>


I've been listening to this read back all morning and thinking of it in terms of data importance priority. (I totally made that up). I swapped the Column Name/ID ahead of the row and column number combo. Putting myself in the place of a user digging through a column of data, I wanted to hear the data first, then the label, then my position in the grid. What do you think about swapping line 99 to be

! data_grid_cell#L99 <EuiScreenReaderOnly> {'- '} <EuiI18n token="euiDataGridCell.position" - default="Row {row}, Column {col} ({columnId})" + default="{columnId}. Row {row}, Column {col}." values={{ row: ariaRowIndex, col: colIndex + 1, columnId: column?.displayAsText || rest.columnId, }} /> </EuiScreenReaderOnly>

1Copenut · 2022-07-07T15:33:29Z

src/components/datagrid/data_grid.tsx

                  data-test-subj="euiDataGridBody"
                  className="euiDataGrid__content"
                  role="grid"
+                  aria-rowcount={rowCount}


The Firefox + VO not honoring aria-rowcount seems like a 🐞 worth filing with Mozilla. I'll do that this afternoon.

1Copenut · 2022-07-07T19:28:35Z

One more thing worth noting - it appears we do not need aria-colcount or aria-colindex as we do not virtualize our sticky header cells with role="columnheader", and as such / because they're always present, the column count is always correct. Feel free to test/confirm this however Trevor, once staging is up

👍 I concur. Getting accurate counts of columns in Safari and Firefox + VO.

kibanamachine · 2022-07-08T16:45:43Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6033/

1Copenut

The new order of announcement sounds great, and feels correct for aural UX. Code changes tested in Safari and Firefox with VoiceOver. Thanks @constancecchen !

cee-chen added 3 commits July 6, 2022 11:10

Add more accurate aria-row attributes to the datagrid

9cd0db7

- this enables row counts in virtualized grids to be correctly announced (instead of only counting the rendered rows) - NOTE: This works in VO+Safari but not VO+FF

Calculate aria-rowindex more accurately

8577851

- which requires passing `pagination` down to `EuiDataGridCell` and adding # of paginated rows to the visible row index

Improve cell position screen reader speech

3fab337

- add column header name/ID in parenthesis after column index - fix row index - was not correct on sort, and should now do so - add `-` for extra spacing between cell content and cell data, remove `:` for more natural speech cadence

cee-chen requested a review from 1Copenut July 6, 2022 20:20

cee-chen commented Jul 6, 2022

View reviewed changes

Changelog

69dd68e

cee-chen marked this pull request as ready for review July 6, 2022 21:24

cee-chen mentioned this pull request Jul 7, 2022

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

Merged

4 tasks

1Copenut suggested changes Jul 7, 2022

View reviewed changes

[PR feedback] Column name/position order

bada576

cee-chen requested a review from 1Copenut July 8, 2022 16:16

1Copenut approved these changes Jul 8, 2022

View reviewed changes

cee-chen merged commit d0bcd50 into elastic:main Jul 8, 2022

cee-chen deleted the datagrid/aria-attrs branch July 8, 2022 21:10

breehall mentioned this pull request Jul 14, 2022

Upgrade EUI to v60.3.0 elastic/kibana#136405

Merged

[EuiDataGrid] Improve aria row counts and indices for screen reader users #6033

[EuiDataGrid] Improve aria row counts and indices for screen reader users #6033

Uh oh!

Conversation

cee-chen commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

kibanamachine commented Jul 6, 2022

Uh oh!

Choose a reason for hiding this comment

Before

After

Uh oh!

cee-chen Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Before

After

*Caveat

Uh oh!

Choose a reason for hiding this comment

Before:

After

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cee-chen Jul 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cee-chen commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented Jul 6, 2022

Uh oh!

1Copenut left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1Copenut commented Jul 7, 2022

Uh oh!

kibanamachine commented Jul 8, 2022

Uh oh!

1Copenut left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[EuiDataGrid] Improve `aria` row counts and indices for screen reader users #6033

[EuiDataGrid] Improve `aria` row counts and indices for screen reader users #6033

cee-chen commented Jul 6, 2022 •

edited

Loading

cee-chen Jul 6, 2022 •

edited

Loading

cee-chen Jul 7, 2022 •

edited

Loading

cee-chen commented Jul 6, 2022 •

edited

Loading