From df1ba214fe8c2f325a0e90fc4c349d5896b0cb46 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 16 Dec 2021 12:38:23 -0800 Subject: [PATCH 1/6] [Setup] Add focusedCell state to shared focus context - will be used by cells to check if they're the currently focused cell --- src/components/datagrid/data_grid.tsx | 3 ++- src/components/datagrid/data_grid_context.tsx | 1 + src/components/datagrid/data_grid_types.ts | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index af29264c086..438462d93cb 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -748,10 +748,11 @@ export const EuiDataGrid: FunctionComponent = (props) => { ); const datagridFocusContext = useMemo(() => { return { + focusedCell, setFocusedCell, onFocusUpdate, }; - }, [setFocusedCell, onFocusUpdate]); + }, [focusedCell, setFocusedCell, onFocusUpdate]); const gridId = useGeneratedHtmlId(); const ariaLabelledById = useGeneratedHtmlId(); diff --git a/src/components/datagrid/data_grid_context.tsx b/src/components/datagrid/data_grid_context.tsx index 9a252106287..5234bb5c83a 100644 --- a/src/components/datagrid/data_grid_context.tsx +++ b/src/components/datagrid/data_grid_context.tsx @@ -16,6 +16,7 @@ import { export const DataGridFocusContext = React.createContext< DataGridFocusContextShape >({ + focusedCell: undefined, setFocusedCell: () => {}, onFocusUpdate: () => () => {}, }); diff --git a/src/components/datagrid/data_grid_types.ts b/src/components/datagrid/data_grid_types.ts index 103898dfc26..9043dc07b17 100644 --- a/src/components/datagrid/data_grid_types.ts +++ b/src/components/datagrid/data_grid_types.ts @@ -167,6 +167,7 @@ export type EuiDataGridFooterRowProps = CommonProps & }; export interface DataGridFocusContextShape { + focusedCell?: EuiDataGridFocusedCell; setFocusedCell: (cell: EuiDataGridFocusedCell) => void; onFocusUpdate: ( cell: EuiDataGridFocusedCell, From bfafed076cb935d9e0f610b5de8a7bb2bfa3e36c Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 16 Dec 2021 12:47:47 -0800 Subject: [PATCH 2/6] Update EuiDataGridCell to handle virtualization - by mounting with focus state (when cells scroll back into view) --- .../datagrid/body/data_grid_cell.test.tsx | 35 +++++++++++++++++++ .../datagrid/body/data_grid_cell.tsx | 13 ++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/components/datagrid/body/data_grid_cell.test.tsx b/src/components/datagrid/body/data_grid_cell.test.tsx index 27763b0dcd0..76dd7c9fec0 100644 --- a/src/components/datagrid/body/data_grid_cell.test.tsx +++ b/src/components/datagrid/body/data_grid_cell.test.tsx @@ -10,6 +10,7 @@ import React from 'react'; import { mount, ReactWrapper } from 'enzyme'; import { keys } from '../../../services'; import { mockRowHeightUtils } from '../__mocks__/row_height_utils'; +import { DataGridFocusContext } from '../data_grid_context'; import { EuiDataGridCell } from './data_grid_cell'; @@ -171,6 +172,40 @@ describe('EuiDataGridCell', () => { }); }); + describe('componentDidMount', () => { + const focusContext = { + focusedCell: undefined, + onFocusUpdate: jest.fn(), + setFocusedCell: jest.fn(), + }; + + it('creates an onFocusUpdate subscription', () => { + mount( + + + + ); + + expect(focusContext.onFocusUpdate).toHaveBeenCalled(); + }); + + it('mounts the cell with focus state if the current cell should be focused', () => { + const component = mount( + + + + ); + + expect((component.instance().state as any).isFocused).toEqual(true); + }); + }); + // TODO: Test ResizeObserver logic in Cypress alongside Jest describe('row height logic & resize observers', () => { describe('recalculateAutoHeight', () => { diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index 9b253a403d4..e2b222f308a 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -225,10 +225,21 @@ export class EuiDataGridCell extends Component< }; componentDidMount() { + const { colIndex, visibleRowIndex } = this.props; + this.unsubscribeCell = this.context.onFocusUpdate( - [this.props.colIndex, this.props.visibleRowIndex], + [colIndex, visibleRowIndex], this.onFocusUpdate ); + + // Account for virtualization - when a cell unmounts when scrolled out of view + // and then remounts when scrolled back into view, it should retain focus state + if ( + this.context.focusedCell?.[0] === colIndex && + this.context.focusedCell?.[1] === visibleRowIndex + ) { + this.onFocusUpdate(true); + } } onFocusUpdate = (isFocused: boolean) => { From 45d793f3ef1e5290703cb30fc2c70e62f4702c0d Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 16 Dec 2021 12:53:18 -0800 Subject: [PATCH 3/6] Misc unit test mount fix - Remove mountEuiDataGridCellWithContext helper - the context wasn't actually getting used (it was falling back to the default context), and wasn't needed by most tests, and the tests that do need it can manually mount() with as a wrapper --- .../datagrid/body/data_grid_cell.test.tsx | 141 ++++++++++-------- 1 file changed, 79 insertions(+), 62 deletions(-) diff --git a/src/components/datagrid/body/data_grid_cell.test.tsx b/src/components/datagrid/body/data_grid_cell.test.tsx index 76dd7c9fec0..32eca039700 100644 --- a/src/components/datagrid/body/data_grid_cell.test.tsx +++ b/src/components/datagrid/body/data_grid_cell.test.tsx @@ -32,31 +32,24 @@ describe('EuiDataGridCell', () => { rowHeightUtils: mockRowHeightUtils, }; - const mountEuiDataGridCellWithContext = ({ ...props } = {}) => { - const focusContext = { - setFocusedCell: jest.fn(), - onFocusUpdate: jest.fn(), - }; - return mount(, { - context: focusContext, - }); - }; - beforeEach(() => jest.clearAllMocks()); it('renders', () => { - const component = mountEuiDataGridCellWithContext(); + const component = mount(); expect(component).toMatchSnapshot(); }); it('renders cell buttons', () => { - const component = mountEuiDataGridCellWithContext({ - isExpandable: false, - column: { - id: 'someColumn', - cellActions: [() =>