[elastic#5517] Add isFocusedCellInView state to track visibility

- and use this to determine whether or not the entire grid should have a tabindex/be focusable, rather than a simple 'hasFocus' check which doesn't account for virtualization - Add new mockFocusContext for easier testing for various tests that need to set a focus context - EuiDataGridCell - DRY out an `this.isFocusedCell` method - Call `setIsFocusedCellInView` on mount and on unmount (`setFocusedCell` handles setting true/false on new cell focus) - Write new unit tests for componentWillUnmount - Clean up unnecessary uncovered function fallback for this.unsubscribeCell - we're already checking for a falsy value before calling it
cee-chen · Jan 11, 2022 · c06db86 · c06db86
1 parent d4f7bfb
commit c06db86
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 26 deletions.
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 '../utils/__mocks__/row_heights';
+import { mockFocusContext } from '../utils/__mocks__/focus_context';
 import { DataGridFocusContext } from '../utils/focus';
 
 import { EuiDataGridCell } from './data_grid_cell';
@@ -166,27 +167,21 @@ describe('EuiDataGridCell', () => {
   });
 
   describe('componentDidMount', () => {
-    const focusContext = {
-      focusedCell: undefined,
-      onFocusUpdate: jest.fn(),
-      setFocusedCell: jest.fn(),
-    };
-
     it('creates an onFocusUpdate subscription', () => {
       mount(
-        <DataGridFocusContext.Provider value={focusContext}>
+        <DataGridFocusContext.Provider value={mockFocusContext}>
           <EuiDataGridCell {...requiredProps} />
         </DataGridFocusContext.Provider>
       );
 
-      expect(focusContext.onFocusUpdate).toHaveBeenCalled();
+      expect(mockFocusContext.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] }}
+          value={{ ...mockFocusContext, focusedCell: [3, 3] }}
         >
           <EuiDataGridCell
             {...requiredProps}
@@ -198,6 +193,44 @@ describe('EuiDataGridCell', () => {
 
       expect((component.instance().state as any).isFocused).toEqual(true);
       expect(focusSpy).toHaveBeenCalledWith({ preventScroll: true });
+      expect(mockFocusContext.setIsFocusedCellInView).toHaveBeenCalledWith(
+        true
+      );
+    });
+  });
+
+  describe('componentWillUnmount', () => {
+    it('unsubscribes from the onFocusUpdate subscription', () => {
+      const unsubscribeCellMock = jest.fn();
+      mockFocusContext.onFocusUpdate.mockReturnValueOnce(unsubscribeCellMock);
+
+      const component = mount(
+        <DataGridFocusContext.Provider value={mockFocusContext}>
+          <EuiDataGridCell {...requiredProps} />
+        </DataGridFocusContext.Provider>
+      );
+      component.unmount();
+
+      expect(unsubscribeCellMock).toHaveBeenCalled();
+    });
+
+    it('sets isFocusedCellInView to false if the current cell is focused and unmounting due to being scrolled out of view', () => {
+      const component = mount(
+        <DataGridFocusContext.Provider
+          value={{ ...mockFocusContext, focusedCell: [3, 3] }}
+        >
+          <EuiDataGridCell
+            {...requiredProps}
+            colIndex={3}
+            visibleRowIndex={3}
+          />
+        </DataGridFocusContext.Provider>
+      );
+      component.unmount();
+
+      expect(mockFocusContext.setIsFocusedCellInView).toHaveBeenCalledWith(
+        false
+      );
     });
   });
 

diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx
@@ -124,7 +124,7 @@ export class EuiDataGridCell extends Component<
     enableInteractions: false,
     disableCellTabIndex: false,
   };
-  unsubscribeCell?: Function = () => {};
+  unsubscribeCell?: Function;
   focusTimeout: number | undefined;
   style = null;
 
@@ -234,16 +234,21 @@ export class EuiDataGridCell extends Component<
 
     // 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
-    ) {
+    if (this.isFocusedCell()) {
       // 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);
+      this.context.setIsFocusedCellInView(true);
     }
   }
 
+  isFocusedCell = () => {
+    return (
+      this.context.focusedCell?.[0] === this.props.colIndex &&
+      this.context.focusedCell?.[1] === this.props.visibleRowIndex
+    );
+  };
+
   onFocusUpdate = (isFocused: boolean, preventScroll = false) => {
     this.setState({ isFocused }, () => {
       if (isFocused) {
@@ -257,6 +262,10 @@ export class EuiDataGridCell extends Component<
     if (this.unsubscribeCell) {
       this.unsubscribeCell();
     }
+
+    if (this.isFocusedCell()) {
+      this.context.setIsFocusedCellInView(false);
+    }
   }
 
   componentDidUpdate(prevProps: EuiDataGridCellProps) {

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
@@ -12,6 +12,7 @@ import { mount } from 'enzyme';
 
 import { keys } from '../../../../services';
 import { DataGridFocusContext } from '../../utils/focus';
+import { mockFocusContext } from '../../utils/__mocks__/focus_context';
 
 import { EuiDataGridHeaderCellWrapper } from './data_grid_header_cell_wrapper';
 
@@ -23,16 +24,12 @@ describe('EuiDataGridHeaderCellWrapper', () => {
     children: <button />,
   };
 
-  const focusContext = {
-    setFocusedCell: jest.fn(),
-    onFocusUpdate: jest.fn(),
-  };
   const mountWithContext = (props = {}, isFocused = true) => {
-    (focusContext.onFocusUpdate as jest.Mock).mockImplementation(
+    (mockFocusContext.onFocusUpdate as jest.Mock).mockImplementation(
       (_, callback) => callback(isFocused) // allows us to mock isFocused state
     );
     return mount(
-      <DataGridFocusContext.Provider value={focusContext}>
+      <DataGridFocusContext.Provider value={mockFocusContext}>
         <EuiDataGridHeaderCellWrapper {...requiredProps} {...props} />
       </DataGridFocusContext.Provider>
     );
@@ -186,7 +183,7 @@ describe('EuiDataGridHeaderCellWrapper', () => {
               act(() => {
                 headerCell.dispatchEvent(new FocusEvent('focusin'));
               });
-              expect(focusContext.setFocusedCell).toHaveBeenCalled();
+              expect(mockFocusContext.setFocusedCell).toHaveBeenCalled();
             });
 
             it('re-enables and focuses cell interactives when already focused', () => {

diff --git a/src/components/datagrid/data_grid_types.ts b/src/components/datagrid/data_grid_types.ts
@@ -183,6 +183,7 @@ export interface DataGridSortingContextShape {
 export interface DataGridFocusContextShape {
   focusedCell?: EuiDataGridFocusedCell;
   setFocusedCell: (cell: EuiDataGridFocusedCell) => void;
+  setIsFocusedCellInView: (isFocusedCellInView: boolean) => void;
   onFocusUpdate: (
     cell: EuiDataGridFocusedCell,
     updateFocus: Function

diff --git a/src/components/datagrid/utils/__mocks__/focus_context.ts b/src/components/datagrid/utils/__mocks__/focus_context.ts
@@ -0,0 +1,14 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+export const mockFocusContext = {
+  focusedCell: undefined,
+  onFocusUpdate: jest.fn(),
+  setFocusedCell: jest.fn(),
+  setIsFocusedCellInView: jest.fn(),
+};
diff --git a/src/components/datagrid/utils/focus.ts b/src/components/datagrid/utils/focus.ts
@@ -28,6 +28,7 @@ import {
 export const DataGridFocusContext = createContext<DataGridFocusContextShape>({
   focusedCell: undefined,
   setFocusedCell: () => {},
+  setIsFocusedCellInView: () => {},
   onFocusUpdate: () => () => {},
 });
 
@@ -54,10 +55,16 @@ export const useFocus = (
   );
 
   // Current focused cell
-  const [focusedCell, setFocusedCell] = useState<
+  const [isFocusedCellInView, setIsFocusedCellInView] = useState(false);
+  const [focusedCell, _setFocusedCell] = useState<
     EuiDataGridFocusedCell | undefined
   >(undefined);
 
+  const setFocusedCell = useCallback((focusedCell: EuiDataGridFocusedCell) => {
+    _setFocusedCell(focusedCell);
+    setIsFocusedCellInView(true); // The cell must be in view to focus it
+  }, []);
+
   const previousCell = useRef<EuiDataGridFocusedCell | undefined>(undefined);
   useEffect(() => {
     if (previousCell.current) {
@@ -74,11 +81,9 @@ export const useFocus = (
     }
   }, [cellsUpdateFocus, focusedCell]);
 
-  const hasHadFocus = useMemo(() => focusedCell != null, [focusedCell]);
-
   const focusProps = useMemo<FocusProps>(
     () =>
-      hasHadFocus
+      isFocusedCellInView
         ? {
             // FireFox allows tabbing to a div that is scrollable, while Chrome does not
             tabIndex: -1,
@@ -95,13 +100,14 @@ export const useFocus = (
               }
             },
           },
-    [hasHadFocus, setFocusedCell, headerIsInteractive]
+    [isFocusedCellInView, setFocusedCell, headerIsInteractive]
   );
 
   return {
     onFocusUpdate,
     focusedCell,
     setFocusedCell,
+    setIsFocusedCellInView,
     focusProps,
   };
 };