From d17b6a5037d5a8baf1b64bd5e49b3c9e7da17c82 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 22 Nov 2023 20:13:39 -0800 Subject: [PATCH 01/20] [organization] Move cell-related files to their own folder - there's enough now that it's clogging up the `body/` folder and it makes sense to --- .../data_grid_cell.test.tsx.snap | 0 .../body/{ => cell}/data_grid_cell.test.tsx | 10 +++++----- .../body/{ => cell}/data_grid_cell.tsx | 20 +++++++++---------- .../data_grid_cell_actions.test.tsx | 2 +- .../{ => cell}/data_grid_cell_actions.tsx | 15 ++++++++------ .../data_grid_cell_popover.spec.tsx | 4 ++-- .../data_grid_cell_popover.test.tsx | 6 +++--- .../{ => cell}/data_grid_cell_popover.tsx | 10 +++++----- .../data_grid_cell_wrapper.test.tsx | 4 ++-- .../{ => cell}/data_grid_cell_wrapper.tsx | 4 ++-- src/components/datagrid/body/cell/index.ts | 16 +++++++++++++++ .../datagrid/body/data_grid_body_custom.tsx | 2 +- .../body/data_grid_body_virtualized.tsx | 2 +- .../body/footer/data_grid_footer_row.tsx | 3 +-- src/components/datagrid/data_grid.tsx | 5 +---- src/components/datagrid/utils/scrolling.tsx | 2 +- 16 files changed, 60 insertions(+), 45 deletions(-) rename src/components/datagrid/body/{ => cell}/__snapshots__/data_grid_cell.test.tsx.snap (100%) rename src/components/datagrid/body/{ => cell}/data_grid_cell.test.tsx (98%) rename src/components/datagrid/body/{ => cell}/data_grid_cell.tsx (97%) rename src/components/datagrid/body/{ => cell}/data_grid_cell_actions.test.tsx (99%) rename src/components/datagrid/body/{ => cell}/data_grid_cell_actions.tsx (93%) rename src/components/datagrid/body/{ => cell}/data_grid_cell_popover.spec.tsx (98%) rename src/components/datagrid/body/{ => cell}/data_grid_cell_popover.test.tsx (97%) rename src/components/datagrid/body/{ => cell}/data_grid_cell_popover.tsx (96%) rename src/components/datagrid/body/{ => cell}/data_grid_cell_wrapper.test.tsx (95%) rename src/components/datagrid/body/{ => cell}/data_grid_cell_wrapper.tsx (98%) create mode 100644 src/components/datagrid/body/cell/index.ts diff --git a/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap b/src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap similarity index 100% rename from src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap rename to src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap diff --git a/src/components/datagrid/body/data_grid_cell.test.tsx b/src/components/datagrid/body/cell/data_grid_cell.test.tsx similarity index 98% rename from src/components/datagrid/body/data_grid_cell.test.tsx rename to src/components/datagrid/body/cell/data_grid_cell.test.tsx index da2eccd6284..58ef5dea8a4 100644 --- a/src/components/datagrid/body/data_grid_cell.test.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.test.tsx @@ -9,11 +9,11 @@ import React, { useEffect } from 'react'; import { mount, ReactWrapper } from 'enzyme'; import { act } from '@testing-library/react'; -import { keys } from '../../../services'; -import { render } from '../../../test/rtl'; -import { RowHeightUtils } from '../utils/__mocks__/row_heights'; -import { mockFocusContext } from '../utils/__mocks__/focus_context'; -import { DataGridFocusContext } from '../utils/focus'; +import { keys } from '../../../../services'; +import { render } from '../../../../test/rtl'; +import { RowHeightUtils } from '../../utils/__mocks__/row_heights'; +import { mockFocusContext } from '../../utils/__mocks__/focus_context'; +import { DataGridFocusContext } from '../../utils/focus'; import { EuiDataGridCell } from './data_grid_cell'; diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/cell/data_grid_cell.tsx similarity index 97% rename from src/components/datagrid/body/data_grid_cell.tsx rename to src/components/datagrid/body/cell/data_grid_cell.tsx index 61ad48987c0..a4e0a87c712 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.tsx @@ -21,14 +21,14 @@ import React, { } from 'react'; import { createPortal } from 'react-dom'; import { tabbable } from 'tabbable'; -import { keys } from '../../../services'; -import { EuiScreenReaderOnly } from '../../accessibility'; -import { EuiFocusTrap } from '../../focus_trap'; -import { EuiI18n } from '../../i18n'; -import { EuiTextBlockTruncate } from '../../text_truncate'; -import { hasResizeObserver } from '../../observer/resize_observer/resize_observer'; -import { DataGridFocusContext } from '../utils/focus'; -import { RowHeightVirtualizationUtils } from '../utils/row_heights'; +import { keys } from '../../../../services'; +import { EuiScreenReaderOnly } from '../../../accessibility'; +import { EuiFocusTrap } from '../../../focus_trap'; +import { EuiI18n } from '../../../i18n'; +import { EuiTextBlockTruncate } from '../../../text_truncate'; +import { hasResizeObserver } from '../../../observer/resize_observer/resize_observer'; +import { DataGridFocusContext } from '../../utils/focus'; +import { RowHeightVirtualizationUtils } from '../../utils/row_heights'; import { EuiDataGridCellProps, EuiDataGridCellState, @@ -37,13 +37,13 @@ import { EuiDataGridCellValueProps, EuiDataGridCellPopoverElementProps, EuiDataGridRowHeightOption, -} from '../data_grid_types'; +} from '../../data_grid_types'; import { EuiDataGridCellActions, EuiDataGridCellPopoverActions, } from './data_grid_cell_actions'; import { DefaultCellPopover } from './data_grid_cell_popover'; -import { IS_JEST_ENVIRONMENT } from '../../../utils'; +import { IS_JEST_ENVIRONMENT } from '../../../../utils'; const EuiDataGridCellContent: FunctionComponent< EuiDataGridCellValueProps & { diff --git a/src/components/datagrid/body/data_grid_cell_actions.test.tsx b/src/components/datagrid/body/cell/data_grid_cell_actions.test.tsx similarity index 99% rename from src/components/datagrid/body/data_grid_cell_actions.test.tsx rename to src/components/datagrid/body/cell/data_grid_cell_actions.test.tsx index 6ab8a4f3e06..fe16e085dd9 100644 --- a/src/components/datagrid/body/data_grid_cell_actions.test.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell_actions.test.tsx @@ -9,7 +9,7 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { EuiDataGridColumnCellAction } from '../data_grid_types'; +import { EuiDataGridColumnCellAction } from '../../data_grid_types'; import { EuiDataGridCellActions, EuiDataGridCellPopoverActions, diff --git a/src/components/datagrid/body/data_grid_cell_actions.tsx b/src/components/datagrid/body/cell/data_grid_cell_actions.tsx similarity index 93% rename from src/components/datagrid/body/data_grid_cell_actions.tsx rename to src/components/datagrid/body/cell/data_grid_cell_actions.tsx index 9c2f231e9e8..701b25655f2 100644 --- a/src/components/datagrid/body/data_grid_cell_actions.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell_actions.tsx @@ -11,13 +11,16 @@ import { EuiDataGridColumn, EuiDataGridColumnCellAction, EuiDataGridColumnCellActionProps, -} from '../data_grid_types'; +} from '../../data_grid_types'; -import { EuiI18n } from '../../i18n'; -import { EuiButtonIcon, EuiButtonIconProps } from '../../button/button_icon'; -import { EuiButtonEmpty, EuiButtonEmptyProps } from '../../button/button_empty'; -import { EuiFlexGroup, EuiFlexItem } from '../../flex'; -import { EuiPopoverFooter } from '../../popover'; +import { EuiI18n } from '../../../i18n'; +import { EuiButtonIcon, EuiButtonIconProps } from '../../../button/button_icon'; +import { + EuiButtonEmpty, + EuiButtonEmptyProps, +} from '../../../button/button_empty'; +import { EuiFlexGroup, EuiFlexItem } from '../../../flex'; +import { EuiPopoverFooter } from '../../../popover'; export const EuiDataGridCellActions = ({ onExpandClick, diff --git a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx b/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx similarity index 98% rename from src/components/datagrid/body/data_grid_cell_popover.spec.tsx rename to src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx index 53e5e49982b..cf02b7a1eee 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx @@ -8,10 +8,10 @@ /// /// -/// +/// import React, { useEffect } from 'react'; -import { EuiDataGrid, EuiDataGridProps } from '../'; +import { EuiDataGrid, EuiDataGridProps } from '../..'; const baseProps: EuiDataGridProps = { 'aria-label': 'Grid cell popover test', diff --git a/src/components/datagrid/body/data_grid_cell_popover.test.tsx b/src/components/datagrid/body/cell/data_grid_cell_popover.test.tsx similarity index 97% rename from src/components/datagrid/body/data_grid_cell_popover.test.tsx rename to src/components/datagrid/body/cell/data_grid_cell_popover.test.tsx index 360b5e86157..8c3762c493b 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.test.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell_popover.test.tsx @@ -7,12 +7,12 @@ */ import React from 'react'; -import { renderHook, renderHookAct } from '../../../test/rtl'; +import { renderHook, renderHookAct } from '../../../../test/rtl'; import { shallow } from 'enzyme'; -import { keys } from '../../../services'; +import { keys } from '../../../../services'; -import { DataGridCellPopoverContextShape } from '../data_grid_types'; +import { DataGridCellPopoverContextShape } from '../../data_grid_types'; import { useCellPopover, DefaultCellPopover } from './data_grid_cell_popover'; describe('useCellPopover', () => { diff --git a/src/components/datagrid/body/data_grid_cell_popover.tsx b/src/components/datagrid/body/cell/data_grid_cell_popover.tsx similarity index 96% rename from src/components/datagrid/body/data_grid_cell_popover.tsx rename to src/components/datagrid/body/cell/data_grid_cell_popover.tsx index 66df03f09d4..4504e1e5862 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell_popover.tsx @@ -9,12 +9,12 @@ import React, { createContext, useState, useCallback, ReactNode } from 'react'; import classNames from 'classnames'; -import { keys } from '../../../services'; -import { EuiWrappingPopover, EuiPopoverProps } from '../../popover'; +import { keys } from '../../../../services'; +import { EuiWrappingPopover, EuiPopoverProps } from '../../../popover'; import { DataGridCellPopoverContextShape, EuiDataGridCellPopoverElementProps, -} from '../data_grid_types'; +} from '../../data_grid_types'; export const DataGridCellPopoverContext = createContext({ @@ -153,8 +153,8 @@ export const useCellPopover = (): { /** * Popover content renderers */ -import { EuiText } from '../../text'; -import { EuiCodeBlock } from '../../code'; +import { EuiText } from '../../../text'; +import { EuiCodeBlock } from '../../../code'; export const DefaultCellPopover = ({ schema, diff --git a/src/components/datagrid/body/data_grid_cell_wrapper.test.tsx b/src/components/datagrid/body/cell/data_grid_cell_wrapper.test.tsx similarity index 95% rename from src/components/datagrid/body/data_grid_cell_wrapper.test.tsx rename to src/components/datagrid/body/cell/data_grid_cell_wrapper.test.tsx index 59ef9d70bf4..6b822ab55a8 100644 --- a/src/components/datagrid/body/data_grid_cell_wrapper.test.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell_wrapper.test.tsx @@ -9,8 +9,8 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { RowHeightUtils } from '../utils/__mocks__/row_heights'; -import { schemaDetectors } from '../utils/data_grid_schema'; +import { RowHeightUtils } from '../../utils/__mocks__/row_heights'; +import { schemaDetectors } from '../../utils/data_grid_schema'; import { Cell } from './data_grid_cell_wrapper'; diff --git a/src/components/datagrid/body/data_grid_cell_wrapper.tsx b/src/components/datagrid/body/cell/data_grid_cell_wrapper.tsx similarity index 98% rename from src/components/datagrid/body/data_grid_cell_wrapper.tsx rename to src/components/datagrid/body/cell/data_grid_cell_wrapper.tsx index 4100e98fc97..3a429494fcf 100644 --- a/src/components/datagrid/body/data_grid_cell_wrapper.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell_wrapper.tsx @@ -14,8 +14,8 @@ import { EuiDataGridBodyProps, EuiDataGridHeaderRowProps, EuiDataGridSchemaDetector, -} from '../data_grid_types'; -import { DataGridSortingContext } from '../utils/sorting'; +} from '../../data_grid_types'; +import { DataGridSortingContext } from '../../utils/sorting'; import { DataGridCellPopoverContext } from './data_grid_cell_popover'; import { EuiDataGridCell } from './data_grid_cell'; diff --git a/src/components/datagrid/body/cell/index.ts b/src/components/datagrid/body/cell/index.ts new file mode 100644 index 00000000000..ed2338e8de2 --- /dev/null +++ b/src/components/datagrid/body/cell/index.ts @@ -0,0 +1,16 @@ +/* + * 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 { EuiDataGridCell } from './data_grid_cell'; + +export { Cell } from './data_grid_cell_wrapper'; + +export { + DataGridCellPopoverContext, + useCellPopover, +} from './data_grid_cell_popover'; diff --git a/src/components/datagrid/body/data_grid_body_custom.tsx b/src/components/datagrid/body/data_grid_body_custom.tsx index 881deb3f642..bc8c4147cd6 100644 --- a/src/components/datagrid/body/data_grid_body_custom.tsx +++ b/src/components/datagrid/body/data_grid_body_custom.tsx @@ -24,7 +24,7 @@ import { } from '../data_grid_types'; import { useDataGridHeader } from './header'; import { useDataGridFooter } from './footer'; -import { Cell } from './data_grid_cell_wrapper'; +import { Cell } from './cell'; export const EuiDataGridBodyCustomRender: FunctionComponent< EuiDataGridBodyProps diff --git a/src/components/datagrid/body/data_grid_body_virtualized.tsx b/src/components/datagrid/body/data_grid_body_virtualized.tsx index c85c7c52cc8..43370d40804 100644 --- a/src/components/datagrid/body/data_grid_body_virtualized.tsx +++ b/src/components/datagrid/body/data_grid_body_virtualized.tsx @@ -24,7 +24,7 @@ import { import { useResizeObserver } from '../../observer/resize_observer'; import { useDataGridHeader } from './header'; import { useDataGridFooter } from './footer'; -import { Cell } from './data_grid_cell_wrapper'; +import { Cell } from './cell'; import { EuiDataGridBodyProps, DataGridWrapperRowsContentsShape, diff --git a/src/components/datagrid/body/footer/data_grid_footer_row.tsx b/src/components/datagrid/body/footer/data_grid_footer_row.tsx index 085bef3dcc1..10b7e3a5b10 100644 --- a/src/components/datagrid/body/footer/data_grid_footer_row.tsx +++ b/src/components/datagrid/body/footer/data_grid_footer_row.tsx @@ -8,8 +8,7 @@ import classnames from 'classnames'; import React, { forwardRef, memo, useContext } from 'react'; -import { EuiDataGridCell } from '../data_grid_cell'; -import { DataGridCellPopoverContext } from '../data_grid_cell_popover'; +import { EuiDataGridCell, DataGridCellPopoverContext } from '../cell'; import { EuiDataGridFooterRowProps } from '../../data_grid_types'; const renderEmpty = () => null; diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index a98630ce39a..39aaa9924b1 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -41,10 +41,7 @@ import { EuiDataGridInMemoryRenderer, } from './utils/in_memory'; import { useHeaderIsInteractive } from './body/header/header_is_interactive'; -import { - DataGridCellPopoverContext, - useCellPopover, -} from './body/data_grid_cell_popover'; +import { DataGridCellPopoverContext, useCellPopover } from './body/cell'; import { computeVisibleRows } from './utils/row_count'; import { EuiDataGridPaginationRenderer } from './utils/data_grid_pagination'; import { diff --git a/src/components/datagrid/utils/scrolling.tsx b/src/components/datagrid/utils/scrolling.tsx index 9e796d7384a..52ff7cd4831 100644 --- a/src/components/datagrid/utils/scrolling.tsx +++ b/src/components/datagrid/utils/scrolling.tsx @@ -16,7 +16,7 @@ import React, { } from 'react'; import { VariableSizeGrid as Grid } from 'react-window'; -import { DataGridCellPopoverContext } from '../body/data_grid_cell_popover'; +import { DataGridCellPopoverContext } from '../body/cell'; import { EuiDataGridStyle } from '../data_grid_types'; import { DataGridFocusContext } from './focus'; From d670e85e1c1a3863fb53175d840d25163d58c223 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 23 Nov 2023 10:56:49 -0800 Subject: [PATCH 02/20] Add new focus util --- .../datagrid/body/cell/focus_utils.test.tsx | 182 ++++++++++++++++++ .../datagrid/body/cell/focus_utils.tsx | 147 ++++++++++++++ 2 files changed, 329 insertions(+) create mode 100644 src/components/datagrid/body/cell/focus_utils.test.tsx create mode 100644 src/components/datagrid/body/cell/focus_utils.tsx diff --git a/src/components/datagrid/body/cell/focus_utils.test.tsx b/src/components/datagrid/body/cell/focus_utils.test.tsx new file mode 100644 index 00000000000..1dcbccacb07 --- /dev/null +++ b/src/components/datagrid/body/cell/focus_utils.test.tsx @@ -0,0 +1,182 @@ +/* + * 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 { fireEvent } from '@testing-library/react'; +import { render } from '../../../../test/rtl'; + +import { FocusTrappedChildren, HandleInteractiveChildren } from './focus_utils'; + +// Test util +const getCellWithInteractiveChildren = () => { + const cell = document.createElement('div'); + cell.setAttribute('tabindex', '0'); + cell.appendChild(document.createElement('button')); + cell.appendChild(document.createElement('button')); + return cell; +}; + +describe('HandleInteractiveChildren', () => { + describe('cell with interactive children', () => { + it('disables tabbing on all interactive children on mount', () => { + const cell = getCellWithInteractiveChildren(); + cell.querySelectorAll('button').forEach((button) => { + expect(button.getAttribute('tabindex')).toBeNull(); + }); + + render( + {}} + /> + ); + + cell.querySelectorAll('button').forEach((button) => { + expect(button.getAttribute('tabindex')).toEqual('-1'); + }); + }); + + it('calls `updateCellFocusContext` on child focus', () => { + const cell = getCellWithInteractiveChildren(); + + const updateCellFocusContext = jest.fn(); + render( + + ); + + fireEvent.focus(cell.querySelector('button')!); + expect(updateCellFocusContext).toHaveBeenCalled(); + }); + + it('renders a focus trap if `renderFocusTrap` is true', () => { + const cell = getCellWithInteractiveChildren(); + + const { container } = render( + {}} + renderFocusTrap={true} + /> + ); + + expect( + container.querySelector('[data-focus-lock-disabled]') + ).toBeInTheDocument(); + }); + + it('does not render a focus trap if `renderFocusTrap` is falsy', () => { + const cell = getCellWithInteractiveChildren(); + + const { container } = render( + {}} + renderFocusTrap={false} + /> + ); + + expect( + container.querySelector('[data-focus-lock-disabled]') + ).not.toBeInTheDocument(); + }); + }); + + describe('cell without any interactive children', () => { + it('never renders a focus trap', () => { + const cell = document.createElement('div'); + + const { container } = render( + {}} + renderFocusTrap={true} + /> + ); + + expect( + container.querySelector('[data-focus-lock-disabled]') + ).not.toBeInTheDocument(); + }); + + it('still calls `updateCellFocusContext` if the cell itself is focused', () => { + const cell = document.createElement('div'); + + const updateCellFocusContext = jest.fn(); + render( + + ); + + fireEvent.focus(cell); + expect(updateCellFocusContext).toHaveBeenCalled(); + }); + }); +}); + +describe('FocusTrappedChildren', () => { + describe('on enter', () => { + it('enables the focus trap, all interactive children, and moves focus to the first focusable child', () => { + const cell = getCellWithInteractiveChildren(); + + const { container } = render(); + fireEvent.keyUp(cell, { key: 'Enter' }); + + expect( + container.querySelector('[data-focus-lock-disabled]') + ).toHaveAttribute('data-focus-lock-disabled', 'false'); + + expect(cell.querySelector('button')).toHaveAttribute('tabindex', '0'); + expect(cell.querySelector('button')).toHaveFocus(); + }); + + it('allows pressing F2 to enter as well', () => { + const cell = getCellWithInteractiveChildren(); + + render(); + fireEvent.keyUp(cell, { key: 'F2' }); + + expect(cell.querySelector('button')).toHaveFocus(); + }); + }); + + describe('on exit', () => { + // Mock requestAnimationFrame to run immediately + jest + .spyOn(window, 'requestAnimationFrame') + .mockImplementation((cb: Function) => cb()); + + it('disables the focus trap, all interactive children and moves focus to the cell wrapper', () => { + const cell = getCellWithInteractiveChildren(); + + const { container } = render(); + fireEvent.keyUp(cell, { key: 'Enter' }); + fireEvent.keyUp(cell, { key: 'Escape' }); + + expect( + container.querySelector('[data-focus-lock-disabled]') + ).toHaveAttribute('data-focus-lock-disabled', 'disabled'); + + expect(cell.querySelector('button')).toHaveAttribute('tabindex', '-1'); + expect(cell).toHaveFocus(); + }); + + it('does nothing if the cell is not entered', () => { + const cell = getCellWithInteractiveChildren(); + + render(); + fireEvent.keyUp(cell, { key: 'Escape' }); + + expect(cell).not.toHaveFocus(); + }); + }); +}); diff --git a/src/components/datagrid/body/cell/focus_utils.tsx b/src/components/datagrid/body/cell/focus_utils.tsx new file mode 100644 index 00000000000..6d80a472955 --- /dev/null +++ b/src/components/datagrid/body/cell/focus_utils.tsx @@ -0,0 +1,147 @@ +/* + * 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, { + PropsWithChildren, + FunctionComponent, + useEffect, + useState, +} from 'react'; +import { tabbable } from 'tabbable'; + +import { keys } from '../../../../services'; +import { EuiFocusTrap } from '../../../focus_trap'; + +/** + * This internal utility component is used by all cells, both header and body/footer cells. + * It always handles: + * 1. Removing any interactive children from keyboard tab order on cell mount + * 2. Listening for focus on any interactive children and updating the cell focus context + * + * It should *only* render focus traps for: + * 1. Header cells that are `actions: false` but still have interactive children + * 2. Body cells that are `isExpandable: false` but still have interactive children + */ +export const HandleInteractiveChildren: FunctionComponent< + PropsWithChildren & { + cellEl?: HTMLElement | null; + updateCellFocusContext: Function; + renderFocusTrap?: boolean; + } +> = ({ cellEl, children, updateCellFocusContext, renderFocusTrap }) => { + const [hasInteractiveChildren, setHasInteractiveChildren] = useState(false); + + // On mount, disable all interactive children + useEffect(() => { + if (cellEl) { + const interactiveChildren = disableInteractives(cellEl); + + if (renderFocusTrap) { + setHasInteractiveChildren(interactiveChildren!.length > 0); + } + } + }, [cellEl, renderFocusTrap]); + + // Ensure that any interactive children that are clicked update the latest cell focus context + useEffect(() => { + if (cellEl) { + const onFocus = () => updateCellFocusContext(); + cellEl.addEventListener('focus', onFocus, true); // useCapture listens for focus on children + return () => { + cellEl.removeEventListener('focus', onFocus, true); + }; + } + }, [cellEl, updateCellFocusContext]); + + if (!cellEl) return children; // Do nothing if cell has yet to mount or is unmounting + if (!renderFocusTrap) return children; // Cells with default actions / expansion popovers do not need to trap + if (!hasInteractiveChildren) return children; // No need to focus trap if no children are interactive + + return ( + {children} + ); +}; + +/** + * Cells with interactive children but no cell popover expansion should render a + * focus trap that can be entered with the Enter key, which cycles keyboard tabs + * through the cell contents only, and exited with the Escape key + */ +export const FocusTrappedChildren: FunctionComponent< + PropsWithChildren & { cellEl: HTMLElement } +> = ({ cellEl, children }) => { + const [isCellEntered, setIsCellEntered] = useState(false); + useEffect(() => { + if (isCellEntered) { + enableAndFocusInteractives(cellEl); + } else { + disableInteractives(cellEl); + } + }, [isCellEntered, cellEl]); + + useEffect(() => { + const onKeyUp = (event: KeyboardEvent) => { + switch (event.key) { + case keys.ENTER: + case keys.F2: + event.preventDefault(); + setIsCellEntered(true); + break; + + case keys.ESCAPE: + event.preventDefault(); + setIsCellEntered((isCellEntered) => { + if (isCellEntered === true) { + requestAnimationFrame(() => cellEl.focus()); // move focus to cell + return false; + } + return isCellEntered; + }); + break; + } + }; + cellEl.addEventListener('keyup', onKeyUp); + return () => { + cellEl.removeEventListener('keyup', onKeyUp); + }; + }, [cellEl]); + + return ( + setIsCellEntered(false)} + clickOutsideDisables={true} + > + {children} + + ); +}; + +/** + * Utility fns for managing child interactive tabIndex state + */ + +const disableInteractives = (cell: HTMLElement) => { + const interactives = tabbable(cell); + interactives.forEach((element) => { + element.setAttribute('data-euigrid-tab-managed', 'true'); + element.setAttribute('tabIndex', '-1'); + }); + return interactives; +}; + +const enableAndFocusInteractives = (cell: HTMLElement) => { + const interactives = cell.querySelectorAll('[data-euigrid-tab-managed]'); + interactives.forEach((element, i) => { + element.setAttribute('tabIndex', '0'); + if (i === 0) { + (element as HTMLElement).focus(); + } + }); + return interactives; +}; From a79faa38a374e0eadc07d284fdda25efb754a345 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 3 Jan 2024 13:56:13 -0800 Subject: [PATCH 03/20] [EuiDataGridCell] Replace focus logic with new focus util - all of the removed methods are now totally unnecessary and handled as-is by `HandleInteractiveChildren` --- .../datagrid/body/cell/data_grid_cell.tsx | 160 +++--------------- 1 file changed, 20 insertions(+), 140 deletions(-) diff --git a/src/components/datagrid/body/cell/data_grid_cell.tsx b/src/components/datagrid/body/cell/data_grid_cell.tsx index a4e0a87c712..11e469597c2 100644 --- a/src/components/datagrid/body/cell/data_grid_cell.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.tsx @@ -11,7 +11,6 @@ import React, { Component, ContextType, createRef, - FocusEvent, FunctionComponent, JSXElementConstructor, KeyboardEvent, @@ -20,10 +19,9 @@ import React, { ReactNode, } from 'react'; import { createPortal } from 'react-dom'; -import { tabbable } from 'tabbable'; + import { keys } from '../../../../services'; import { EuiScreenReaderOnly } from '../../../accessibility'; -import { EuiFocusTrap } from '../../../focus_trap'; import { EuiI18n } from '../../../i18n'; import { EuiTextBlockTruncate } from '../../../text_truncate'; import { hasResizeObserver } from '../../../observer/resize_observer/resize_observer'; @@ -44,6 +42,7 @@ import { } from './data_grid_cell_actions'; import { DefaultCellPopover } from './data_grid_cell_popover'; import { IS_JEST_ENVIRONMENT } from '../../../../utils'; +import { HandleInteractiveChildren } from './focus_utils'; const EuiDataGridCellContent: FunctionComponent< EuiDataGridCellValueProps & { @@ -173,38 +172,18 @@ export class EuiDataGridCell extends Component< static contextType = DataGridFocusContext; declare context: ContextType; - getInteractables = () => { - const tabbingRef = this.cellContentsRef; - - if (tabbingRef) { - return tabbingRef.querySelectorAll( - '[data-datagrid-interactable=true]' - ); - } - - return []; + updateCellFocusContext = () => { + this.context.setFocusedCell([ + this.props.colIndex, + this.props.visibleRowIndex, + ]); }; takeFocus = (preventScroll: boolean) => { const cell = this.cellRef.current; - - if (cell) { - // only update focus if we are not already focused on something in this cell - let element: Element | null = document.activeElement; - while (element != null && element !== cell) { - element = element.parentElement; - } - const doFocusUpdate = element !== cell; - - if (doFocusUpdate) { - const interactables = this.getInteractables(); - if (this.isExpandable() === false && interactables.length === 1) { - // Only one element can be interacted with - interactables[0].focus({ preventScroll }); - } else { - cell.focus({ preventScroll }); - } - } + // Only focus the cell if not already focused on something in the cell + if (cell && !cell.contains(document.activeElement)) { + cell.focus({ preventScroll }); } }; @@ -429,62 +408,9 @@ export class EuiDataGridCell extends Component< } else if (this.contentObserver) { this.contentObserver.disconnect(); } - this.preventTabbing(); this.setCellTextAlign(); }; - onFocus = (e: FocusEvent) => { - // only perform this logic when the event's originating element (e.target) is - // the wrapping element with the onFocus logic - // reasons: - // * the outcome is only meaningful when the focus shifts to the wrapping element - // * if the cell children include portalled content React will bubble the focus - // event up, which can trigger the focus() call below, causing focus lock fighting - if (this.cellRef.current === e.target) { - const { colIndex, visibleRowIndex } = this.props; - // focus in next tick to give potential focus capturing mechanisms time to release their traps - // also clear any previous focus timeout that may still be queued - if (EuiDataGridCell.activeFocusTimeoutId) { - window.clearTimeout(EuiDataGridCell.activeFocusTimeoutId); - } - EuiDataGridCell.activeFocusTimeoutId = this.focusTimeout = - window.setTimeout(() => { - this.context.setFocusedCell([colIndex, visibleRowIndex]); - - const interactables = this.getInteractables(); - if (interactables.length === 1 && this.isExpandable() === false) { - interactables[0].focus(); - this.setState({ disableCellTabIndex: true }); - } - }, 0); - } - }; - - onBlur = () => { - this.setState({ disableCellTabIndex: false }); - }; - - preventTabbing = () => { - if (this.cellContentsRef) { - const tabbables = tabbable(this.cellContentsRef); - for (let i = 0; i < tabbables.length; i++) { - const element = tabbables[i]; - element.setAttribute('tabIndex', '-1'); - element.setAttribute('data-datagrid-interactable', 'true'); - } - } - }; - - enableTabbing = () => { - if (this.cellContentsRef) { - const interactables = this.getInteractables(); - for (let i = 0; i < interactables.length; i++) { - const element = interactables[i]; - element.removeAttribute('tabIndex'); - } - } - }; - setCellTextAlign = () => { if (this.cellContentsRef) { const { columnType } = this.props; @@ -659,48 +585,6 @@ export class EuiDataGridCell extends Component< openCellPopover({ rowIndex: visibleRowIndex, colIndex }); break; } - } else { - if ( - event.key === keys.ENTER || - event.key === keys.F2 || - event.key === keys.ESCAPE - ) { - const interactables = this.getInteractables(); - if (interactables.length >= 2) { - switch (event.key) { - case keys.ENTER: - // `Enter` only activates the trap - if (this.state.isEntered === false) { - this.enableTabbing(); - this.setState({ isEntered: true }); - - // result of this keypress is focus shifts to the first interactive element - // and then the browser fires the onClick event because that's how [Enter] works - // so we need to prevent that default action otherwise entering the trap triggers the first element - event.preventDefault(); - } - break; - case keys.F2: - // toggle interactives' focus trap - this.setState(({ isEntered }) => { - if (isEntered) { - this.preventTabbing(); - } else { - this.enableTabbing(); - } - return { isEntered: !isEntered }; - }); - break; - case keys.ESCAPE: - // `Escape` only de-activates the trap - this.preventTabbing(); - if (this.state.isEntered === true) { - this.setState({ isEntered: false }); - } - break; - } - } - } } }; @@ -751,19 +635,17 @@ export class EuiDataGridCell extends Component< ); - const cellContent = isExpandable ? ( - - ) : ( - { - this.setState({ isEntered: false }, this.preventTabbing); - }} - clickOutsideDisables={true} + const cellContent = ( + - - + + ); const cell = ( @@ -782,14 +664,12 @@ export class EuiDataGridCell extends Component< data-gridcell-row-index={this.props.rowIndex} // Index from data, not affected by sorting or pagination data-gridcell-visible-row-index={this.props.visibleRowIndex} // Affected by sorting & pagination onKeyDown={handleCellKeyDown} - onFocus={this.onFocus} onMouseEnter={() => { this.setState({ enableInteractions: true }); }} onMouseLeave={() => { this.setState({ enableInteractions: false }); }} - onBlur={this.onBlur} > {cellContent} From 3c04c7535a3aeeb2e922a5637a719fed24d60280 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 3 Jan 2024 13:57:34 -0800 Subject: [PATCH 04/20] [EuiDataGridCell] Continue removing now-unnecessary focus timeouts & state + misc imports reorder/reorg --- .../body/cell/data_grid_cell.test.tsx | 15 ------- .../datagrid/body/cell/data_grid_cell.tsx | 41 ++----------------- src/components/datagrid/data_grid_types.ts | 3 -- 3 files changed, 4 insertions(+), 55 deletions(-) diff --git a/src/components/datagrid/body/cell/data_grid_cell.test.tsx b/src/components/datagrid/body/cell/data_grid_cell.test.tsx index 58ef5dea8a4..0fc0b91632a 100644 --- a/src/components/datagrid/body/cell/data_grid_cell.test.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.test.tsx @@ -184,26 +184,11 @@ describe('EuiDataGridCell', () => { component.setState({ cellProps: {} }); }); }); - it('isEntered', () => { - act(() => { - component.setState({ isEntered: true }); - }); - }); it('isFocused', () => { act(() => { component.setState({ isFocused: true }); }); }); - it('enableInteractions', () => { - act(() => { - component.setState({ enableInteractions: true }); - }); - }); - it('disableCellTabIndex', () => { - act(() => { - component.setState({ disableCellTabIndex: true }); - }); - }); }); }); diff --git a/src/components/datagrid/body/cell/data_grid_cell.tsx b/src/components/datagrid/body/cell/data_grid_cell.tsx index 11e469597c2..092212d9c43 100644 --- a/src/components/datagrid/body/cell/data_grid_cell.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.tsx @@ -20,11 +20,13 @@ import React, { } from 'react'; import { createPortal } from 'react-dom'; +import { IS_JEST_ENVIRONMENT } from '../../../../utils'; import { keys } from '../../../../services'; import { EuiScreenReaderOnly } from '../../../accessibility'; import { EuiI18n } from '../../../i18n'; import { EuiTextBlockTruncate } from '../../../text_truncate'; import { hasResizeObserver } from '../../../observer/resize_observer/resize_observer'; + import { DataGridFocusContext } from '../../utils/focus'; import { RowHeightVirtualizationUtils } from '../../utils/row_heights'; import { @@ -41,7 +43,6 @@ import { EuiDataGridCellPopoverActions, } from './data_grid_cell_actions'; import { DefaultCellPopover } from './data_grid_cell_popover'; -import { IS_JEST_ENVIRONMENT } from '../../../../utils'; import { HandleInteractiveChildren } from './focus_utils'; const EuiDataGridCellContent: FunctionComponent< @@ -142,17 +143,6 @@ export class EuiDataGridCell extends Component< EuiDataGridCellProps, EuiDataGridCellState > { - // focus tracking is split between the entire grid & individual cells, - // the parent grid owns which cell is focused, - // but individual cells need to react to changes and also report that - // they are focused in response to user actions like clicking on the cell - // to avoid focus trap fighting, cells wait a tick after being clicked to allow - // any existing traps to disconnect before the cell reports the new focus state to the parent grid - // but because of this small delay, multiple cells could queue up focus and - // create an infinite loop as the cells activate->deactivate->... - // so we track the last timeout id and clear that request if superseded - static activeFocusTimeoutId: number | undefined = undefined; - cellRef = createRef() as MutableRefObject; contentObserver!: any; // Cell Content ResizeObserver popoverAnchorRef = createRef() as MutableRefObject; @@ -160,13 +150,9 @@ export class EuiDataGridCell extends Component< state: EuiDataGridCellState = { cellProps: {}, isFocused: false, - isEntered: false, - enableInteractions: false, - disableCellTabIndex: false, cellTextAlign: 'Left', }; unsubscribeCell?: Function; - focusTimeout: number | undefined; style = null; static contextType = DataGridFocusContext; @@ -284,7 +270,6 @@ export class EuiDataGridCell extends Component< }; componentWillUnmount() { - window.clearTimeout(this.focusTimeout); if (this.unsubscribeCell) { this.unsubscribeCell(); } @@ -383,12 +368,7 @@ export class EuiDataGridCell extends Component< } if (nextState.cellProps !== this.state.cellProps) return true; - if (nextState.isEntered !== this.state.isEntered) return true; if (nextState.isFocused !== this.state.isFocused) return true; - if (nextState.enableInteractions !== this.state.enableInteractions) - return true; - if (nextState.disableCellTabIndex !== this.state.disableCellTabIndex) - return true; return false; } @@ -531,11 +511,6 @@ export class EuiDataGridCell extends Component< const isExpandable = this.isExpandable(); const popoverIsOpen = this.isPopoverOpen(); - const showCellActions = - this.state.isFocused || - this.state.isEntered || - this.state.enableInteractions || - popoverIsOpen; const cellClasses = classNames( 'euiDataGridRowCell', @@ -611,7 +586,7 @@ export class EuiDataGridCell extends Component< ariaRowIndex, }; - const cellActions = showCellActions && ( + const cellActions = ( <> { - this.setState({ enableInteractions: true }); - }} - onMouseLeave={() => { - this.setState({ enableInteractions: false }); - }} > {cellContent} diff --git a/src/components/datagrid/data_grid_types.ts b/src/components/datagrid/data_grid_types.ts index 004e0d2f2a4..f8cadd2308c 100644 --- a/src/components/datagrid/data_grid_types.ts +++ b/src/components/datagrid/data_grid_types.ts @@ -629,9 +629,6 @@ export interface EuiDataGridCellProps { export interface EuiDataGridCellState { cellProps: EuiDataGridSetCellProps; isFocused: boolean; // tracks if this cell has focus or not, used to enable tabIndex on the cell - isEntered: boolean; // enables focus trap for non-expandable cells with multiple interactive elements - enableInteractions: boolean; // cell got hovered at least once, so cell button and popover interactions are rendered - disableCellTabIndex: boolean; // disables tabIndex on the wrapping cell, used for focus management of a single interactive child cellTextAlign: 'Left' | 'Right'; // determines the cell actions and cell popover expansion position } From 21ae96c8743e1784998761f8d44781bab16806ca Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 4 Jan 2024 13:45:51 -0800 Subject: [PATCH 05/20] [downstream tests] Update tests to account for cellActions change - cell actions are rendered in the DOM, just not visible, which saves us from having to add a bunch of extra hover/focus state trackers --- .../datagrid/body/cell/data_grid_cell.tsx | 6 ++--- .../body/cell/data_grid_cell_popover.spec.tsx | 24 ++++++++++++++----- .../body/footer/data_grid_footer_row.spec.tsx | 6 ++--- src/components/datagrid/data_grid.test.tsx | 24 ++++++++----------- 4 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/components/datagrid/body/cell/data_grid_cell.tsx b/src/components/datagrid/body/cell/data_grid_cell.tsx index 092212d9c43..7166d75e3ee 100644 --- a/src/components/datagrid/body/cell/data_grid_cell.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.tsx @@ -586,7 +586,7 @@ export class EuiDataGridCell extends Component< ariaRowIndex, }; - const cellActions = ( + const cellActions = isExpandable ? ( <> - ); + ) : undefined; const cellContent = ( ); diff --git a/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx b/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx index cf02b7a1eee..a1bd45cdd36 100644 --- a/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx @@ -53,7 +53,9 @@ describe('EuiDataGridCellPopover', () => { '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]') + .filter(':visible') + .click(); cy.focused().should( 'have.attr', 'data-test-subj', @@ -73,7 +75,9 @@ describe('EuiDataGridCellPopover', () => { '[data-gridcell-row-index="1"][data-gridcell-column-index="1"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]') + .filter(':visible') + .click(); cy.focused().should( 'have.attr', 'data-test-subj', @@ -108,7 +112,9 @@ describe('EuiDataGridCellPopover', () => { }); it('when the cell expand action button is clicked', () => { - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]') + .filter(':visible') + .click(); cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should( 'not.exist' ); @@ -150,8 +156,12 @@ describe('EuiDataGridCellPopover', () => { cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist'); // Close and re-open the cell popover by clicking - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]') + .filter(':visible') + .click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]') + .filter(':visible') + .click(); cy.get('[data-test-subj="cellActionB"]').first().realClick(); cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist'); @@ -191,7 +201,9 @@ describe('EuiDataGridCellPopover', () => { cy.get( '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]') + .filter(':visible') + .click(); cy.get('.euiDataGridRowCell__popover.hello.world').should('exist'); }); diff --git a/src/components/datagrid/body/footer/data_grid_footer_row.spec.tsx b/src/components/datagrid/body/footer/data_grid_footer_row.spec.tsx index 5dc1a7b7f3f..dddd82bb296 100644 --- a/src/components/datagrid/body/footer/data_grid_footer_row.spec.tsx +++ b/src/components/datagrid/body/footer/data_grid_footer_row.spec.tsx @@ -65,9 +65,9 @@ describe('EuiDataGridFooterRow', () => { cy.get( '[data-gridcell-column-index="0"][data-gridcell-row-index="3"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').should( - 'not.exist' - ); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]') + .filter(':visible') + .should('not.exist'); }); // Regression test for #5720 diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index 2eb963f13dd..af7adc821b3 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -1329,7 +1329,7 @@ describe('EuiDataGrid', () => { const component = mount( {}, @@ -2177,21 +2177,17 @@ describe('EuiDataGrid', () => { /> ); - // cell buttons should not get rendered for unfocused, unhovered cell - expect(findTestSubject(component, 'alertAction').exists()).toBe(false); - expect(findTestSubject(component, 'happyAction').exists()).toBe(false); - - act(() => { - findTestSubject(component, 'dataGridRowCell') - .at(1) - .prop('onMouseEnter')!({} as React.MouseEvent); - }); - - component.update(); + // cell buttons should be `display: none` for unfocused, unhovered cell + expect( + findTestSubject(component, 'alertAction').last().getDOMNode() + ).not.toBeVisible(); + expect( + findTestSubject(component, 'happyAction').last().getDOMNode() + ).not.toBeVisible(); - findTestSubject(component, 'alertAction').at(0).simulate('click'); + findTestSubject(component, 'alertAction').at(1).simulate('click'); expect(alertFn).toHaveBeenCalledWith(1, 'A'); - findTestSubject(component, 'happyAction').at(0).simulate('click'); + findTestSubject(component, 'happyAction').at(1).simulate('click'); expect(happyFn).toHaveBeenCalledWith(1, 'A'); alertFn.mockReset(); happyFn.mockReset(); From 1845f5a1f7f673f6a0888c62068019fb872d761b Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 3 Jan 2024 14:17:49 -0800 Subject: [PATCH 06/20] [EuiDataGridCell] Remove Jest focus tests in favor of Cypress ones --- .../data_grid_cell.test.tsx.snap | 24 +++- .../body/cell/data_grid_cell.test.tsx | 72 +--------- .../datagrid/body/cell/focus_utils.spec.tsx | 128 ++++++++++++++++++ 3 files changed, 152 insertions(+), 72 deletions(-) create mode 100644 src/components/datagrid/body/cell/focus_utils.spec.tsx diff --git a/src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap b/src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap index d06990806c9..c455eda3dfe 100644 --- a/src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap +++ b/src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap @@ -55,13 +55,11 @@ exports[`EuiDataGridCell renders 1`] = `
@@ -74,5 +72,27 @@ exports[`EuiDataGridCell renders 1`] = ` - someColumn, column 1, row 1

+
+ +
+
`; diff --git a/src/components/datagrid/body/cell/data_grid_cell.test.tsx b/src/components/datagrid/body/cell/data_grid_cell.test.tsx index 0fc0b91632a..1e3db47cb5b 100644 --- a/src/components/datagrid/body/cell/data_grid_cell.test.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.test.tsx @@ -9,7 +9,6 @@ import React, { useEffect } from 'react'; import { mount, ReactWrapper } from 'enzyme'; import { act } from '@testing-library/react'; -import { keys } from '../../../../services'; import { render } from '../../../../test/rtl'; import { RowHeightUtils } from '../../utils/__mocks__/row_heights'; import { mockFocusContext } from '../../utils/__mocks__/focus_context'; @@ -636,75 +635,6 @@ describe('EuiDataGridCell', () => { }); }); - // TODO: Test interacting/focus/tabbing in Cypress instead of Jest - describe('interactions', () => { - describe('keyboard events', () => { - it('when cell is expandable', () => { - const component = mount(); - const preventDefault = jest.fn(); - - component.simulate('keyDown', { preventDefault, key: keys.ENTER }); - component.simulate('keyDown', { preventDefault, key: keys.F2 }); - - expect(mockPopoverContext.openCellPopover).toHaveBeenCalledWith({ - rowIndex: 0, - colIndex: 0, - }); - expect(mockPopoverContext.openCellPopover).toHaveBeenCalledTimes(2); - - // If the cell popover is open, the nothing should happen - jest.clearAllMocks(); - component.setProps({ - popoverContext: { ...mockPopoverContext, popoverIsOpen: true }, - }); - - component.simulate('keyDown', { preventDefault, key: keys.ENTER }); - component.simulate('keyDown', { preventDefault, key: keys.F2 }); - - expect(mockPopoverContext.openCellPopover).not.toHaveBeenCalled(); - }); - - it('when cell is not expandable', () => { - const component = mount( - - ); - const preventDefault = jest.fn(); - - component.simulate('keyDown', { preventDefault, key: keys.ENTER }); - // TODO: Assert that tabbing should be enabled - expect(component.state('isEntered')).toEqual(true); - - component.simulate('keyDown', { preventDefault, key: keys.F2 }); - // TODO: Assert that tabbing should be prevented - expect(component.state('isEntered')).toEqual(false); - - component.simulate('keyDown', { preventDefault, key: keys.F2 }); - // TODO: Assert that tabbing should be enabled - expect(component.state('isEntered')).toEqual(true); - - component.simulate('keyDown', { preventDefault, key: keys.ENTER }); - component.simulate('keyDown', { preventDefault, key: keys.ESCAPE }); - // TODO: Assert that tabbing should be prevented - expect(component.state('isEntered')).toEqual(false); - }); - }); - - it('mouse events', () => { - const component = mount(); - component.simulate('mouseEnter'); - expect(component.state('enableInteractions')).toEqual(true); - component.simulate('mouseLeave'); - expect(component.state('enableInteractions')).toEqual(false); - }); - - it('focus/blur events', () => { - const component = mount(); - component.simulate('focus'); - component.simulate('blur'); - expect(component.state('disableCellTabIndex')).toEqual(false); - }); - }); - describe('renders certain classes/styles based on rowHeightOptions', () => { const props = { ...requiredProps, renderCellValue: () => null }; @@ -762,4 +692,6 @@ describe('EuiDataGridCell', () => { expect(component.find('.euiTextBlockTruncate').exists()).toBe(true); }); }); + + // Note: Tests for cell interactivity (focus, tabbing, etc) are in `focus_utils.spec.tsx` }); diff --git a/src/components/datagrid/body/cell/focus_utils.spec.tsx b/src/components/datagrid/body/cell/focus_utils.spec.tsx new file mode 100644 index 00000000000..780930884be --- /dev/null +++ b/src/components/datagrid/body/cell/focus_utils.spec.tsx @@ -0,0 +1,128 @@ +/* + * 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 { EuiDataGrid } from '../../data_grid'; + +describe('Cell focus utils', () => { + const baseProps = { + 'aria-label': 'Test', + width: 300, + rowCount: 1, + renderCellValue: () => 'cell', + columnVisibility: { + setVisibleColumns: () => {}, + visibleColumns: ['column'], + }, + }; + + const interactiveChildren = ( + <> + + + + ); + + describe('does not render a focus trap', () => { + const props = { + ...baseProps, + columns: [{ id: 'column', isExpandable: true, actions: {} }], + }; + + it('when body cells are expandable', () => { + cy.mount(); + + const cell = '[data-test-subj="dataGridRowCell"]'; + const cellAction = '[data-test-subj="euiDataGridCellExpandButton"]'; + const cellPopover = '[data-test-subj="euiDataGridExpansionPopover"]'; + + // Should toggle the cell expansion popover instead + cy.get(cell).click(); + cy.get(cellAction).should('be.visible'); + cy.get(cellAction).realClick(); + cy.get(cellPopover).should('be.visible'); + + // Keyboard behavior + cy.realPress('Escape'); + cy.get(cellPopover).should('not.exist'); + cy.realPress('Enter'); + cy.get(cellPopover).should('exist'); + }); + }); + + describe('renders a focus trap', () => { + it('when body cells are not expandable', () => { + cy.mount( + interactiveChildren} + /> + ); + // Enter the trap + cy.get('[data-test-subj="dataGridRowCell"]').realClick(); + cy.realPress('Enter'); + + // Should cycle through focus trap + cy.focused().should('have.attr', 'data-test-subj', 'interactiveChildA'); + cy.realPress('Tab'); + cy.focused().should('have.attr', 'data-test-subj', 'interactiveChildB'); + cy.realPress('Tab'); + cy.focused().should('have.attr', 'data-test-subj', 'interactiveChildA'); + + // Exit the trap + cy.realPress('Escape'); + cy.focused().should('have.attr', 'data-test-subj', 'dataGridRowCell'); + }); + }); + + describe('focus context', () => { + it('updates the cell focus context if interactive cell children are clicked', () => { + cy.realMount( + <> + {}, + visibleColumns: ['A', 'B'], + }} + columns={[ + { + id: 'A', + isExpandable: true, + }, + { + id: 'B', + isExpandable: false, + }, + ]} + renderCellValue={() => interactiveChildren} + /> + + + ); + + cy.repeatRealPress('Tab', 4); + cy.focused() + .parent() + .should('have.attr', 'data-gridcell-row-index', '-1') + .should('have.attr', 'data-gridcell-column-index', '0'); + + cy.get('[data-test-subj="interactiveChildB"]').last().realClick(); + cy.realPress('Tab'); + cy.realPress(['Shift', 'Tab']); + cy.focused() + .should('have.attr', 'data-gridcell-row-index', '0') + .should('have.attr', 'data-gridcell-column-index', '1'); + }); + }); +}); From fdb932abcef39e7dab74292e3c3265e3d8058b7a Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 4 Jan 2024 11:48:22 -0800 Subject: [PATCH 07/20] [EuiDataGridHeaderCell] Replace focus logic with new util - all of the removed methods are now totally unnecessary and handled as-is by `HandleInteractiveChildren --- .../header/data_grid_header_cell.test.tsx | 1 - .../body/header/data_grid_header_cell.tsx | 5 +- .../data_grid_header_cell_wrapper.test.tsx | 46 ++++++ .../header/data_grid_header_cell_wrapper.tsx | 136 +++--------------- 4 files changed, 70 insertions(+), 118 deletions(-) diff --git a/src/components/datagrid/body/header/data_grid_header_cell.test.tsx b/src/components/datagrid/body/header/data_grid_header_cell.test.tsx index 109489fedc3..afe7466d882 100644 --- a/src/components/datagrid/body/header/data_grid_header_cell.test.tsx +++ b/src/components/datagrid/body/header/data_grid_header_cell.test.tsx @@ -255,7 +255,6 @@ describe('EuiDataGridHeaderCell', () => { fireEvent.click(toggle); waitForEuiPopoverOpen(); - expect(mockFocusContext.setFocusedCell).toHaveBeenCalledWith([0, -1]); fireEvent.click(toggle); waitForEuiPopoverClose(); diff --git a/src/components/datagrid/body/header/data_grid_header_cell.tsx b/src/components/datagrid/body/header/data_grid_header_cell.tsx index beaf8936913..1a2ff0592c9 100644 --- a/src/components/datagrid/body/header/data_grid_header_cell.tsx +++ b/src/components/datagrid/body/header/data_grid_header_cell.tsx @@ -139,10 +139,7 @@ export const EuiDataGridHeaderCell: FunctionComponent< <>
+ } + renderFocusTrap={false} + updateCellFocusContext={[Function]} > + +
+
+ +
+
+
+ +
+
+
+ +
+
+
+ +
+
+
+ +
+
@@ -1041,7 +1173,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
-
-
- 0 -
- + 0
-
+
+
+ +
+
+
+ +
+
-
-
- 0 -
- + 0
-
+
-
-
- 1 -
- + 1
-
+
+
+ +
+
+
+ +
+
-
-
- 1 -
- + 1
-
+
-
-
- 2 -
- + 2
-
+
+
+ +
+
+
+ +
+
-
-
- 2 -
- + 2
-
+
@@ -1724,7 +1904,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = ` role="row" >
+
+ +
+
+
+ +
+
+
+ +
+
+
+ +
+
+
+ +
+
+
+ +
+
@@ -2150,7 +2462,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` role="row" >
+
+ +
+
+
+ +
+
+
+ +
+
+
+ +
+
+
+ +
+
+
+ +
+
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap index 2a9d732b062..064c0d43817 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap @@ -10,7 +10,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render role="row" >
+
+ +
+
+
+ +
+
+
+ +
+
+
+ +
+
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap index c13007e730b..7a16c48647e 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap @@ -14,7 +14,7 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = ` role="row" >
+
+ +
+
+
+ +
+
diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index 5c8cc75505c..49120c60c4d 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -546,11 +546,7 @@ describe('EuiDataGrid', () => { "data-gridcell-row-index": 0, "data-gridcell-visible-row-index": 0, "data-test-subj": "dataGridRowCell", - "onBlur": [Function], - "onFocus": [Function], "onKeyDown": [Function], - "onMouseEnter": [Function], - "onMouseLeave": [Function], "role": "gridcell", "style": Object { "color": "red", @@ -572,11 +568,7 @@ describe('EuiDataGrid', () => { "data-gridcell-row-index": 0, "data-gridcell-visible-row-index": 0, "data-test-subj": "dataGridRowCell", - "onBlur": [Function], - "onFocus": [Function], "onKeyDown": [Function], - "onMouseEnter": [Function], - "onMouseLeave": [Function], "role": "gridcell", "style": Object { "color": "blue", @@ -598,11 +590,7 @@ describe('EuiDataGrid', () => { "data-gridcell-row-index": 1, "data-gridcell-visible-row-index": 1, "data-test-subj": "dataGridRowCell", - "onBlur": [Function], - "onFocus": [Function], "onKeyDown": [Function], - "onMouseEnter": [Function], - "onMouseLeave": [Function], "role": "gridcell", "style": Object { "color": "red", @@ -624,11 +612,7 @@ describe('EuiDataGrid', () => { "data-gridcell-row-index": 1, "data-gridcell-visible-row-index": 1, "data-test-subj": "dataGridRowCell", - "onBlur": [Function], - "onFocus": [Function], "onKeyDown": [Function], - "onMouseEnter": [Function], - "onMouseLeave": [Function], "role": "gridcell", "style": Object { "color": "blue", From 292b7d89f73dfb7f5cc9c0539f992cb1e26303d8 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 4 Jan 2024 14:02:07 -0800 Subject: [PATCH 16/20] changelog --- changelogs/upcoming/7448.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelogs/upcoming/7448.md diff --git a/changelogs/upcoming/7448.md b/changelogs/upcoming/7448.md new file mode 100644 index 00000000000..bc8e1e3eecc --- /dev/null +++ b/changelogs/upcoming/7448.md @@ -0,0 +1,6 @@ +**Accessibility** + +- `EuiDataGrid`'s keyboard/screenreader experience has been tweaked to be more consistent for varying complex data: + - Headers are now always navigable by arrow key, regardless of whether the header cells contain interactive content + - Non-expandable cells containing any amount of interactive content now must be entered via Enter or F2 keypress + - Expandable cells continue to be toggled via Enter or F2 keypress From 0bcef9bcb6b45c7039f94ff13452928c910e5597 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 4 Jan 2024 14:08:29 -0800 Subject: [PATCH 17/20] Typescript lint workaround - my local vscode doesn't complain about this, but CLI does - not totally sure why or if this makes sense :| --- src/components/datagrid/body/cell/focus_utils.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/datagrid/body/cell/focus_utils.tsx b/src/components/datagrid/body/cell/focus_utils.tsx index 6d80a472955..77b655eb6cb 100644 --- a/src/components/datagrid/body/cell/focus_utils.tsx +++ b/src/components/datagrid/body/cell/focus_utils.tsx @@ -11,6 +11,7 @@ import React, { FunctionComponent, useEffect, useState, + useMemo, } from 'react'; import { tabbable } from 'tabbable'; @@ -58,9 +59,10 @@ export const HandleInteractiveChildren: FunctionComponent< } }, [cellEl, updateCellFocusContext]); - if (!cellEl) return children; // Do nothing if cell has yet to mount or is unmounting - if (!renderFocusTrap) return children; // Cells with default actions / expansion popovers do not need to trap - if (!hasInteractiveChildren) return children; // No need to focus trap if no children are interactive + const _children = useMemo(() => <>{children}, [children]); + if (!cellEl) return _children; // Do nothing if cell has yet to mount or is unmounting + if (!renderFocusTrap) return _children; // Cells with default actions / expansion popovers do not need to trap + if (!hasInteractiveChildren) return _children; // No need to focus trap if no children are interactive return ( {children} From 41ed92e76fbb5bcbc7988ba7dce679d07562fb57 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 4 Jan 2024 15:24:30 -0800 Subject: [PATCH 18/20] Add screen reader text for Enter key behavior --- changelogs/upcoming/7448.md | 1 + .../__snapshots__/data_grid.test.tsx.snap | 48 +++++++++++++++++++ .../data_grid_body_custom.test.tsx.snap | 8 ++++ .../data_grid_body_virtualized.test.tsx.snap | 4 ++ .../data_grid_cell.test.tsx.snap | 2 + .../datagrid/body/cell/data_grid_cell.tsx | 9 ++++ .../datagrid/body/cell/focus_utils.test.tsx | 17 +++++++ .../datagrid/body/cell/focus_utils.tsx | 13 +++++ 8 files changed, 102 insertions(+) diff --git a/changelogs/upcoming/7448.md b/changelogs/upcoming/7448.md index bc8e1e3eecc..ac918fe177b 100644 --- a/changelogs/upcoming/7448.md +++ b/changelogs/upcoming/7448.md @@ -4,3 +4,4 @@ - Headers are now always navigable by arrow key, regardless of whether the header cells contain interactive content - Non-expandable cells containing any amount of interactive content now must be entered via Enter or F2 keypress - Expandable cells continue to be toggled via Enter or F2 keypress +- `EuiDataGrid` now provides a direct screen reader hint for Enter key behavior for expandable & interactive cells diff --git a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap index 4327dfe50c4..cfbf6e00f0d 100644 --- a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap +++ b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap @@ -725,6 +725,8 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` > - A, column 1, row 1 + . + Press the Enter key to expand this cell.

- B, column 2, row 1 + . + Press the Enter key to expand this cell.

- A, column 1, row 2 + . + Press the Enter key to expand this cell.

- B, column 2, row 2 + . + Press the Enter key to expand this cell.

- A, column 1, row 3 + . + Press the Enter key to expand this cell.

- B, column 2, row 3 + . + Press the Enter key to expand this cell.

- A, column 2, row 1 + . + Press the Enter key to expand this cell.

- B, column 3, row 1 + . + Press the Enter key to expand this cell.

- A, column 2, row 2 + . + Press the Enter key to expand this cell.

- B, column 3, row 2 + . + Press the Enter key to expand this cell.

- A, column 2, row 3 + . + Press the Enter key to expand this cell.

- B, column 3, row 3 + . + Press the Enter key to expand this cell.

- A, column 1, row 1 + . + Press the Enter key to expand this cell.

- B, column 2, row 1 + . + Press the Enter key to expand this cell.

- A, column 1, row 2 + . + Press the Enter key to expand this cell.

- B, column 2, row 2 + . + Press the Enter key to expand this cell.

- A, column 1, row 3 + . + Press the Enter key to expand this cell.

- B, column 2, row 3 + . + Press the Enter key to expand this cell.

- A, column 1, row 1 + . + Press the Enter key to expand this cell.

- B, column 2, row 1 + . + Press the Enter key to expand this cell.

- A, column 1, row 2 + . + Press the Enter key to expand this cell.

- B, column 2, row 2 + . + Press the Enter key to expand this cell.

- A, column 1, row 3 + . + Press the Enter key to expand this cell.

- B, column 2, row 3 + . + Press the Enter key to expand this cell.

- columnA, column 1, row 1 + . + Press the Enter key to expand this cell.

- columnB, column 2, row 1 + . + Press the Enter key to expand this cell.

- columnA, column 1, row 2 + . + Press the Enter key to expand this cell.

- columnB, column 2, row 2 + . + Press the Enter key to expand this cell.

- columnA, column 1, row 1 + . + Press the Enter key to expand this cell.

- columnB, column 2, row 1 + . + Press the Enter key to expand this cell.

- someColumn, column 1, row 1 + . + Press the Enter key to expand this cell.

+ {cellActions && ( + <> + {'. '} + + + )}

); diff --git a/src/components/datagrid/body/cell/focus_utils.test.tsx b/src/components/datagrid/body/cell/focus_utils.test.tsx index 1dcbccacb07..a35407d00dc 100644 --- a/src/components/datagrid/body/cell/focus_utils.test.tsx +++ b/src/components/datagrid/body/cell/focus_utils.test.tsx @@ -124,6 +124,23 @@ describe('HandleInteractiveChildren', () => { }); describe('FocusTrappedChildren', () => { + it('renders a screen reader hint', () => { + const cell = getCellWithInteractiveChildren(); + const { container } = render(); + expect(container.childNodes[1]).toMatchInlineSnapshot(` +
+

+ - + Press the Enter key to interact with this cell's contents. +

+
+ `); + }); + describe('on enter', () => { it('enables the focus trap, all interactive children, and moves focus to the first focusable child', () => { const cell = getCellWithInteractiveChildren(); diff --git a/src/components/datagrid/body/cell/focus_utils.tsx b/src/components/datagrid/body/cell/focus_utils.tsx index 77b655eb6cb..4d425244f83 100644 --- a/src/components/datagrid/body/cell/focus_utils.tsx +++ b/src/components/datagrid/body/cell/focus_utils.tsx @@ -17,6 +17,8 @@ import { tabbable } from 'tabbable'; import { keys } from '../../../../services'; import { EuiFocusTrap } from '../../../focus_trap'; +import { EuiScreenReaderOnly } from '../../../accessibility'; +import { EuiI18n } from '../../../i18n'; /** * This internal utility component is used by all cells, both header and body/footer cells. @@ -120,6 +122,17 @@ export const FocusTrappedChildren: FunctionComponent< clickOutsideDisables={true} > {children} + + +

+ {' - '} + +

+ ); }; From d6c70f6cd0e7d7c1082d9ba888910e9533b144eb Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 5 Jan 2024 10:42:14 -0800 Subject: [PATCH 19/20] [PR feedback] Fix focus fighting bug + add E2E regression test for behavior --- .../data_grid_header_cell_wrapper.test.tsx | 2 ++ .../header/data_grid_header_cell_wrapper.tsx | 14 ++++++++- src/components/datagrid/data_grid.spec.tsx | 31 ++++++++++++++++--- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx index 3d4fd903bbe..a1ed1e2a62f 100644 --- a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx +++ b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx @@ -52,6 +52,7 @@ describe('EuiDataGridHeaderCellWrapper', () => { data-gridcell-row-index="-1" data-gridcell-visible-row-index="-1" data-test-subj="dataGridHeaderCell-someColumn" + onFocus={[Function]} role="columnheader" style={Object {}} tabIndex={0} @@ -100,6 +101,7 @@ describe('EuiDataGridHeaderCellWrapper', () => { data-gridcell-row-index="-1" data-gridcell-visible-row-index="-1" data-test-subj="dataGridHeaderCell-someColumn" + onFocus={[Function]} role="columnheader" style={ Object { diff --git a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx index fc969865f61..fde48ab0620 100644 --- a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx +++ b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx @@ -9,6 +9,7 @@ import classnames from 'classnames'; import React, { FunctionComponent, + FocusEventHandler, useContext, useEffect, useRef, @@ -64,12 +65,23 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent< } }, [isFocused]); + // For cell headers with actions, auto-focus into the button instead of the cell wrapper div + // The button text is significantly more useful to screen readers (e.g. contains sort order & hints) + const onFocus: FocusEventHandler = useCallback( + (e) => { + if (hasActionsPopover && e.target === headerRef.current) { + focusActionsButton?.(); + } + }, + [hasActionsPopover, focusActionsButton] + ); + return (
{ { id: 'no_interactive_expandable', display: '0 interactive', - actions: false, }, { id: 'one_interactive', display: '1 interactive', isExpandable: false, - actions: false, }, { id: 'one_interactive_expandable', display: '1 interactive', - actions: false, }, { id: 'two_interactives', display: '2 interactives', isExpandable: false, - actions: false, }, { id: 'two_interactives_expandable', display: '2 interactives', - actions: false, }, ]; const columnVisibility = { @@ -436,6 +431,32 @@ describe('EuiDataGrid', () => { .should('have.attr', 'data-gridcell-column-index', '5') .should('have.attr', 'data-gridcell-row-index', '0'); }); + + it('column header cells', () => { + cy.realMount(); + cy.repeatRealPress('Tab', 5); + cy.realPress('{rightarrow}'); + + // Should auto-focus the actions button (over the cell itself) + cy.focused() + .parent() + .should('have.attr', 'data-gridcell-column-index', '1') + .should('have.attr', 'data-gridcell-row-index', '-1'); + + // Pressing enter should toggle the actions popover + cy.realPress('Enter'); + cy.get( + '[data-test-subj="dataGridHeaderCellActionGroup-no_interactive_expandable"]' + ).should('be.visible'); + + // The actions popover should be fully tabbable/focus trapped with no regressions + cy.realPress('Tab'); + cy.focused().should('have.text', 'Hide column'); + cy.realPress('Tab'); + cy.focused().should('have.text', 'Move left'); + cy.realPress('Tab'); + cy.focused().should('have.text', 'Move right'); + }); }); }); From 1d9e46faa0f1f3fcbc912e625e20574825477698 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 5 Jan 2024 13:06:39 -0800 Subject: [PATCH 20/20] Fix flaky Cypress test - caused by race condition with ref not updating `HandleInteractiveChildren` --- .../data_grid_body_custom.test.tsx.snap | 4 ++++ .../data_grid_body_virtualized.test.tsx.snap | 4 ++++ .../data_grid_header_cell.test.tsx.snap | 2 ++ .../header/data_grid_header_cell_wrapper.tsx | 21 +++++++++---------- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap index f3d5e74d38e..73d35ea0a01 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap @@ -28,7 +28,9 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render