diff --git a/CHANGELOG.md b/CHANGELOG.md index 5359b1b46b7..32cb9f63277 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ **Bug fixes** - Fixed an `EuiDataGrid` race condition where grid rows had incorrect heights if loaded in before CSS ([#5284](https://github.com/elastic/eui/pull/5284)) +- Fixed an accessibility issue where `EuiDataGrid` cells weren't owned by `role=row` elements ([#5285](https://github.com/elastic/eui/pull/5285)) ## [`41.0.0`](https://github.com/elastic/eui/tree/v41.0.0) diff --git a/package.json b/package.json index b35fde2064d..a555911e6f6 100644 --- a/package.json +++ b/package.json @@ -32,8 +32,8 @@ "test-unit": "cross-env NODE_ENV=test jest --config ./scripts/jest/config.json", "test-a11y": "node ./scripts/a11y-testing", "test-staged": "yarn lint && node scripts/test-staged.js", - "test-cypress": "cross-env NODE_ENV=cypress_test cypress run-ct", - "test-cypress-dev": "cross-env NODE_ENV=cypress_test cypress open-ct", + "test-cypress": "cross-env BABEL_MODULES=false NODE_ENV=cypress_test cypress run-ct", + "test-cypress-dev": "cross-env BABEL_MODULES=false NODE_ENV=cypress_test cypress open-ct", "combine-test-coverage": "sh ./scripts/combine-coverage.sh", "start-test-server": "BABEL_MODULES=false NODE_ENV=puppeteer NODE_OPTIONS=--max-old-space-size=4096 webpack-dev-server --config src-docs/webpack.config.js --port 9999", "yo-component": "yo ./generator-eui/app/component.js", diff --git a/scripts/a11y-testing.js b/scripts/a11y-testing.js index d770e901c61..f21ac10ab80 100644 --- a/scripts/a11y-testing.js +++ b/scripts/a11y-testing.js @@ -11,13 +11,7 @@ const docsPages = async (root, page) => { `${root}#/forms/color-selection`, `${root}#/forms/date-picker`, `${root}#/forms/super-date-picker`, - `${root}#/tabular-content/data-grid`, - `${root}#/tabular-content/data-grid-in-memory-settings`, - `${root}#/tabular-content/data-grid-schemas-and-popovers`, - `${root}#/tabular-content/data-grid-focus`, `${root}#/tabular-content/data-grid-styling-and-control`, - `${root}#/tabular-content/data-grid-control-columns`, - `${root}#/tabular-content/data-grid-footer-row`, `${root}#/tabular-content/data-grid-virtualization`, `${root}#/tabular-content/data-grid-row-heights-options`, ]; diff --git a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap index 27962a9b854..cd3b13a872c 100644 --- a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap +++ b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap @@ -1037,6 +1037,7 @@ Array [ tabindex="0" >
= ({ schemaDetectors, rowHeightsOptions, rowHeightUtils, + rowManager, } = data; const { headerRowHeight } = useContext(DataGridWrapperRowsContext); @@ -131,6 +134,7 @@ export const Cell: FunctionComponent = ({ rowHeightsOptions, rowHeightUtils, setRowHeight: isFirstColumn ? setRowHeight : undefined, + rowManager: rowManager, }; if (isLeadingControlColumn) { @@ -583,6 +587,14 @@ export const EuiDataGridBody: FunctionComponent = ( const wrapperRef = useRef(null); const wrapperDimensions = useResizeObserver(wrapperRef.current); + const innerGridRef = useRef(null); + + // useState instead of useMemo as React reserves the right to drop memoized + // values in the future, and that would be very bad here + const [rowManager] = useState(() => + makeRowManager(innerGridRef) + ); + // reset height constraint when rowCount changes useEffect(() => { setHeight(undefined); @@ -647,6 +659,7 @@ export const EuiDataGridBody: FunctionComponent = ( > {(mutationRef) => (
{ wrapperRef.current = el; @@ -661,6 +674,7 @@ export const EuiDataGridBody: FunctionComponent = ( {...(virtualizationOptions ? virtualizationOptions : {})} ref={setGridRef} innerElementType={InnerElement} + innerRef={innerGridRef} className={VIRTUALIZED_CONTAINER_CLASS} columnCount={ leadingControlColumns.length + @@ -690,6 +704,7 @@ export const EuiDataGridBody: FunctionComponent = ( interactiveCellId, rowHeightsOptions, rowHeightUtils, + rowManager, }} rowCount={ IS_JEST_ENVIRONMENT || headerRowHeight > 0 diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index e3b2fb07c5b..581c89706e5 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -18,6 +18,7 @@ import React, { memo, MutableRefObject, } from 'react'; +import { createPortal } from 'react-dom'; import tabbable from 'tabbable'; import { keys } from '../../../services'; import { EuiScreenReaderOnly } from '../../accessibility'; @@ -33,6 +34,7 @@ import { } from '../data_grid_types'; import { EuiDataGridCellButtons } from './data_grid_cell_buttons'; import { EuiDataGridCellPopover } from './data_grid_cell_popover'; +import { IS_JEST_ENVIRONMENT } from '../../../test'; const EuiDataGridCellContent: FunctionComponent< EuiDataGridCellValueProps & { @@ -362,9 +364,11 @@ export class EuiDataGridCell extends Component< className, column, style, + rowHeightsOptions, + rowManager, ...rest } = this.props; - const { rowIndex, rowHeightsOptions } = rest; + const { rowIndex } = rest; const showCellButtons = this.state.isFocused || @@ -553,7 +557,7 @@ export class EuiDataGridCell extends Component< } } - return ( + const content = (
{ @@ -575,5 +580,9 @@ export class EuiDataGridCell extends Component< {innerContent}
); + + return rowManager && !IS_JEST_ENVIRONMENT + ? createPortal(content, rowManager.getRow(rowIndex)) + : content; } } diff --git a/src/components/datagrid/body/data_grid_footer_row.test.tsx b/src/components/datagrid/body/data_grid_footer_row.test.tsx index 4344a2a39a2..e62f3c5fc1c 100644 --- a/src/components/datagrid/body/data_grid_footer_row.test.tsx +++ b/src/components/datagrid/body/data_grid_footer_row.test.tsx @@ -30,7 +30,7 @@ describe('EuiDataGridFooterRow', () => { expect(component).toMatchInlineSnapshot(`
{ expect(component).toMatchInlineSnapshot(`
{ expect(component).toMatchInlineSnapshot(`
+): EuiDataGridRowManager => { + const rowIdToElements = new Map(); + + return { + getRow(rowIndex) { + let rowElement = rowIdToElements.get(rowIndex); + + if (rowElement == null) { + rowElement = document.createElement('div'); + rowElement.setAttribute('role', 'row'); + rowElement.classList.add('euiDataGridRow'); + rowIdToElements.set(rowIndex, rowElement); + + // add the element to the wrapping container + containerRef.current?.appendChild(rowElement); + + // watch the row's children, if they all disappear then remove this row + const observer = new MutationObserver((records) => { + if ((records[0].target as HTMLElement).childElementCount === 0) { + observer.disconnect(); + rowElement?.remove(); + rowIdToElements.delete(rowIndex); + } + }); + observer.observe(rowElement, { childList: true }); + } + + return rowElement; + }, + }; +}; diff --git a/src/components/datagrid/data_grid.spec.tsx b/src/components/datagrid/data_grid.spec.tsx new file mode 100644 index 00000000000..39a33169a5a --- /dev/null +++ b/src/components/datagrid/data_grid.spec.tsx @@ -0,0 +1,114 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; +import { mount } from '@cypress/react'; +import { EuiDataGrid, EuiDataGridProps } from './index'; + +const baseProps: EuiDataGridProps = { + 'aria-label': 'grid for testing', + columns: [ + { id: 'a', display: 'First' }, + { id: 'b', display: 'Second' }, + ], + columnVisibility: { + visibleColumns: ['a', 'b'], + setVisibleColumns: () => {}, + }, + rowCount: 3, + renderCellValue: ({ rowIndex, columnId }) => `${columnId}, ${rowIndex}`, + renderFooterCellValue: ({ columnId }) => `${columnId}, footer`, +}; + +describe('EuiDataGrid', () => { + describe('row creation', () => { + it('creates rows', () => { + mount(); + + getGridData().then((data) => { + expect(data).to.deep.equal({ + headers: ['First', 'Second'], + data: [ + { First: 'a, 0', Second: 'b, 0' }, + { First: 'a, 1', Second: 'b, 1' }, + { First: 'a, 2', Second: 'b, 2' }, + ], + footer: { First: 'a, footer', Second: 'b, footer' }, + }); + }); + + // find all cells and verify they all belong to a row or columnheader + cy.get('[role=gridcell]') + .filter((idx, element) => { + const role = element.parentElement?.getAttribute('role'); + return role === 'row' || role === 'columnheader' ? false : true; + }) + .should('have.lengthOf', 0); + }); + }); +}); + +function getGridData() { + // wait for the virtualized cells to render + cy.get('[data-gridcell-id="1,0"]'); + const rows = cy.get('[role=row]'); + return rows.then((rows) => { + const headers: string[] = []; + const data = []; + + // process header + const headerRow = rows[0]; + const headerCells = headerRow.querySelectorAll('[role=columnheader]'); + for (let i = 0; i < headerCells.length; i++) { + const headerCell = headerCells[i]; + headers.push(headerCell.textContent ?? ''); + } + + // process data rows + for (let i = 1; i < rows.length; i++) { + const row = rows[i]; + + if (row.getAttribute('data-test-subj')?.includes('dataGridFooterRow')) { + // we don't want to load the footer data yet + continue; + } + + const cellData: { [key: string]: string } = {}; + + const cells = row.querySelectorAll( + '[role=gridcell] [data-datagrid-cellcontent]' + ); + for (let j = 0; j < cells.length; j++) { + const cell = cells[j]; + cellData[headers[j]] = cell.textContent ?? ''; + } + + data.push(cellData); + } + + // find any footer data + const footerRow = rows[rows.length - 1]; + const hasFooterRow = footerRow + .getAttribute('data-test-subj') + ?.includes('dataGridFooterRow'); + const footerData: { [key: string]: string } | null = hasFooterRow + ? {} + : null; + if (hasFooterRow === true) { + const footerCells = footerRow.querySelectorAll( + '[role=gridcell] [data-datagrid-cellcontent]' + ); + for (let i = 0; i < footerCells.length; i++) { + const footerCell = footerCells[i]; + footerData![headers[i]] = footerCell.textContent ?? ''; + } + } + + return { headers, data, footer: footerData }; + }); +} diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index 00a27d220f2..a05f1df80be 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -529,6 +529,7 @@ describe('EuiDataGrid', () => { Array [ Object { "className": "euiDataGridRowCell euiDataGridRowCell--firstColumn customClass", + "data-gridcell-id": "0,0", "data-test-subj": "dataGridRowCell", "onBlur": [Function], "onFocus": [Function], @@ -549,6 +550,7 @@ describe('EuiDataGrid', () => { }, Object { "className": "euiDataGridRowCell euiDataGridRowCell--lastColumn customClass", + "data-gridcell-id": "0,1", "data-test-subj": "dataGridRowCell", "onBlur": [Function], "onFocus": [Function], @@ -569,6 +571,7 @@ describe('EuiDataGrid', () => { }, Object { "className": "euiDataGridRowCell euiDataGridRowCell--stripe euiDataGridRowCell--firstColumn customClass", + "data-gridcell-id": "1,0", "data-test-subj": "dataGridRowCell", "onBlur": [Function], "onFocus": [Function], @@ -589,6 +592,7 @@ describe('EuiDataGrid', () => { }, Object { "className": "euiDataGridRowCell euiDataGridRowCell--stripe euiDataGridRowCell--lastColumn customClass", + "data-gridcell-id": "1,1", "data-test-subj": "dataGridRowCell", "onBlur": [Function], "onFocus": [Function], diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index ebf2a51b60c..e9908d8b22c 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -61,9 +61,7 @@ import { EuiDataGridStyleRowHover, } from './data_grid_types'; import { RowHeightUtils } from './row_height_utils'; - -// Used to short-circuit some async browser behaviour that is difficult to account for in tests -const IS_JEST_ENVIRONMENT = global.hasOwnProperty('_isJest'); +import { IS_JEST_ENVIRONMENT } from '../../test'; // Each gridStyle object above sets a specific CSS select to .euiGrid const fontSizesToClassMap: { [size in EuiDataGridStyleFontSizes]: string } = { diff --git a/src/components/datagrid/data_grid_types.ts b/src/components/datagrid/data_grid_types.ts index 21aee05569a..4c3691467c2 100644 --- a/src/components/datagrid/data_grid_types.ts +++ b/src/components/datagrid/data_grid_types.ts @@ -383,6 +383,7 @@ export interface EuiDataGridCellProps { style?: React.CSSProperties; rowHeightsOptions?: EuiDataGridRowHeightsOptions; rowHeightUtils?: RowHeightUtils; + rowManager?: EuiDataGridRowManager; } export interface EuiDataGridCellState { @@ -396,7 +397,7 @@ export interface EuiDataGridCellState { export type EuiDataGridCellValueProps = Omit< EuiDataGridCellProps, - 'width' | 'interactiveCellId' | 'popoverContent' + 'width' | 'interactiveCellId' | 'popoverContent' | 'rowManager' >; export interface EuiDataGridControlColumn { /** @@ -724,3 +725,7 @@ export interface EuiDataGridRowHeightsOptions { */ lineHeight?: string; } + +export interface EuiDataGridRowManager { + getRow(rowIndex: number): HTMLDivElement; +} diff --git a/src/test/index.ts b/src/test/index.ts index 4a7c8ff1450..499734984a4 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -14,3 +14,4 @@ export { stopThrowingReactWarnings, } from './react_warnings'; export { sleep } from './sleep'; +export { IS_JEST_ENVIRONMENT } from './is_jest'; diff --git a/src/test/is_jest.ts b/src/test/is_jest.ts new file mode 100644 index 00000000000..d304842ca67 --- /dev/null +++ b/src/test/is_jest.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export const IS_JEST_ENVIRONMENT = global.hasOwnProperty('_isJest');