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

[DataGrid] Fix multiple focus behaviors #1421

Merged
merged 17 commits into from
Apr 28, 2021
Merged

[DataGrid] Fix multiple focus behaviors #1421

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e6979df
refacoring on focus cell and cols
dtassone Apr 15, 2021
62d5db9
refactor focus
dtassone Apr 19, 2021
021080c
cleanup
dtassone Apr 19, 2021
a2a9f75
Merge branch 'master' of github.com:mui-org/material-ui-x into cellFo…
dtassone Apr 21, 2021
9779067
add test for focus
dtassone Apr 21, 2021
09c285b
fix double tabbable and roving
dtassone Apr 22, 2021
fae10b2
fix test, cleanup
dtassone Apr 22, 2021
5dbc0ec
refactor input keydown handling
dtassone Apr 22, 2021
c8aa6e7
split checkbox renderer file
dtassone Apr 22, 2021
afeb0bc
Merge branch 'master' of github.com:mui-org/material-ui-x into cellFo…
dtassone Apr 26, 2021
dcd52a1
fix header focus selector
dtassone Apr 26, 2021
9d44f78
prettier
dtassone Apr 26, 2021
aad29c6
fix e2e tests
dtassone Apr 26, 2021
9d58c14
merge test from 1509
dtassone Apr 28, 2021
43406a8
Update packages/grid/x-grid/src/tests/editRows.XGrid.test.tsx
dtassone Apr 28, 2021
a718d99
Merge branch 'master' into cellFocusIssue
dtassone Apr 28, 2021
678b549
lint
dtassone Apr 28, 2021
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
6 changes: 2 additions & 4 deletions packages/grid/_modules_/grid/components/cell/GridCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ export const GridCell: React.FC<GridCellProps> = React.memo((props) => {
const valueToRender = formattedValue || value;
const cellRef = React.useRef<HTMLDivElement>(null);
const apiRef = React.useContext(GridApiContext);
const currentFocusedCell = apiRef!.current.getState().keyboard.cell;
const isCellFocused =
DanailH marked this conversation as resolved.
Show resolved Hide resolved
(currentFocusedCell && hasFocus) || (rowIndex === 0 && colIndex === 0 && !currentFocusedCell);

const cssClasses = classnames(
GRID_CELL_CSS_CLASS,
Expand All @@ -88,6 +85,7 @@ export const GridCell: React.FC<GridCellProps> = React.memo((props) => {
},
[apiRef, field, rowId],
);

const publishClick = React.useCallback(
(eventName: string) => (event: React.MouseEvent) => {
const params = apiRef!.current.getCellParams(rowId, field || '');
Expand Down Expand Up @@ -165,7 +163,7 @@ export const GridCell: React.FC<GridCellProps> = React.memo((props) => {
aria-colindex={colIndex}
style={style}
/* eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex */
tabIndex={isCellFocused ? 0 : -1}
tabIndex={hasFocus ? 0 : -1}
{...eventsHandlers}
>
{children || valueToRender?.toString()}
Expand Down
10 changes: 9 additions & 1 deletion packages/grid/_modules_/grid/components/columnHeaders/GridColumnHeaderItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import {
GRID_COLUMN_HEADER_DRAG_START,
GRID_COLUMN_HEADER_DRAG_END,
GRID_COLUMN_HEADER_FOCUS,
GRID_COLUMN_HEADER_BLUR,
} from '../../constants/eventsConstants';
import { gridKeyboardStateSelector } from '../../hooks/features/keyboard/gridKeyboardSelector';
import { GridColDef, GRID_NUMBER_COLUMN_TYPE } from '../../models/colDef/index';
import { GridOptions } from '../../models/gridOptions';
import { GridSortDirection } from '../../models/gridSortModel';
Expand Down Expand Up @@ -53,6 +55,7 @@ export const GridColumnHeaderItem = ({
const apiRef = React.useContext(GridApiContext);
const headerCellRef = React.useRef<HTMLDivElement>(null);
const headerHeight = useGridSelector(apiRef, gridDensityHeaderHeightSelector);
const keyboardState = useGridSelector(apiRef, gridKeyboardStateSelector);
const {
disableColumnReorder,
showColumnRightBorder,
Expand Down Expand Up @@ -93,6 +96,7 @@ export const GridColumnHeaderItem = ({
onMouseLeave: publish(GRID_COLUMN_HEADER_LEAVE),
onKeyDown: publish(GRID_COLUMN_HEADER_KEYDOWN),
onFocus: publish(GRID_COLUMN_HEADER_FOCUS),
onBlur: publish(GRID_COLUMN_HEADER_BLUR),
}),
[publish],
);
Expand Down Expand Up @@ -149,6 +153,10 @@ export const GridColumnHeaderItem = ({
}
});

const isFirstTabbable =
Copy link
Member

@DanailH DanailH Apr 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure that the first column header should be tabbable. I was left with the impression that the first cell should be tabbable.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the thing, IMO, the first tabbable control of a sortable grid should be the first sortable column header
https://www.w3.org/TR/wai-aria-practices/examples/grid/dataGrids.html

So in our case, I put the first column so we could argue that if none of the columns have an action then it should be the first cell of the first row

Copy link
Member

Choose a reason for hiding this comment

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

Actually, if we use that logic then the first tabbable cell should be either the column header that is sortable or the one that has a column menu as it is also a interaction.

keyboardState.cell === null && keyboardState.columnHeader === null && colIndex === 0;
const tabIndex = hasFocus || isFirstTabbable ? 0 : -1;

return (
<div
ref={headerCellRef}
Expand All @@ -161,7 +169,7 @@ export const GridColumnHeaderItem = ({
maxWidth: width,
}}
role="columnheader"
tabIndex={hasFocus ? 0 : -1}
tabIndex={tabIndex}
aria-colindex={colIndex + 1}
{...ariaSort}
{...mouseEventsHandlers}
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/constants/eventsConstants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const GRID_ROW_LEAVE = 'rowLeave';
export const GRID_ROW_EDIT_MODEL_CHANGE = 'editRowModelChange';
export const GRID_ROW_SELECTED = 'rowSelected';

export const GRID_COLUMN_HEADER_BLUR = 'columnHeaderBlur';
export const GRID_COLUMN_HEADER_FOCUS = 'columnHeaderFocus';
export const GRID_COLUMN_HEADER_NAVIGATION_KEYDOWN = 'columnHeaderNavigationKeydown';
export const GRID_COLUMN_HEADER_KEYDOWN = 'columnHeaderKeydown';
Expand Down
12 changes: 12 additions & 0 deletions packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import * as React from 'react';
import {
GRID_CELL_BLUR,
GRID_CELL_FOCUS,
GRID_CELL_NAVIGATION_KEYDOWN,
GRID_COLUMN_HEADER_BLUR,
GRID_COLUMN_HEADER_FOCUS,
GRID_COLUMN_HEADER_NAVIGATION_KEYDOWN,
} from '../../../constants/eventsConstants';
Expand Down Expand Up @@ -262,6 +264,14 @@ export const useGridKeyboardNavigation = (
[apiRef],
);

const handleBlur = React.useCallback(() => {
logger.debug(`Clearing focus`);
setGridState((previousState) => ({
...previousState,
keyboard: { ...previousState.keyboard, cell: null, columnHeader: null },
}));
}, [logger, setGridState]);

useGridApiMethod<GridNavigationApi>(
apiRef,
{
Expand All @@ -271,6 +281,8 @@ export const useGridKeyboardNavigation = (
'GridNavigationApi',
);
useGridApiEventHandler(apiRef, GRID_CELL_NAVIGATION_KEYDOWN, navigateCells);
useGridApiEventHandler(apiRef, GRID_COLUMN_HEADER_BLUR, handleBlur);
useGridApiEventHandler(apiRef, GRID_CELL_BLUR, handleBlur);
useGridApiEventHandler(apiRef, GRID_CELL_FOCUS, handleCellFocus);
useGridApiEventHandler(apiRef, GRID_COLUMN_HEADER_NAVIGATION_KEYDOWN, navigateColumnHeaders);
useGridApiEventHandler(apiRef, GRID_COLUMN_HEADER_FOCUS, handleColumnHeaderFocus);
Expand Down