From 6378a99cdbe6fdaa0850cf2e31653ed5a6a11ac9 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 13 Nov 2024 11:05:58 -0600 Subject: [PATCH 1/3] Use `flushSync` to focus --- src/DataGrid.tsx | 53 ++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/DataGrid.tsx b/src/DataGrid.tsx index 593ed97154..5e8b08d518 100644 --- a/src/DataGrid.tsx +++ b/src/DataGrid.tsx @@ -298,7 +298,6 @@ function DataGrid( const [isDragging, setDragging] = useState(false); const [draggedOverRowIdx, setOverRowIdx] = useState(undefined); const [scrollToPosition, setScrollToPosition] = useState(null); - const [shouldFocusCell, setShouldFocusCell] = useState(false); const [previousRowIdx, setPreviousRowIdx] = useState(-1); const getColumnWidth = useCallback( @@ -468,16 +467,6 @@ function DataGrid( latestDraggedOverRowIdx.current = rowIdx; }, []); - const focusCellOrCellContent = useCallback(() => { - const cell = getCellToScroll(gridRef.current!); - if (cell === null) return; - - scrollIntoView(cell); - // Focus cell content when available instead of the cell itself - const elementToFocus = cell.querySelector('[tabindex="0"]') ?? cell; - elementToFocus.focus({ preventScroll: true }); - }, [gridRef]); - /** * effects */ @@ -492,13 +481,6 @@ function DataGrid( } }, [selectedCellIsWithinSelectionBounds, selectedPosition]); - useLayoutEffect(() => { - if (shouldFocusCell) { - setShouldFocusCell(false); - focusCellOrCellContent(); - } - }, [shouldFocusCell, focusCellOrCellContent]); - useImperativeHandle(ref, () => ({ element: gridRef.current, scrollToCell({ idx, rowIdx }) { @@ -751,6 +733,16 @@ function DataGrid( ); } + function focusCellOrCellContent() { + const cell = getCellToScroll(gridRef.current!); + if (cell === null) return; + + scrollIntoView(cell); + // Focus cell content when available instead of the cell itself + const elementToFocus = cell.querySelector('[tabindex="0"]') ?? cell; + elementToFocus.focus({ preventScroll: true }); + } + function selectCell(position: Position, enableEditor?: Maybe): void { if (!isCellWithinSelectionBounds(position)) return; commitEditorChanges(); @@ -764,8 +756,10 @@ function DataGrid( // Avoid re-renders if the selected cell state is the same scrollIntoView(getCellToScroll(gridRef.current!)); } else { - setShouldFocusCell(true); - setSelectedPosition({ ...position, mode: 'SELECT' }); + flushSync(() => { + setSelectedPosition({ ...position, mode: 'SELECT' }); + }); + focusCellOrCellContent(); } if (onSelectedCellChange && !samePosition) { @@ -913,6 +907,10 @@ function DataGrid( ); } + function cancelEditMode() { + setSelectedPosition(({ idx, rowIdx }) => ({ idx, rowIdx, mode: 'SELECT' })); + } + function getCellEditor(rowIdx: number) { if (selectedPosition.rowIdx !== rowIdx || selectedPosition.mode === 'SELECT') return; @@ -921,8 +919,12 @@ function DataGrid( const colSpan = getColSpan(column, lastFrozenColumnIndex, { type: 'ROW', row }); const closeEditor = (shouldFocusCell: boolean) => { - setShouldFocusCell(shouldFocusCell); - setSelectedPosition(({ idx, rowIdx }) => ({ idx, rowIdx, mode: 'SELECT' })); + if (shouldFocusCell) { + flushSync(cancelEditMode); + focusCellOrCellContent(); + } else { + cancelEditMode(); + } }; const onRowChange = (row: R, commitChanges: boolean, shouldFocusCell: boolean) => { @@ -933,8 +935,11 @@ function DataGrid( // SELECT and this results in onRowChange getting called twice. flushSync(() => { updateRow(column, selectedPosition.rowIdx, row); - closeEditor(shouldFocusCell); + cancelEditMode(); }); + if (shouldFocusCell) { + focusCellOrCellContent(); + } } else { setSelectedPosition((position) => ({ ...position, row })); } @@ -942,7 +947,7 @@ function DataGrid( if (rows[selectedPosition.rowIdx] !== selectedPosition.originalRow) { // Discard changes if rows are updated from outside - closeEditor(false); + cancelEditMode(); } return ( From 8b8f16151a2f4cb200050b52e82b966c1221b264 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 13 Nov 2024 11:12:53 -0600 Subject: [PATCH 2/3] Fix tests --- src/cellRenderers/renderCheckbox.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/cellRenderers/renderCheckbox.tsx b/src/cellRenderers/renderCheckbox.tsx index a24ac0ebe4..93dd532167 100644 --- a/src/cellRenderers/renderCheckbox.tsx +++ b/src/cellRenderers/renderCheckbox.tsx @@ -22,9 +22,14 @@ const checkbox = css` const checkboxClassname = `rdg-checkbox-input ${checkbox}`; -export function renderCheckbox({ onChange, indeterminate, ...props }: RenderCheckboxProps) { +export function renderCheckbox({ + onChange, + indeterminate, + checked, + ...props +}: RenderCheckboxProps) { function handleChange(e: React.ChangeEvent) { - onChange(e.target.checked, (e.nativeEvent as MouseEvent).shiftKey); + onChange(!checked, (e.nativeEvent as MouseEvent).shiftKey); } return ( @@ -36,6 +41,7 @@ export function renderCheckbox({ onChange, indeterminate, ...props }: RenderChec }} type="checkbox" className={checkboxClassname} + checked={checked} onChange={handleChange} {...props} /> From 1adb7d049bbb664b272ce9d8081137b70260acbd Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 13 Nov 2024 11:24:19 -0600 Subject: [PATCH 3/3] Add issue link --- src/cellRenderers/renderCheckbox.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cellRenderers/renderCheckbox.tsx b/src/cellRenderers/renderCheckbox.tsx index 93dd532167..be2c798f62 100644 --- a/src/cellRenderers/renderCheckbox.tsx +++ b/src/cellRenderers/renderCheckbox.tsx @@ -29,6 +29,8 @@ export function renderCheckbox({ ...props }: RenderCheckboxProps) { function handleChange(e: React.ChangeEvent) { + // https://github.com/facebook/react/issues/31358 + // onChange(e.target.checked, (e.nativeEvent as MouseEvent).shiftKey); onChange(!checked, (e.nativeEvent as MouseEvent).shiftKey); }