Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use flushSync to focus #3630

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Use flushSync to focus #3630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6378a99
Use `flushSync` to focus
amanmahajan7 Nov 13, 2024
8b8f161
Fix tests
amanmahajan7 Nov 13, 2024
1adb7d0
Add issue link
amanmahajan7 Nov 13, 2024
3f1142f
Merge branch 'main' into am-fix-act-warnings
amanmahajan7 Nov 15, 2024
e4eb8e7
Merge branch 'main' into am-fix-act-warnings
amanmahajan7 Nov 21, 2024
373e7a3
Merge branch 'main' into am-fix-act-warnings
amanmahajan7 Nov 22, 2024
a664328
Merge branch 'main' into am-fix-act-warnings
amanmahajan7 Dec 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 29 additions & 24 deletions src/DataGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@ function DataGrid<R, SR, K extends Key>(
const [isDragging, setDragging] = useState(false);
const [draggedOverRowIdx, setOverRowIdx] = useState<number | undefined>(undefined);
const [scrollToPosition, setScrollToPosition] = useState<PartialPosition | null>(null);
const [shouldFocusCell, setShouldFocusCell] = useState(false);
const [previousRowIdx, setPreviousRowIdx] = useState(-1);

const getColumnWidth = useCallback(
Expand Down Expand Up @@ -470,16 +469,6 @@ function DataGrid<R, SR, K extends Key>(
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<Element & HTMLOrSVGElement>('[tabindex="0"]') ?? cell;
elementToFocus.focus({ preventScroll: true });
}, [gridRef]);

/**
* effects
*/
Expand All @@ -494,13 +483,6 @@ function DataGrid<R, SR, K extends Key>(
}
}, [selectedCellIsWithinSelectionBounds, selectedPosition]);

useLayoutEffect(() => {
if (shouldFocusCell) {
setShouldFocusCell(false);
focusCellOrCellContent();
}
}, [shouldFocusCell, focusCellOrCellContent]);

useImperativeHandle(ref, () => ({
element: gridRef.current,
scrollToCell({ idx, rowIdx }) {
Expand Down Expand Up @@ -753,6 +735,16 @@ function DataGrid<R, SR, K extends Key>(
);
}

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<Element & HTMLOrSVGElement>('[tabindex="0"]') ?? cell;
elementToFocus.focus({ preventScroll: true });
}

function selectCell(position: Position, enableEditor?: Maybe<boolean>): void {
if (!isCellWithinSelectionBounds(position)) return;
commitEditorChanges();
Expand All @@ -766,8 +758,10 @@ function DataGrid<R, SR, K extends Key>(
// 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) {
Expand Down Expand Up @@ -915,6 +909,10 @@ function DataGrid<R, SR, K extends Key>(
);
}

function cancelEditMode() {
setSelectedPosition(({ idx, rowIdx }) => ({ idx, rowIdx, mode: 'SELECT' }));
}

function getCellEditor(rowIdx: number) {
if (selectedPosition.rowIdx !== rowIdx || selectedPosition.mode === 'SELECT') return;

Expand All @@ -923,8 +921,12 @@ function DataGrid<R, SR, K extends Key>(
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) => {
Expand All @@ -935,16 +937,19 @@ function DataGrid<R, SR, K extends Key>(
// 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 }));
}
};

if (rows[selectedPosition.rowIdx] !== selectedPosition.originalRow) {
// Discard changes if rows are updated from outside
closeEditor(false);
cancelEditMode();
}

return (
Expand Down
12 changes: 10 additions & 2 deletions src/cellRenderers/renderCheckbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,16 @@ 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<HTMLInputElement>) {
onChange(e.target.checked, (e.nativeEvent as MouseEvent).shiftKey);
// https://github.com/facebook/react/issues/31358
// onChange(e.target.checked, (e.nativeEvent as MouseEvent).shiftKey);
onChange(!checked, (e.nativeEvent as MouseEvent).shiftKey);
}

return (
Expand All @@ -36,6 +43,7 @@ export function renderCheckbox({ onChange, indeterminate, ...props }: RenderChec
}}
type="checkbox"
className={checkboxClassname}
checked={checked}
onChange={handleChange}
{...props}
/>
Expand Down
Loading