[DataGrid] Keyboard navigation broken on page > 0 (#3074)

mui · Nov 3, 2021 · 6273301 · 6273301
1 parent 9fbc568
commit 6273301
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 22 deletions.
diff --git a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts
@@ -23,6 +23,7 @@ import { useGridApiEventHandler } from '../../utils/useGridApiEventHandler';
 import { GridComponentProps } from '../../../GridComponentProps';
 import { gridVisibleSortedRowEntriesSelector } from '../filter/gridFilterSelector';
 import { useCurrentPageRows } from '../../utils/useCurrentPageRows';
+import { clamp } from '../../../utils/utils';
 
 const getNextCellIndexes = (key: string, indexes: GridCellIndexCoordinates) => {
   if (!isArrowKeys(key)) {
@@ -102,7 +103,6 @@ export const useGridKeyboardNavigation = (
 
       const key = mapKey(event);
       const isCtrlPressed = event.ctrlKey || event.metaKey || event.shiftKey;
-      const rowCount = currentPage.range.lastRowIndex - currentPage.range.firstRowIndex + 1;
 
       let nextCellIndexes: GridCellIndexCoordinates;
       if (isArrowKeys(key)) {
@@ -122,7 +122,7 @@ export const useGridKeyboardNavigation = (
           if (colIdx === 0) {
             newRowIndex = currentPage.range.firstRowIndex;
           } else {
-            newRowIndex = rowCount - 1;
+            newRowIndex = currentPage.range.lastRowIndex;
           }
           nextCellIndexes = { colIndex: colIdx, rowIndex: newRowIndex };
         }
@@ -143,13 +143,8 @@ export const useGridKeyboardNavigation = (
         return;
       }
 
-      nextCellIndexes.rowIndex =
-        nextCellIndexes.rowIndex >= rowCount && rowCount > 0
-          ? rowCount - 1
-          : nextCellIndexes.rowIndex;
-      nextCellIndexes.colIndex = nextCellIndexes.colIndex <= 0 ? 0 : nextCellIndexes.colIndex;
-      nextCellIndexes.colIndex =
-        nextCellIndexes.colIndex >= colCount ? colCount - 1 : nextCellIndexes.colIndex;
+      nextCellIndexes.rowIndex = clamp(nextCellIndexes.rowIndex, 0, currentPage.range.lastRowIndex);
+      nextCellIndexes.colIndex = clamp(nextCellIndexes.colIndex, 0, colCount - 1);
       logger.debug(
         `Navigating to next cell row ${nextCellIndexes.rowIndex}, col ${nextCellIndexes.colIndex}`,
       );

diff --git a/packages/grid/_modules_/grid/utils/utils.ts b/packages/grid/_modules_/grid/utils/utils.ts
@@ -31,3 +31,6 @@ export function localStorageAvailable() {
 export function escapeRegExp(value: string): string {
   return value.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&');
 }
+
+export const clamp = (value: number, min: number, max: number) =>
+  Math.min(max, Math.max(min, value));
diff --git a/packages/grid/data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/data-grid/src/tests/keyboard.DataGrid.test.tsx
@@ -20,7 +20,7 @@ import {
   getColumnValues,
   getRow,
 } from 'test/utils/helperFn';
-import { DataGrid } from '@mui/x-data-grid';
+import { DataGrid, DataGridProps } from '@mui/x-data-grid';
 import { useData } from 'packages/storybook/src/hooks/useData';
 import { GridColumns } from 'packages/grid/_modules_/grid/models/colDef/gridColDef';
 
@@ -128,26 +128,24 @@ describe('<DataGrid /> - Keyboard', () => {
     expect(handleKeyDown.returnValues).to.deep.equal([true]);
   });
 
-  const KeyboardTest = (props: {
-    nbRows?: number;
-    checkboxSelection?: boolean;
-    disableVirtualization?: boolean;
-    filterModel?: any;
-    width?: number;
-  }) => {
-    const data = useData(props.nbRows || 100, 20);
+  const KeyboardTest = (
+    props: Omit<DataGridProps, 'autoHeight' | 'rows' | 'columns'> & {
+      width?: number;
+      nbRows?: number;
+    },
+  ) => {
+    const { nbRows = 100, width = 300, ...other } = props;
+    const data = useData(nbRows, 20);
     const transformColSizes = (columns: GridColumns) =>
       columns.map((column) => ({ ...column, width: 60 }));
 
     return (
-      <div style={{ width: props.width || 300, height: 360 }}>
+      <div style={{ width, height: 360 }}>
         <DataGrid
           autoHeight={isJSDOM}
           rows={data.rows}
           columns={transformColSizes(data.columns)}
-          checkboxSelection={props.checkboxSelection}
-          disableVirtualization={props.disableVirtualization}
-          filterModel={props.filterModel}
+          {...other}
         />
       </div>
     );
@@ -203,6 +201,16 @@ describe('<DataGrid /> - Keyboard', () => {
     expect(getActiveCell()).to.equal('0-0');
   });
 
+  it('should support cell navigation with arrows when page > 0', () => {
+    render(<KeyboardTest nbRows={10} page={1} pageSize={2} rowsPerPageOptions={[2]} />);
+    getCell(2, 0).focus();
+    expect(getActiveCell()).to.equal('2-0');
+    fireEvent.keyDown(document.activeElement!, { key: 'ArrowDown' });
+    expect(getActiveCell()).to.equal('3-0');
+    fireEvent.keyDown(document.activeElement!, { key: 'ArrowUp' });
+    expect(getActiveCell()).to.equal('2-0');
+  });
+
   it('should support cell navigation with arrows when rows are filtered', () => {
     render(
       <KeyboardTest