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] Improve aria row counts and indices for screen reader users #6033

Merged
merged 5 commits into from
Jul 8, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9cd0db7
Add more accurate aria-row attributes to the datagrid
cee-chen Jul 6, 2022
8577851
Calculate `aria-rowindex` more accurately
cee-chen Jul 6, 2022
3fab337
Improve cell position screen reader speech
cee-chen Jul 6, 2022
69dd68e
Changelog
cee-chen Jul 6, 2022
bada576
[PR feedback] Column name/position order
cee-chen Jul 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 64 additions & 30 deletions src/components/datagrid/__snapshots__/data_grid.test.tsx.snap
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions src/components/datagrid/body/__snapshots__/data_grid_body.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ exports[`EuiDataGridBody renders 1`] = `
</div>
</div>
<div
aria-rowindex="1"
class="euiDataGridRowCell euiDataGridRowCell--boolean euiDataGridRowCell--firstColumn"
data-gridcell-column-id="columnA"
data-gridcell-column-index="0"
Expand Down Expand Up @@ -128,12 +129,13 @@ exports[`EuiDataGridBody renders 1`] = `
<p
class="emotion-euiScreenReaderOnly"
>
Row: 1; Column: 1
- columnA, column 1, row 1
</p>
</div>
</div>
</div>
<div
aria-rowindex="1"
class="euiDataGridRowCell euiDataGridRowCell--string euiDataGridRowCell--lastColumn"
data-gridcell-column-id="columnB"
data-gridcell-column-index="1"
Expand Down Expand Up @@ -161,7 +163,7 @@ exports[`EuiDataGridBody renders 1`] = `
<p
class="emotion-euiScreenReaderOnly"
>
Row: 1; Column: 2
- columnB, column 2, row 1
</p>
</div>
</div>
Expand Down
3 changes: 2 additions & 1 deletion src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Array [

exports[`EuiDataGridCell renders 1`] = `
<div
aria-rowindex="1"
class="euiDataGridRowCell"
data-gridcell-column-id="someColumn"
data-gridcell-column-index="0"
Expand Down Expand Up @@ -74,7 +75,7 @@ exports[`EuiDataGridCell renders 1`] = `
<p
class="emotion-euiScreenReaderOnly"
>
Row: 1; Column: 1
- someColumn, column 1, row 1
</p>
</div>
</div>
Expand Down
3 changes: 3 additions & 0 deletions src/components/datagrid/body/data_grid_body.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export const Cell: FunctionComponent<GridChildComponentProps> = ({
rowHeightsOptions,
rowHeightUtils,
rowManager,
pagination,
} = data;
const popoverContext = useContext(DataGridCellPopoverContext);
const { headerRowHeight } = useContext(DataGridWrapperRowsContext);
Expand Down Expand Up @@ -118,6 +119,7 @@ export const Cell: FunctionComponent<GridChildComponentProps> = ({
setRowHeight: isFirstColumn ? setRowHeight : undefined,
rowManager,
popoverContext,
pagination,
};

if (isLeadingControlColumn) {
Expand Down Expand Up @@ -487,6 +489,7 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
rowHeightsOptions,
rowHeightUtils,
rowManager,
pagination,
}}
rowCount={
IS_JEST_ENVIRONMENT || headerRowHeight > 0 ? visibleRowCount : 0
Expand Down
17 changes: 17 additions & 0 deletions src/components/datagrid/body/data_grid_cell.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,23 @@ describe('EuiDataGridCell', () => {
expect(component).toMatchSnapshot();
});

it("renders the cell's `aria-rowindex` correctly when paginated on a different page", () => {
const component = mount(
<EuiDataGridCell
{...requiredProps}
pagination={{
pageIndex: 3,
pageSize: 20,
onChangePage: () => {},
onChangeItemsPerPage: () => {},
}}
/>
);
expect(
component.find('[data-test-subj="dataGridRowCell"]').prop('aria-rowindex')
).toEqual(61);
});

it('renders cell actions', () => {
const component = mount(
<EuiDataGridCell
Expand Down
30 changes: 22 additions & 8 deletions src/components/datagrid/body/data_grid_cell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { tabbable } from 'tabbable';
import { keys } from '../../../services';
import { EuiScreenReaderOnly } from '../../accessibility';
import { EuiFocusTrap } from '../../focus_trap';
import { useEuiI18n } from '../../i18n';
import { EuiI18n } from '../../i18n';
import { hasResizeObserver } from '../../observer/resize_observer/resize_observer';
import { DataGridFocusContext } from '../utils/focus';
import {
Expand All @@ -46,6 +46,7 @@ const EuiDataGridCellContent: FunctionComponent<
setCellContentsRef: EuiDataGridCell['setCellContentsRef'];
isExpanded: boolean;
isDefinedHeight: boolean;
ariaRowIndex: number;
}
> = memo(
({
Expand All @@ -55,6 +56,7 @@ const EuiDataGridCellContent: FunctionComponent<
rowHeightsOptions,
rowIndex,
colIndex,
ariaRowIndex,
rowHeightUtils,
isDefinedHeight,
...rest
Expand All @@ -64,12 +66,6 @@ const EuiDataGridCellContent: FunctionComponent<
EuiDataGridCellValueElementProps
>;

const positionText = useEuiI18n(
'euiDataGridCell.position',
'Row: {row}; Column: {col}',
{ row: rowIndex + 1, col: colIndex + 1 }
);

return (
<>
<div
Expand All @@ -96,7 +92,18 @@ const EuiDataGridCellContent: FunctionComponent<
/>
</div>
<EuiScreenReaderOnly>
<p>{positionText}</p>
<p>
{'- '}
<EuiI18n
token="euiDataGridCell.position"
default="{columnId}, column {col}, row {row}"
values={{
columnId: column?.displayAsText || rest.columnId,
col: colIndex + 1,
row: ariaRowIndex,
}}
/>
</p>
</EuiScreenReaderOnly>
</>
);
Expand Down Expand Up @@ -518,6 +525,7 @@ export class EuiDataGridCell extends Component<
rowHeightUtils,
rowHeightsOptions,
rowManager,
pagination,
...rest
} = this.props;
const { rowIndex, visibleRowIndex, colIndex } = rest;
Expand All @@ -539,6 +547,10 @@ export class EuiDataGridCell extends Component<
className
);

const ariaRowIndex = pagination
? visibleRowIndex + 1 + pagination.pageSize * pagination.pageIndex
: visibleRowIndex + 1;

const {
isExpandable: _, // Not a valid DOM property, so needs to be destructured out
style: cellPropsStyle,
Expand Down Expand Up @@ -635,6 +647,7 @@ export class EuiDataGridCell extends Component<
rowHeightsOptions,
rowHeightUtils,
isDefinedHeight,
ariaRowIndex,
};

const anchorClass = 'euiDataGridRowCell__expandFlex';
Expand Down Expand Up @@ -686,6 +699,7 @@ export class EuiDataGridCell extends Component<
const content = (
<div
role="gridcell"
aria-rowindex={ariaRowIndex}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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
  2. Since screen reader navigation doesn't scroll grids, it will never work properly with scrolling virtualized grids

tabIndex={
this.state.isFocused && !this.state.disableCellTabIndex ? 0 : -1
}
Expand Down
4 changes: 4 additions & 0 deletions src/components/datagrid/data_grid.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ describe('EuiDataGrid', () => {
).toMatchInlineSnapshot(`
Array [
Object {
"aria-rowindex": 1,
"className": "euiDataGridRowCell euiDataGridRowCell--firstColumn customClass",
"data-gridcell-column-id": "A",
"data-gridcell-column-index": 0,
Expand All @@ -567,6 +568,7 @@ describe('EuiDataGrid', () => {
"tabIndex": -1,
},
Object {
"aria-rowindex": 1,
"className": "euiDataGridRowCell euiDataGridRowCell--lastColumn customClass",
"data-gridcell-column-id": "B",
"data-gridcell-column-index": 1,
Expand All @@ -592,6 +594,7 @@ describe('EuiDataGrid', () => {
"tabIndex": -1,
},
Object {
"aria-rowindex": 2,
"className": "euiDataGridRowCell euiDataGridRowCell--firstColumn customClass",
"data-gridcell-column-id": "A",
"data-gridcell-column-index": 0,
Expand All @@ -617,6 +620,7 @@ describe('EuiDataGrid', () => {
"tabIndex": -1,
},
Object {
"aria-rowindex": 2,
"className": "euiDataGridRowCell euiDataGridRowCell--lastColumn customClass",
"data-gridcell-column-id": "B",
"data-gridcell-column-index": 1,
Expand Down
1 change: 1 addition & 0 deletions src/components/datagrid/data_grid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ export const EuiDataGrid = forwardRef<EuiDataGridRefProps, EuiDataGridProps>(
data-test-subj="euiDataGridBody"
className="euiDataGrid__content"
role="grid"
aria-rowcount={rowCount}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@cee-chen cee-chen Jul 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

id={gridId}
{...wrappingDivFocusProps} // re: above jsx-a11y - tabIndex is handled by these props, but the linter isn't smart enough to know that
{...gridAriaProps}
Expand Down
1 change: 1 addition & 0 deletions src/components/datagrid/data_grid_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ export interface EuiDataGridCellProps {
rowHeightsOptions?: EuiDataGridRowHeightsOptions;
rowHeightUtils?: RowHeightUtils;
rowManager?: EuiDataGridRowManager;
pagination?: EuiDataGridPaginationProps;
}

export interface EuiDataGridCellState {
Expand Down
4 changes: 4 additions & 0 deletions upcoming_changelogs/6033.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
**Bug fixes**

- Fixed `EuiDataGrid`'s row count/indices announced to screen readers when virtualized
- Fixed `EuiDataGrid`'s current cell row/column position announced to screen readers when sorted and paginated, and also improved column identification and announcement cadence