Skip to content

Fix cell stealing focus on click #3790

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.

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

Merged
merged 6 commits into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 2 additions & 2 deletions src/Cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ function Cell<R, SR>({
);
const isEditable = isCellEditableUtil(column, row);

function selectCellWrapper(openEditor?: boolean) {
selectCell({ rowIdx, idx: column.idx }, openEditor);
function selectCellWrapper(enableEditor?: boolean) {
selectCell({ rowIdx, idx: column.idx }, { enableEditor });
}

function handleMouseEvent(
Expand Down
30 changes: 14 additions & 16 deletions src/DataGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import type {
Position,
Renderers,
RowsChangeData,
SelectCellOptions,
SelectHeaderRowEvent,
SelectRowEvent,
SortColumn
Expand Down Expand Up @@ -109,7 +110,7 @@ export type DefaultColumnOptions<R, SR> = Pick<
export interface DataGridHandle {
element: HTMLDivElement | null;
scrollToCell: (position: PartialPosition) => void;
selectCell: (position: Position, enableEditor?: Maybe<boolean>) => void;
selectCell: (position: Position, options: SelectCellOptions) => void;
}

type SharedDivProps = Pick<
Expand Down Expand Up @@ -502,7 +503,7 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
/**
* callbacks
*/
const focusCellOrCellContent = useCallback(
const focusCell = useCallback(
(shouldScroll = true) => {
const cell = getCellToScroll(gridRef.current!);
if (cell === null) return;
Expand All @@ -511,10 +512,7 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
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 });
cell.focus({ preventScroll: true });
},
[gridRef]
);
Expand All @@ -528,11 +526,11 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
focusSinkRef.current.focus({ preventScroll: true });
scrollIntoView(focusSinkRef.current);
} else {
focusCellOrCellContent();
focusCell();
}
setShouldFocusCell(false);
}
}, [shouldFocusCell, focusCellOrCellContent, selectedPosition.idx]);
}, [shouldFocusCell, focusCell, selectedPosition.idx]);

useImperativeHandle(ref, () => ({
element: gridRef.current,
Expand Down Expand Up @@ -654,7 +652,7 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
function handleFocus(event: React.FocusEvent<HTMLDivElement>) {
// select the first header cell if the focus event is triggered by the grid
if (event.target === event.currentTarget) {
selectHeaderCell({ idx: minColIdx, rowIdx: headerRowsCount });
selectHeaderCell({ idx: minColIdx, rowIdx: headerRowsCount }, { shouldFocusCell: true });
}
}

Expand Down Expand Up @@ -777,7 +775,7 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr

function handleDragHandleClick() {
// keep the focus on the cell but do not scroll
focusCellOrCellContent(false);
focusCell(false);
}

function handleDragHandleDoubleClick(event: React.MouseEvent<HTMLDivElement>) {
Expand Down Expand Up @@ -838,20 +836,20 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
);
}

function selectCell(position: Position, enableEditor?: Maybe<boolean>): void {
function selectCell(position: Position, options?: SelectCellOptions): void {
if (!isCellWithinSelectionBounds(position)) return;
commitEditorChanges();

const samePosition = isSamePosition(selectedPosition, position);

if (enableEditor && isCellEditable(position)) {
if (options?.enableEditor && isCellEditable(position)) {
const row = rows[position.rowIdx];
setSelectedPosition({ ...position, mode: 'EDIT', row, originalRow: row });
} else if (samePosition) {
// Avoid re-renders if the selected cell state is the same
scrollIntoView(getCellToScroll(gridRef.current!));
} else {
setShouldFocusCell(true);
setShouldFocusCell(options?.shouldFocusCell === true);
setSelectedPosition({ ...position, mode: 'SELECT' });
}

Expand All @@ -864,8 +862,8 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
}
}

function selectHeaderCell({ idx, rowIdx }: Position) {
selectCell({ rowIdx: minRowIdx + rowIdx - 1, idx });
function selectHeaderCell({ idx, rowIdx }: Position, options?: SelectCellOptions): void {
selectCell({ rowIdx: minRowIdx + rowIdx - 1, idx }, options);
}

function getNextPosition(key: string, ctrlKey: boolean, shiftKey: boolean): Position {
Expand Down Expand Up @@ -952,7 +950,7 @@ export function DataGrid<R, SR = unknown, K extends Key = Key>(props: DataGridPr
isCellWithinBounds: isCellWithinSelectionBounds
});

selectCell(nextSelectedCellPosition);
selectCell(nextSelectedCellPosition, { shouldFocusCell: true });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need to set shouldFocusCell flag on cell navigation using keyboard.

}

function getDraggedOverCellIdx(currentRowIdx: number): number | undefined {
Expand Down
2 changes: 1 addition & 1 deletion src/GroupRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function GroupedRow<R, SR>({
const idx = viewportColumns[0].key === SELECT_COLUMN_KEY ? row.level + 1 : row.level;

function handleSelectGroup() {
selectCell({ rowIdx, idx: -1 });
selectCell({ rowIdx, idx: -1 }, { shouldFocusCell: true });
}

const selectionValue = useMemo(
Expand Down
8 changes: 7 additions & 1 deletion src/hooks/useRovingTabIndex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ export function useRovingTabIndex(isSelected: boolean) {
}

function onFocus(event: React.FocusEvent<HTMLDivElement>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a cell is clicked it is automatically focused as the cell has a tabIndex: -1. We can focus the child here

if (event.target !== event.currentTarget) {
const elementToFocus = event.currentTarget.querySelector<Element & HTMLOrSVGElement>(
'[tabindex="0"]'
);

// Focus cell content when available instead of the cell itself
if (elementToFocus !== null) {
elementToFocus.focus({ preventScroll: true });
setIsChildFocused(true);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export type {
RenderSummaryCellProps,
RowHeightArgs,
RowsChangeData,
SelectCellOptions,
SelectHeaderRowEvent,
SelectRowEvent,
SortColumn,
Expand Down
9 changes: 7 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ interface BaseCellRendererProps<TRow, TSummaryRow = unknown>
'onCellMouseDown' | 'onCellClick' | 'onCellDoubleClick' | 'onCellContextMenu'
> {
rowIdx: number;
selectCell: (position: Position, enableEditor?: Maybe<boolean>) => void;
selectCell: (position: Position, options?: SelectCellOptions) => void;
}

export interface CellRendererProps<TRow, TSummaryRow>
Expand Down Expand Up @@ -203,7 +203,7 @@ interface SelectCellKeyDownArgs<TRow, TSummaryRow = unknown> {
column: CalculatedColumn<TRow, TSummaryRow>;
row: TRow;
rowIdx: number;
selectCell: (position: Position, enableEditor?: Maybe<boolean>) => void;
selectCell: (position: Position, options?: SelectCellOptions) => void;
}

export interface EditCellKeyDownArgs<TRow, TSummaryRow = unknown> {
Expand Down Expand Up @@ -334,6 +334,11 @@ export interface Renderers<TRow, TSummaryRow> {
noRowsFallback?: Maybe<ReactNode>;
}

export interface SelectCellOptions {
enableEditor?: Maybe<boolean>;
shouldFocusCell?: Maybe<boolean>;
}

export interface ColumnWidth {
readonly type: 'resized' | 'measured';
readonly width: number;
Expand Down
29 changes: 29 additions & 0 deletions test/browser/column/renderCell.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,35 @@ describe('Custom cell renderer', () => {
});
});

test('Focus child if it sets tabIndex', async () => {
const column: Column<Row> = {
key: 'test',
name: 'test',
renderCell(props) {
return (
<>
<button type="button" tabIndex={props.tabIndex}>
value: {props.row.id}
</button>
<span>External Text</span>
</>
);
}
};

page.render(<DataGrid columns={[column]} rows={[{ id: 1 }]} />);

const [cell] = getCells();
const button = page.getByRole('button', { name: 'value: 1' });
expect(cell).toHaveTextContent('value: 1External Text');
await userEvent.click(page.getByText('External Text'));
expect(button).toHaveFocus();
await userEvent.tab();
expect(button).not.toHaveFocus();
await userEvent.click(button);
expect(button).toHaveFocus();
});

test('Cell should not steal focus when the focus is outside the grid and cell is recreated', async () => {
const columns: readonly Column<Row>[] = [{ key: 'id', name: 'ID' }];

Expand Down