EuiDataGrid keyboard navigation tweaks

elastic · Nov 19, 2019 · 8c01784 · 8c01784
1 parent 0fd79d8
commit 8c01784
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 89 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,7 +1,7 @@
 ## [`master`](https://github.com/elastic/eui/tree/master)
 
-- Added new keyboard shortcuts for the data grid component: `Home` (same row, first column), `End` (same row, last column), `Ctrl+Home` (first row, first column), `Ctrl+End` (last row, last column), `Page Up` (next page) and `Page Down` (previous page)
 - Added `badge` prop and new styles `EuiHeaderAlert` ([#2506](https://github.com/elastic/eui/pull/2506))
+- Added new keyboard shortcuts for the data grid component: `Home` (same row, first column), `End` (same row, last column), `Ctrl+Home` (first row, first column), `Ctrl+End` (last row, last column), `Page Up` (next page) and `Page Down` (previous page) ([#2519](https://github.com/elastic/eui/pull/2519))
 
 ## [`16.0.1`](https://github.com/elastic/eui/tree/v16.0.1)
 

diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx
@@ -1441,6 +1441,20 @@ Array [
 
   describe('keyboard controls', () => {
     it('supports simple arrow navigation', () => {
+      let pagination = {
+        pageIndex: 0,
+        pageSize: 3,
+        pageSizeOptions: [3, 6, 10],
+        onChangePage: (pageIndex: number) => {
+          pagination = {
+            ...pagination,
+            pageIndex,
+          };
+          component.setProps({ pagination });
+        },
+        onChangeItemsPerPage: () => {},
+      };
+
       const component = mount(
         <EuiDataGrid
           {...requiredProps}
@@ -1453,151 +1467,157 @@ Array [
           renderCellValue={({ rowIndex, columnId }) =>
             `${rowIndex}, ${columnId}`
           }
-          pagination={{
-            pageIndex: 0,
-            pageSize: 3,
-            pageSizeOptions: [3, 6, 10],
-            onChangePage: () => {},
-            onChangeItemsPerPage: () => {},
-          }}
+          pagination={pagination}
         />
       );
 
       let focusableCell = getFocusableCell(component);
+      // focus should begin at the first cell
       expect(focusableCell.length).toEqual(1);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('0, A');
 
+      // focus should not move when up against the left edge
       focusableCell
         .simulate('focus')
         .simulate('keydown', { keyCode: keyCodes.LEFT });
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
-      ).toEqual('0, A'); // focus should not move when up against an edge
+      ).toEqual('0, A');
 
+      // focus should not move when up against the top edge
       focusableCell.simulate('keydown', { keyCode: keyCodes.UP });
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
-      ).toEqual('0, A'); // focus should not move when up against an edge
+      ).toEqual('0, A');
 
+      // move down
       focusableCell.simulate('keydown', { keyCode: keyCodes.DOWN });
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('1, A');
 
+      // move right
       focusableCell.simulate('keydown', { keyCode: keyCodes.RIGHT });
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('1, B');
 
+      // move up
       focusableCell.simulate('keydown', { keyCode: keyCodes.UP });
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('0, B');
 
+      // move left
       focusableCell.simulate('keydown', { keyCode: keyCodes.LEFT });
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('0, A');
 
+      // move down and to the end of the row
       focusableCell
         .simulate('keydown', { keyCode: keyCodes.DOWN })
         .simulate('keydown', { keyCode: keyCodes.END });
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('1, C');
 
+      // move up and to the beginning of the row
       focusableCell
         .simulate('keydown', { keyCode: keyCodes.UP })
         .simulate('keydown', { keyCode: keyCodes.HOME });
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('0, A');
 
+      // jump to the last cell
       focusableCell.simulate('keydown', {
         ctrlKey: true,
         keyCode: keyCodes.END,
       });
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('2, C');
 
+      // jump to the first cell
       focusableCell.simulate('keydown', {
         ctrlKey: true,
         keyCode: keyCodes.HOME,
       });
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('0, A');
 
+      // page should not change when moving before the first entry
       focusableCell.simulate('keydown', {
         keyCode: keyCodes.PAGE_UP,
-      }); // 0, A
-
+      });
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
-      ).toEqual('0, A'); // focus should not move when up against an edge
+      ).toEqual('0, A');
 
+      // advance to the next page
       focusableCell.simulate('keydown', {
         keyCode: keyCodes.PAGE_DOWN,
-      }); // 3, A
-
+      });
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('3, A');
 
+      // move over one column and advance one more page
       focusableCell
         .simulate('keydown', { keyCode: keyCodes.RIGHT }) // 3, B
         .simulate('keydown', {
           keyCode: keyCodes.PAGE_DOWN,
         }); // 6, B
+      focusableCell = getFocusableCell(component);
+      expect(
+        focusableCell.find('[data-test-subj="cell-content"]').text()
+      ).toEqual('6, B');
 
+      // does not advance beyond the last page
+      focusableCell.simulate('keydown', {
+        keyCode: keyCodes.PAGE_DOWN,
+      });
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
-      ).toEqual('6, B'); // should move page forward and keep focus on the same cell
+      ).toEqual('6, B');
 
+      // move left one column, return to the previous page
       focusableCell
         .simulate('keydown', { keyCode: keyCodes.LEFT }) // 6, A
         .simulate('keydown', {
           keyCode: keyCodes.PAGE_UP,
         }); // 3, A
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
-      ).toEqual('3, A'); // back one page, at the first cell
+      ).toEqual('3, A');
 
+      // return to the previous (first) page
       focusableCell.simulate('keydown', {
         keyCode: keyCodes.PAGE_UP,
-      }); // 0, A
-
+      });
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
-      ).toEqual('0, A'); // should be back in the first page
+      ).toEqual('0, A');
 
+      // move to the last cell of the page then advance one page
       focusableCell
         .simulate('keydown', {
           ctrlKey: true,
@@ -1606,24 +1626,21 @@ Array [
         .simulate('keydown', {
           keyCode: keyCodes.PAGE_DOWN,
         }); // 5, C (last cell of the second page, same cell position as previous page)
-
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('5, C');
 
+      // advance to the final page, but there is 1 row less on page 3 so focus should retreat a row but retain the column
       focusableCell.simulate('keydown', {
         keyCode: keyCodes.PAGE_DOWN,
-      }); // 7, C (should recalculate row since there is not as many rows as previous page)
-
+      }); // 7, C
       focusableCell = getFocusableCell(component);
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('7, C');
-      // (equivalent cell position does not exist in last page (would be 8, C),
-      // so keeps the same column position. but moves to the last available row,
-      // which should be (7, C))
     });
+
     it('does not break arrow key focus control behavior when also using a mouse', () => {
       const component = mount(
         <EuiDataGrid
@@ -1641,7 +1658,6 @@ Array [
       );
 
       let focusableCell = getFocusableCell(component);
-      // console.log(focusableCell.debug());
       expect(
         focusableCell.find('[data-test-subj="cell-content"]').text()
       ).toEqual('0, A');

diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx
@@ -160,7 +160,9 @@ const cellPaddingsToClassMap: {
   l: 'euiDataGrid--paddingLarge',
 };
 
-function computeVisibleRows(props: EuiDataGridProps) {
+function computeVisibleRows(
+  props: Pick<EuiDataGridProps, 'pagination' | 'rowCount'>
+) {
   const { pagination, rowCount } = props;
 
   const startRow = pagination ? pagination.pageIndex * pagination.pageSize : 0;
@@ -320,14 +322,19 @@ function createKeyDownHandler(
     } else if (keyCode === keyCodes.PAGE_DOWN) {
       if (props.pagination) {
         event.preventDefault();
-        const totalRowCount = props.rowCount;
+        const rowCount = props.rowCount;
         const pageIndex = props.pagination.pageIndex;
         const pageSize = props.pagination.pageSize;
-        const pageCount = Math.ceil(totalRowCount / pageSize);
-        if (pageIndex < pageCount) {
-          props.pagination!.pageIndex = pageIndex + 1;
-          props.pagination.onChangePage(props.pagination.pageIndex);
-          const newPageRowCount = computeVisibleRows(props);
+        const pageCount = Math.ceil(rowCount / pageSize);
+        if (pageIndex < pageCount - 1) {
+          props.pagination.onChangePage(pageIndex + 1);
+          const newPageRowCount = computeVisibleRows({
+            rowCount,
+            pagination: {
+              ...props.pagination,
+              pageIndex: pageIndex + 1,
+            },
+          });
           const rowIndex =
             focusedCell[1] < newPageRowCount
               ? focusedCell[1]
@@ -341,9 +348,14 @@ function createKeyDownHandler(
         event.preventDefault();
         const pageIndex = props.pagination.pageIndex;
         if (pageIndex > 0) {
-          props.pagination!.pageIndex = pageIndex - 1;
-          props.pagination.onChangePage(props.pagination.pageIndex);
-          const newPageRowCount = computeVisibleRows(props);
+          props.pagination.onChangePage(pageIndex - 1);
+          const newPageRowCount = computeVisibleRows({
+            rowCount,
+            pagination: {
+              ...props.pagination,
+              pageIndex: pageIndex - 1,
+            },
+          });
           const rowIndex =
             focusedCell[1] < newPageRowCount
               ? focusedCell[1]
@@ -613,22 +625,16 @@ export const EuiDataGrid: FunctionComponent<EuiDataGridProps> = props => {
   const datagridContext = {
     onFocusUpdate: (cell: [number, number], updateFocus: Function) => {
       if (pagination) {
-        // Receives the row index as for the whole set
-        // and normalizes it for the visible rows in the grid
-        const pageIndex = pagination.pageIndex;
-        const pageSize = pagination.pageSize;
-        const rowIndex = Math.ceil(cell[1] - pageIndex * pageSize);
-
         if (!cellsUpdateFocus[cell[0]]) {
           cellsUpdateFocus[cell[0]] = [];
         }
 
-        cellsUpdateFocus[cell[0]][rowIndex] = updateFocus;
+        cellsUpdateFocus[cell[0]][cell[1]] = updateFocus;
 
         setCellsUpdateFocus(cellsUpdateFocus);
 
         return () => {
-          cellsUpdateFocus[cell[0]][rowIndex] = null;
+          cellsUpdateFocus[cell[0]][cell[1]] = null;
         };
       }
     },

diff --git a/src/components/datagrid/data_grid_body.tsx b/src/components/datagrid/data_grid_body.tsx
@@ -1,9 +1,4 @@
-import React, {
-  Fragment,
-  FunctionComponent,
-  useCallback,
-  useMemo,
-} from 'react';
+import React, { Fragment, FunctionComponent, useMemo } from 'react';
 // @ts-ignore-next-line
 import { EuiCodeBlock } from '../code';
 import {
@@ -163,30 +158,6 @@ export const EuiDataGridBody: FunctionComponent<
     return rowMap;
   }, [sorting, inMemory, inMemoryValues, schema, schemaDetectors]);
 
-  const setCellFocus = useCallback(
-    ([colIndex, rowIndex]) => {
-      // If the rows in the grid have been mapped in some way (e.g. sorting)
-      // then this callback must unmap the reported rowIndex
-      const mappedRowIndicies = Object.keys(rowMap);
-      let reverseMappedIndex = rowIndex;
-      for (let i = 0; i < mappedRowIndicies.length; i++) {
-        const mappedRowIndex = mappedRowIndicies[i];
-        const rowMappedToIndex = rowMap[(mappedRowIndex as any) as number];
-        if (`${rowMappedToIndex}` === `${rowIndex}`) {
-          reverseMappedIndex = parseInt(mappedRowIndex);
-          break;
-        }
-      }
-
-      // map the row into the visible rows
-      if (pagination) {
-        reverseMappedIndex -= pagination.pageIndex * pagination.pageSize;
-      }
-      onCellFocus([colIndex, reverseMappedIndex]);
-    },
-    [onCellFocus, rowMap, pagination]
-  );
-
   const rows = useMemo(() => {
     const rows = [];
     for (let i = 0; i < visibleRowIndices.length; i++) {
@@ -209,7 +180,7 @@ export const EuiDataGridBody: FunctionComponent<
           columnWidths={columnWidths}
           defaultColumnWidth={defaultColumnWidth}
           focusedCell={focusedCell}
-          onCellFocus={setCellFocus}
+          onCellFocus={onCellFocus}
           renderCellValue={renderCellValue}
           rowIndex={rowIndex}
           visibleRowIndex={i}