[EuiDataGrid] Handle exposed ref APIs potentially pointing to invalid…

…, off-page, or out of view cells (#5572) * Enable sorting + targeting row indices outside of the current page - to test handling the exposed APIs when dealing with sorted/paginated data * Switch data grid example to Typescript - to test type issues during consumer usage + @ts-ignore faker complaints * Fix cell expansion buttons on paginated pages not working correctly * Attempt to more clearly document `rowIndex`s that are actually `visibleRowIndex`s * [setup] Move imperative handler setup to its own util file - this will let us set up ref-specific helpers & add more comment context without bloating the main file * Add catch/check for cell locations that do not exist in the current grid * Add getVisibleRowIndex helper - Converts the `rowIndex` from the consumer to a `visibleRowIndex` that our internal state can use - Account for sorted grid by finding the inversed index of the `sortedRowMap` - To make this easier, I converted soredRowMap to an array (since it's already only uses numbers for both keys and values), since arrays have a handy .findIndex method - Handles automatically paginating the grid if the targeted cell is on a different page * Replace grid ref Jest tests with more complete Cypress tests * Update documentation with new behavior * [PR feedback] Rename fns to indicate multiple concerns - having a side effect in a getter feels bad, so change that to a `find` - rename use hook to indicate sorting and pagination concerns
elastic · Feb 1, 2022 · a31880b · a31880b
1 parent 0a94221
commit a31880b
Show file tree

Hide file tree

Showing 12 changed files with 556 additions and 87 deletions.
diff --git a/src-docs/src/views/datagrid/datagrid_ref_example.js b/src-docs/src/views/datagrid/datagrid_ref_example.js
@@ -28,7 +28,7 @@ export const DataGridRefExample = {
     {
       source: [
         {
-          type: GuideSectionTypes.JS,
+          type: GuideSectionTypes.TSX,
           code: dataGridRefSource,
         },
       ],
@@ -63,10 +63,30 @@ export const DataGridRefExample = {
               <EuiSpacer size="m" />
             </li>
             <li>
-              <p>
-                <EuiCode>openCellPopover({'{ rowIndex, colIndex }'})</EuiCode> -
-                opens the specified cell&apos;s popover contents.
-              </p>
+              <EuiCode>openCellPopover({'{ rowIndex, colIndex }'})</EuiCode> -
+              opens the specified cell&apos;s popover contents.
+              <EuiSpacer size="s" />
+              <EuiCallOut iconType="mapMarker" title="Handling cell location">
+                When using <EuiCode>setFocusedCell</EuiCode> or{' '}
+                <EuiCode>openCellPopover</EuiCode>, keep in mind:
+                <ul>
+                  <li>
+                    colIndex is affected by the user reordering or hiding
+                    columns.
+                  </li>
+                  <li>
+                    If the passed cell indices are outside the data grid&apos;s
+                    total row count or visible column count, an error will be
+                    thrown.
+                  </li>
+                  <li>
+                    If the data grid is paginated or sorted, the grid will
+                    handle automatically finding specified row index&apos;s
+                    correct location for you.
+                  </li>
+                </ul>
+              </EuiCallOut>
+              <EuiSpacer size="m" />
             </li>
             <li>
               <p>

diff --git a/src-docs/src/views/datagrid/ref.js → src-docs/src/views/datagrid/ref.tsx b/src-docs/src/views/datagrid/ref.js → src-docs/src/views/datagrid/ref.tsx
@@ -1,4 +1,5 @@
 import React, { useCallback, useMemo, useState, useRef } from 'react';
+// @ts-ignore - faker does not have type declarations
 import { fake } from 'faker';
 
 import {
@@ -9,15 +10,16 @@ import {
   EuiFieldNumber,
   EuiButton,
   EuiDataGrid,
+  EuiDataGridRefProps,
   EuiModal,
   EuiModalBody,
   EuiModalFooter,
   EuiModalHeader,
   EuiModalHeaderTitle,
   EuiText,
-} from '../../../../src/components/';
+} from '../../../../src/components';
 
-const raw_data = [];
+const raw_data: Array<{ [key: string]: string }> = [];
 for (let i = 1; i < 100; i++) {
   raw_data.push({
     name: fake('{{name.lastName}}, {{name.firstName}}'),
@@ -29,20 +31,23 @@ for (let i = 1; i < 100; i++) {
 }
 
 export default () => {
-  const dataGridRef = useRef();
+  const dataGridRef = useRef<EuiDataGridRefProps>(null);
 
   // Modal
   const [isModalVisible, setIsModalVisible] = useState(false);
-  const [lastFocusedCell, setLastFocusedCell] = useState({});
+  const [lastFocusedCell, setLastFocusedCell] = useState({
+    rowIndex: 0,
+    colIndex: 0,
+  });
 
   const closeModal = useCallback(() => {
     setIsModalVisible(false);
-    dataGridRef.current.setFocusedCell(lastFocusedCell); // Set the data grid focus back to the cell that opened the modal
+    dataGridRef.current!.setFocusedCell(lastFocusedCell); // Set the data grid focus back to the cell that opened the modal
   }, [lastFocusedCell]);
 
   const showModal = useCallback(({ rowIndex, colIndex }) => {
     setIsModalVisible(true);
-    dataGridRef.current.closeCellPopover(); // Close any open cell popovers
+    dataGridRef.current!.closeCellPopover(); // Close any open cell popovers
     setLastFocusedCell({ rowIndex, colIndex }); // Store the cell that opened this modal
   }, []);
 
@@ -101,11 +106,18 @@ export default () => {
 
   // Pagination
   const [pagination, setPagination] = useState({ pageIndex: 0, pageSize: 25 });
-  const onChangePage = useCallback(
-    (pageIndex) =>
-      setPagination((pagination) => ({ ...pagination, pageIndex })),
-    []
-  );
+  const onChangePage = useCallback((pageIndex) => {
+    setPagination((pagination) => ({ ...pagination, pageIndex }));
+  }, []);
+  const onChangePageSize = useCallback((pageSize) => {
+    setPagination((pagination) => ({ ...pagination, pageSize }));
+  }, []);
+
+  // Sorting
+  const [sortingColumns, setSortingColumns] = useState([]);
+  const onSort = useCallback((sortingColumns) => {
+    setSortingColumns(sortingColumns);
+  }, []);
 
   // Manual cell focus
   const [rowIndexAction, setRowIndexAction] = useState(0);
@@ -118,7 +130,7 @@ export default () => {
           <EuiFormRow label="Row index">
             <EuiFieldNumber
               min={0}
-              max={24}
+              max={raw_data.length - 1}
               value={rowIndexAction}
               onChange={(e) => setRowIndexAction(Number(e.target.value))}
               compressed
@@ -129,7 +141,7 @@ export default () => {
           <EuiFormRow label="Column index">
             <EuiFieldNumber
               min={0}
-              max={4}
+              max={visibleColumns.length - 1}
               value={colIndexAction}
               onChange={(e) => setColIndexAction(Number(e.target.value))}
               compressed
@@ -140,7 +152,7 @@ export default () => {
           <EuiButton
             size="s"
             onClick={() =>
-              dataGridRef.current.setFocusedCell({
+              dataGridRef.current!.setFocusedCell({
                 rowIndex: rowIndexAction,
                 colIndex: colIndexAction,
               })
@@ -153,7 +165,7 @@ export default () => {
           <EuiButton
             size="s"
             onClick={() =>
-              dataGridRef.current.openCellPopover({
+              dataGridRef.current!.openCellPopover({
                 rowIndex: rowIndexAction,
                 colIndex: colIndexAction,
               })
@@ -165,7 +177,7 @@ export default () => {
         <EuiFlexItem grow={false}>
           <EuiButton
             size="s"
-            onClick={() => dataGridRef.current.setIsFullScreen(true)}
+            onClick={() => dataGridRef.current!.setIsFullScreen(true)}
           >
             Set grid to full screen
           </EuiButton>
@@ -177,14 +189,17 @@ export default () => {
         aria-label="Data grid demo"
         columns={columns}
         columnVisibility={{ visibleColumns, setVisibleColumns }}
+        sorting={{ columns: sortingColumns, onSort }}
+        inMemory={{ level: 'sorting' }}
         rowCount={raw_data.length}
         renderCellValue={({ rowIndex, columnId }) =>
           raw_data[rowIndex][columnId]
         }
         pagination={{
           ...pagination,
-          pageSizeOptions: [25],
+          pageSizeOptions: [25, 50],
           onChangePage: onChangePage,
+          onChangeItemsPerPage: onChangePageSize,
         }}
         height={400}
         ref={dataGridRef}

diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx
@@ -490,7 +490,7 @@ export class EuiDataGridCell extends Component<
       rowManager,
       ...rest
     } = this.props;
-    const { rowIndex, colIndex } = rest;
+    const { rowIndex, visibleRowIndex, colIndex } = rest;
 
     const popoverIsOpen = this.isPopoverOpen();
     const hasCellButtons = isExpandable || column?.cellActions;
@@ -534,7 +534,7 @@ export class EuiDataGridCell extends Component<
           case keys.ENTER:
           case keys.F2:
             event.preventDefault();
-            openCellPopover({ rowIndex, colIndex });
+            openCellPopover({ rowIndex: visibleRowIndex, colIndex });
             break;
         }
       } else {
@@ -641,7 +641,7 @@ export class EuiDataGridCell extends Component<
                 if (popoverIsOpen) {
                   closeCellPopover();
                 } else {
-                  openCellPopover({ rowIndex, colIndex });
+                  openCellPopover({ rowIndex: visibleRowIndex, colIndex });
                 }
               }}
             />

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
@@ -49,7 +49,7 @@ describe('EuiDataGridHeaderCell', () => {
         <DataGridSortingContext.Provider
           value={{
             sorting: { ...sortingContext, ...sorting },
-            sortedRowMap: {},
+            sortedRowMap: [],
             getCorrectRowIndex: jest.fn(),
           }}
         >

diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx
@@ -6,10 +6,10 @@
  * Side Public License, v 1.
  */
 
-import React, { useEffect, useState, createRef } from 'react';
+import React, { useEffect, useState } from 'react';
 import { mount, ReactWrapper, render } from 'enzyme';
 import { EuiDataGrid } from './';
-import { EuiDataGridProps, EuiDataGridRefProps } from './data_grid_types';
+import { EuiDataGridProps } from './data_grid_types';
 import {
   findTestSubject,
   requiredProps,
@@ -2749,29 +2749,4 @@ describe('EuiDataGrid', () => {
       expect(takeMountedSnapshot(component)).toMatchSnapshot();
     });
   });
-
-  it('returns a ref which exposes internal imperative APIs', () => {
-    const gridRef = createRef<EuiDataGridRefProps>();
-
-    mount(
-      <EuiDataGrid
-        {...requiredProps}
-        columns={[{ id: 'A' }, { id: 'B' }]}
-        columnVisibility={{
-          visibleColumns: ['A', 'B'],
-          setVisibleColumns: () => {},
-        }}
-        rowCount={1}
-        renderCellValue={() => 'value'}
-        ref={gridRef}
-      />
-    );
-
-    expect(gridRef.current).toEqual({
-      setIsFullScreen: expect.any(Function),
-      setFocusedCell: expect.any(Function),
-      openCellPopover: expect.any(Function),
-      closeCellPopover: expect.any(Function),
-    });
-  });
 });
diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx
@@ -13,7 +13,6 @@ import React, {
   useMemo,
   useRef,
   useState,
-  useImperativeHandle,
 } from 'react';
 import {
   VariableSizeGrid as Grid,
@@ -56,6 +55,7 @@ import {
   schemaDetectors as providedSchemaDetectors,
   useMergedSchema,
 } from './utils/data_grid_schema';
+import { useImperativeGridRef } from './utils/ref';
 import {
   EuiDataGridColumn,
   EuiDataGridProps,
@@ -302,23 +302,18 @@ export const EuiDataGrid = forwardRef<EuiDataGridRefProps, EuiDataGridProps>(
     };
 
     /**
-     * Expose internal APIs as ref to consumer
+     * Expose certain internal APIs as ref to consumer
      */
-    const { setFocusedCell } = focusContext; // eslint complains about the dependency array otherwise
-    const { openCellPopover, closeCellPopover } = cellPopoverContext;
-
-    useImperativeHandle(
+    useImperativeGridRef({
       ref,
-      () => ({
-        setIsFullScreen,
-        setFocusedCell: ({ rowIndex, colIndex }) => {
-          setFocusedCell([colIndex, rowIndex]); // Transmog args from obj to array
-        },
-        openCellPopover,
-        closeCellPopover,
-      }),
-      [setFocusedCell, openCellPopover, closeCellPopover]
-    );
+      setIsFullScreen,
+      focusContext,
+      cellPopoverContext,
+      sortingContext,
+      pagination,
+      rowCount,
+      visibleColCount,
+    });
 
     /**
      * Classes

diff --git a/src/components/datagrid/data_grid_types.ts b/src/components/datagrid/data_grid_types.ts
@@ -179,10 +179,13 @@ export interface EuiDataGridVisibleRows {
 
 export interface DataGridSortingContextShape {
   sorting?: EuiDataGridSorting;
-  sortedRowMap: { [key: number]: number };
-  getCorrectRowIndex: (rowIndex: number) => number;
+  sortedRowMap: number[];
+  getCorrectRowIndex: (visibleRowIndex: number) => number;
 }
 
+// An array of [x,y] coordinates. Note that the `y` value expected internally is a `visibleRowIndex`
+export type EuiDataGridFocusedCell = [number, number];
+
 export interface DataGridFocusContextShape {
   focusedCell?: EuiDataGridFocusedCell;
   setFocusedCell: (cell: EuiDataGridFocusedCell) => void;
@@ -196,6 +199,7 @@ export interface DataGridFocusContextShape {
 
 export interface DataGridCellPopoverContextShape {
   popoverIsOpen: boolean;
+  // Note that the rowIndex used to locate cells internally is a `visibleRowIndex`
   cellLocation: { rowIndex: number; colIndex: number };
   openCellPopover(args: { rowIndex: number; colIndex: number }): void;
   closeCellPopover(): void;
@@ -764,8 +768,6 @@ export interface EuiDataGridInMemory {
   skipColumns?: string[];
 }
 
-export type EuiDataGridFocusedCell = [number, number];
-
 export interface EuiDataGridInMemoryValues {
   [rowIndex: string]: { [columnId: string]: string };
 }