From 6273301d2972637b6ea1187e0677dfb7d0835f0e Mon Sep 17 00:00:00 2001 From: Flavien DELANGLE Date: Wed, 3 Nov 2021 16:32:58 +0100 Subject: [PATCH] [DataGrid] Keyboard navigation broken on page > 0 (#3074) --- .../keyboard/useGridKeyboardNavigation.ts | 13 +++---- packages/grid/_modules_/grid/utils/utils.ts | 3 ++ .../src/tests/keyboard.DataGrid.test.tsx | 34 ++++++++++++------- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts index 80436f6a160e8..21999592a8703 100644 --- 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 index 44c540561f0e6..553d557747e07 100644 --- 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 index d9414cbd27b4c..48adb4b334f80 100644 --- 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(' - 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 & { + 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 ( -
+
); @@ -203,6 +201,16 @@ describe(' - Keyboard', () => { expect(getActiveCell()).to.equal('0-0'); }); + it('should support cell navigation with arrows when page > 0', () => { + render(); + 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(