[EuiDataGrid] Fix cells losing focus state when scrolled out of view (#…

…5488) * [Setup] Add focusedCell state to shared focus context - will be used by cells to check if they're the currently focused cell * Update EuiDataGridCell to handle virtualization - by mounting with focus state (when cells scroll back into view) * 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 <DataGridFocusContext.Provider> as a wrapper * Fix misc documentation typos * Add changelog entry * [PR feedback] Prevent focus scroll on virtualization re-mount - since the user is presumably already scrolling, there's no need to hijack their current scroll
elastic · Dec 28, 2021 · 6835c2d · 6835c2d
1 parent ec599f4
commit 6835c2d
Show file tree

Hide file tree

Showing 8 changed files with 143 additions and 72 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 - Fixed a `EuiDataGrid` sizing bug which didn't account for a horizontal scrollbar ([#5478](https://github.com/elastic/eui/pull/5478))
 - Fixed a `EuiDatePicker` a11y bug where axe-core reported missing ARIA and role attributes ([#5501](https://github.com/elastic/eui/pull/5501))
 - Fixed `EuiModalHeaderTitle` to conditionally wrap title strings in an H1 ([#5494](https://github.com/elastic/eui/pull/5494))
+- Fixed a `EuiDataGrid` issue where a focused cell would lose focus when scrolled out of and back into view ([#5488](https://github.com/elastic/eui/pull/5488))
 
 **Deprecations**
 

diff --git a/src-docs/src/views/datagrid/datagrid_example.js b/src-docs/src/views/datagrid/datagrid_example.js
@@ -358,8 +358,8 @@ export const DataGridExample = {
                 in memory level
               </Link>{' '}
               to have the grid automatically handle updating your columns.
-              Depending upon the level choosen you may need to manage the
-              content order separate from the grid.
+              Depending upon the level chosen, you may need to manage the
+              content order separately from the grid.
             </li>
             <li>
               <Link to="/tabular-content/data-grid-schemas-and-popovers/">

diff --git a/src-docs/src/views/datagrid/datagrid_focus_example.js b/src-docs/src/views/datagrid/datagrid_focus_example.js
@@ -93,7 +93,7 @@ export const DataGridFocusExample = {
             color="warning"
             title="A caution about turning off cell expansion when the width of the column is unknown"
           >
-            In general, you should turn <EuiCode>isExpandible</EuiCode> to false
+            In general, you should turn <EuiCode>isExpandable</EuiCode> to false
             only when you know the exact width and number of items that a cell
             will include. Control columns that contain row actions are a good
             example of when to use them. In certain scenarios, allowing multiple

diff --git 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';
 
@@ -31,31 +32,24 @@ describe('EuiDataGridCell', () => {
     rowHeightUtils: mockRowHeightUtils,
   };
 
-  const mountEuiDataGridCellWithContext = ({ ...props } = {}) => {
-    const focusContext = {
-      setFocusedCell: jest.fn(),
-      onFocusUpdate: jest.fn(),
-    };
-    return mount(<EuiDataGridCell {...requiredProps} {...props} />, {
-      context: focusContext,
-    });
-  };
-
   beforeEach(() => jest.clearAllMocks());
 
   it('renders', () => {
-    const component = mountEuiDataGridCellWithContext();
+    const component = mount(<EuiDataGridCell {...requiredProps} />);
     expect(component).toMatchSnapshot();
   });
 
   it('renders cell buttons', () => {
-    const component = mountEuiDataGridCellWithContext({
-      isExpandable: false,
-      column: {
-        id: 'someColumn',
-        cellActions: [() => <button />],
-      },
-    });
+    const component = mount(
+      <EuiDataGridCell
+        {...requiredProps}
+        isExpandable={false}
+        column={{
+          id: 'someColumn',
+          cellActions: [() => <button />],
+        }}
+      />
+    );
     component.setState({ popoverIsOpen: true });
 
     const cellButtons = component.find('EuiDataGridCellButtons');
@@ -80,7 +74,7 @@ describe('EuiDataGridCell', () => {
         EuiDataGridCell.prototype,
         'shouldComponentUpdate'
       );
-      component = mountEuiDataGridCellWithContext();
+      component = mount(<EuiDataGridCell {...requiredProps} />);
     });
     afterEach(() => {
       shouldComponentUpdate.mockRestore();
@@ -164,13 +158,49 @@ describe('EuiDataGridCell', () => {
   describe('componentDidUpdate', () => {
     it('resets cell props when the cell columnId changes', () => {
       const setState = jest.spyOn(EuiDataGridCell.prototype, 'setState');
-      const component = mountEuiDataGridCellWithContext();
+      const component = mount(<EuiDataGridCell {...requiredProps} />);
 
       component.setProps({ columnId: 'newColumnId' });
       expect(setState).toHaveBeenCalledWith({ cellProps: {} });
     });
   });
 
+  describe('componentDidMount', () => {
+    const focusContext = {
+      focusedCell: undefined,
+      onFocusUpdate: jest.fn(),
+      setFocusedCell: jest.fn(),
+    };
+
+    it('creates an onFocusUpdate subscription', () => {
+      mount(
+        <DataGridFocusContext.Provider value={focusContext}>
+          <EuiDataGridCell {...requiredProps} />
+        </DataGridFocusContext.Provider>
+      );
+
+      expect(focusContext.onFocusUpdate).toHaveBeenCalled();
+    });
+
+    it('mounts the cell with focus state if the current cell should be focused', () => {
+      const focusSpy = jest.spyOn(HTMLElement.prototype, 'focus');
+      const component = mount(
+        <DataGridFocusContext.Provider
+          value={{ ...focusContext, focusedCell: [3, 3] }}
+        >
+          <EuiDataGridCell
+            {...requiredProps}
+            colIndex={3}
+            visibleRowIndex={3}
+          />
+        </DataGridFocusContext.Provider>
+      );
+
+      expect((component.instance().state as any).isFocused).toEqual(true);
+      expect(focusSpy).toHaveBeenCalledWith({ preventScroll: true });
+    });
+  });
+
   // TODO: Test ResizeObserver logic in Cypress alongside Jest
   describe('row height logic & resize observers', () => {
     describe('recalculateAutoHeight', () => {
@@ -184,9 +214,12 @@ describe('EuiDataGridCell', () => {
       it('sets the row height cache with cell heights on update', () => {
         (mockRowHeightUtils.isAutoHeight as jest.Mock).mockReturnValue(true);
 
-        const component = mountEuiDataGridCellWithContext({
-          rowHeightsOptions: { defaultHeight: 'auto' },
-        });
+        const component = mount(
+          <EuiDataGridCell
+            {...requiredProps}
+            rowHeightsOptions={{ defaultHeight: 'auto' }}
+          />
+        );
 
         triggerUpdate(component);
         expect(mockRowHeightUtils.setRowHeight).toHaveBeenCalled();
@@ -195,9 +228,12 @@ describe('EuiDataGridCell', () => {
       it('does not update the cache if cell height is not auto', () => {
         (mockRowHeightUtils.isAutoHeight as jest.Mock).mockReturnValue(false);
 
-        const component = mountEuiDataGridCellWithContext({
-          rowHeightsOptions: { defaultHeight: 34 },
-        });
+        const component = mount(
+          <EuiDataGridCell
+            {...requiredProps}
+            rowHeightsOptions={{ defaultHeight: 34 }}
+          />
+        );
 
         triggerUpdate(component);
         expect(mockRowHeightUtils.setRowHeight).not.toHaveBeenCalled();
@@ -212,10 +248,13 @@ describe('EuiDataGridCell', () => {
 
       describe('default height', () => {
         it('observes the first cell for size changes and calls this.props.setRowHeight on change', () => {
-          const component = mountEuiDataGridCellWithContext({
-            rowHeightsOptions: { defaultHeight: { lineCount: 3 } },
-            setRowHeight,
-          });
+          const component = mount(
+            <EuiDataGridCell
+              {...requiredProps}
+              rowHeightsOptions={{ defaultHeight: { lineCount: 3 } }}
+              setRowHeight={setRowHeight}
+            />
+          );
 
           callMethod(component);
           expect(
@@ -227,14 +266,17 @@ describe('EuiDataGridCell', () => {
 
       describe('row height overrides', () => {
         it('uses the rowHeightUtils.setRowHeight cache instead of this.props.setRowHeight', () => {
-          const component = mountEuiDataGridCellWithContext({
-            rowHeightsOptions: {
-              defaultHeight: { lineCount: 3 },
-              rowHeights: { 10: { lineCount: 10 } },
-            },
-            rowIndex: 10,
-            setRowHeight,
-          });
+          const component = mount(
+            <EuiDataGridCell
+              {...requiredProps}
+              rowHeightsOptions={{
+                defaultHeight: { lineCount: 3 },
+                rowHeights: { 10: { lineCount: 10 } },
+              }}
+              rowIndex={10}
+              setRowHeight={setRowHeight}
+            />
+          );
 
           callMethod(component);
           expect(
@@ -246,10 +288,13 @@ describe('EuiDataGridCell', () => {
       });
 
       it('recalculates when rowHeightsOptions.defaultHeight.lineCount changes', () => {
-        const component = mountEuiDataGridCellWithContext({
-          rowHeightsOptions: { defaultHeight: { lineCount: 7 } },
-          setRowHeight,
-        });
+        const component = mount(
+          <EuiDataGridCell
+            {...requiredProps}
+            rowHeightsOptions={{ defaultHeight: { lineCount: 7 } }}
+            setRowHeight={setRowHeight}
+          />
+        );
 
         component.setProps({
           rowHeightsOptions: { defaultHeight: { lineCount: 6 } },
@@ -258,10 +303,13 @@ describe('EuiDataGridCell', () => {
       });
 
       it('calculates undefined heights as single rows with a lineCount of 1', () => {
-        const component = mountEuiDataGridCellWithContext({
-          rowHeightsOptions: { defaultHeight: undefined },
-          setRowHeight,
-        });
+        const component = mount(
+          <EuiDataGridCell
+            {...requiredProps}
+            rowHeightsOptions={{ defaultHeight: undefined }}
+            setRowHeight={setRowHeight}
+          />
+        );
 
         callMethod(component);
         expect(
@@ -271,10 +319,13 @@ describe('EuiDataGridCell', () => {
       });
 
       it('does nothing if cell height is not lineCount or undefined', () => {
-        const component = mountEuiDataGridCellWithContext({
-          rowHeightsOptions: { defaultHeight: 34 },
-          setRowHeight,
-        });
+        const component = mount(
+          <EuiDataGridCell
+            {...requiredProps}
+            rowHeightsOptions={{ defaultHeight: 34 }}
+            setRowHeight={setRowHeight}
+          />
+        );
 
         callMethod(component);
         expect(setRowHeight).not.toHaveBeenCalled();
@@ -286,7 +337,7 @@ describe('EuiDataGridCell', () => {
   describe('interactions', () => {
     describe('keyboard events', () => {
       it('when cell is expandable', () => {
-        const component = mountEuiDataGridCellWithContext();
+        const component = mount(<EuiDataGridCell {...requiredProps} />);
         const preventDefault = jest.fn();
 
         component.simulate('keyDown', { preventDefault, key: keys.ENTER });
@@ -296,9 +347,9 @@ describe('EuiDataGridCell', () => {
       });
 
       it('when cell is not expandable', () => {
-        const component = mountEuiDataGridCellWithContext({
-          isExpandable: false,
-        });
+        const component = mount(
+          <EuiDataGridCell {...requiredProps} isExpandable={false} />
+        );
         const preventDefault = jest.fn();
 
         component.simulate('keyDown', { preventDefault, key: keys.ENTER });
@@ -321,28 +372,31 @@ describe('EuiDataGridCell', () => {
     });
 
     it('mouse events', () => {
-      const component = mountEuiDataGridCellWithContext();
+      const component = mount(<EuiDataGridCell {...requiredProps} />);
       component.simulate('mouseEnter');
       expect(component.state('enableInteractions')).toEqual(true);
       component.simulate('mouseLeave');
       expect(component.state('enableInteractions')).toEqual(false);
     });
 
     it('focus/blur events', () => {
-      const component = mountEuiDataGridCellWithContext();
+      const component = mount(<EuiDataGridCell {...requiredProps} />);
       component.simulate('focus');
       component.simulate('blur');
       expect(component.state('disableCellTabIndex')).toEqual(false);
     });
   });
 
   it('renders certain classes/styles if rowHeightOptions is passed', () => {
-    const component = mountEuiDataGridCellWithContext({
-      rowHeightsOptions: {
-        defaultHeight: 20,
-        rowHeights: { 0: 10 },
-      },
-    });
+    const component = mount(
+      <EuiDataGridCell
+        {...requiredProps}
+        rowHeightsOptions={{
+          defaultHeight: 20,
+          rowHeights: { 0: 10 },
+        }}
+      />
+    );
     component.setState({ popoverIsOpen: true });
 
     expect(

diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx
@@ -142,7 +142,7 @@ export class EuiDataGridCell extends Component<
     return [];
   };
 
-  takeFocus = () => {
+  takeFocus = (preventScroll: boolean) => {
     const cell = this.cellRef.current;
 
     if (cell) {
@@ -157,9 +157,9 @@ export class EuiDataGridCell extends Component<
         const interactables = this.getInteractables();
         if (this.props.isExpandable === false && interactables.length === 1) {
           // Only one element can be interacted with
-          interactables[0].focus();
+          interactables[0].focus({ preventScroll });
         } else {
-          cell.focus();
+          cell.focus({ preventScroll });
         }
       }
     }
@@ -225,16 +225,29 @@ 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
+    ) {
+      // The second flag sets preventScroll: true as a focus option, which prevents
+      // hijacking the user's scroll behavior when the cell re-mounts on scroll
+      this.onFocusUpdate(true, true);
+    }
   }
 
-  onFocusUpdate = (isFocused: boolean) => {
+  onFocusUpdate = (isFocused: boolean, preventScroll = false) => {
     this.setState({ isFocused }, () => {
       if (isFocused) {
-        this.takeFocus();
+        this.takeFocus(preventScroll);
       }
     });
   };

diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx
@@ -748,10 +748,11 @@ export const EuiDataGrid: FunctionComponent<EuiDataGridProps> = (props) => {
   );
   const datagridFocusContext = useMemo<DataGridFocusContextShape>(() => {
     return {
+      focusedCell,
       setFocusedCell,
       onFocusUpdate,
     };
-  }, [setFocusedCell, onFocusUpdate]);
+  }, [focusedCell, setFocusedCell, onFocusUpdate]);
 
   const gridId = useGeneratedHtmlId();
   const ariaLabelledById = useGeneratedHtmlId();