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] Remove unnecessary keyboard navigation events #6863

Merged
merged 1 commit into from
Nov 23, 2022
Merged

[DataGrid] Remove unnecessary keyboard navigation events #6863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 1 addition & 15 deletions docs/data/data-grid/tree-data/TreeDataCustomGroupingColumn.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,6 @@ function CustomGridTreeDataGroupingCell(props) {

const filteredDescendantCount = filteredDescendantCountLookup[rowNode.id] ?? 0;

const handleKeyDown = (event) => {
if (event.key === ' ') {
event.stopPropagation();
}
if (isNavigationKey(event.key) && !event.shiftKey) {
apiRef.current.publishEvent('cellNavigationKeyDown', props, event);
}
};

const handleClick = (event) => {
if (rowNode.type !== 'group') {
return;
Expand All @@ -48,12 +39,7 @@ function CustomGridTreeDataGroupingCell(props) {
<Box sx={{ ml: rowNode.depth * 4 }}>
<div>
{filteredDescendantCount > 0 ? (
<Button
onClick={handleClick}
onKeyDown={handleKeyDown}
tabIndex={-1}
size="small"
>
<Button onClick={handleClick} tabIndex={-1} size="small">
See {filteredDescendantCount} employees
</Button>
) : (
Expand Down
16 changes: 1 addition & 15 deletions docs/data/data-grid/tree-data/TreeDataCustomGroupingColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,6 @@ function CustomGridTreeDataGroupingCell(props: GridRenderCellParams) {
);
const filteredDescendantCount = filteredDescendantCountLookup[rowNode.id] ?? 0;

const handleKeyDown: ButtonProps['onKeyDown'] = (event) => {
if (event.key === ' ') {
event.stopPropagation();
}
if (isNavigationKey(event.key) && !event.shiftKey) {
apiRef.current.publishEvent('cellNavigationKeyDown', props, event);
}
};

const handleClick: ButtonProps['onClick'] = (event) => {
if (rowNode.type !== 'group') {
return;
Expand All @@ -52,12 +43,7 @@ function CustomGridTreeDataGroupingCell(props: GridRenderCellParams) {
<Box sx={{ ml: rowNode.depth * 4 }}>
<div>
{filteredDescendantCount > 0 ? (
<Button
onClick={handleClick}
onKeyDown={handleKeyDown}
tabIndex={-1}
size="small"
>
<Button onClick={handleClick} tabIndex={-1} size="small">
See {filteredDescendantCount} employees
</Button>
) : (
Expand Down
10 changes: 0 additions & 10 deletions docs/data/data-grid/tree-data/TreeDataLazyLoading.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,6 @@ function GroupingCellWithLazyLoading(props) {
? rootProps.components.TreeDataCollapseIcon
: rootProps.components.TreeDataExpandIcon;

const handleKeyDown = (event) => {
if (event.key === ' ') {
event.stopPropagation();
}
if (isNavigationKey(event.key) && !event.shiftKey) {
apiRef.current.publishEvent('cellNavigationKeyDown', props, event);
}
};

const handleClick = (event) => {
apiRef.current.setRowChildrenExpansion(id, !rowNode.childrenExpanded);
apiRef.current.setCellFocus(id, field);
Expand All @@ -195,7 +186,6 @@ function GroupingCellWithLazyLoading(props) {
<IconButton
size="small"
onClick={handleClick}
onKeyDown={handleKeyDown}
tabIndex={-1}
aria-label={
rowNode.childrenExpanded
Expand Down
10 changes: 0 additions & 10 deletions docs/data/data-grid/tree-data/TreeDataLazyLoading.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,15 +193,6 @@ function GroupingCellWithLazyLoading(props: GroupingCellWithLazyLoadingProps) {
? rootProps.components.TreeDataCollapseIcon
: rootProps.components.TreeDataExpandIcon;

const handleKeyDown: IconButtonProps['onKeyDown'] = (event) => {
if (event.key === ' ') {
event.stopPropagation();
}
if (isNavigationKey(event.key) && !event.shiftKey) {
apiRef.current.publishEvent('cellNavigationKeyDown', props, event);
}
};

const handleClick: IconButtonProps['onClick'] = (event) => {
apiRef.current.setRowChildrenExpansion(id, !rowNode.childrenExpanded);
apiRef.current.setCellFocus(id, field);
Expand All @@ -215,7 +206,6 @@ function GroupingCellWithLazyLoading(props: GroupingCellWithLazyLoadingProps) {
<IconButton
size="small"
onClick={handleClick}
onKeyDown={handleKeyDown}
tabIndex={-1}
aria-label={
rowNode.childrenExpanded
Expand Down
2 changes: 2 additions & 0 deletions docs/data/migration/migration-data-grid-v5/migration-data-grid-v5.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ Below are described the steps you need to make to migrate from v5 to v6.

- The `selectionChange` event was renamed to `rowSelectionChange`.
- The `columnVisibilityChange` event was removed. Use `columnVisibilityModelChange` instead.
- The `cellNavigationKeyDown` event was removed. Use `cellKeyDown` and check the key provided in the event argument.
- The `columnHeaderNavigationKeyDown` event was removed. Use `columnHeaderKeyDown` and check the key provided in the event argument.
- The `GridCallbackDetails['api']` was removed from event details. Use the `apiRef` returned by `useGridApiContext` or `useGridApiRef` instead.

### Columns
Expand Down
2 changes: 2 additions & 0 deletions packages/grid/x-data-grid-premium/src/components/GridGroupingCriteriaCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export function GridGroupingCriteriaCell(props: GridGroupingCriteriaCellProps) {

const handleKeyDown = (event: React.KeyboardEvent<HTMLButtonElement>) => {
if (event.key === ' ') {
// We call event.stopPropagation to avoid unfolding the row and also scrolling to bottom
// TODO: Remove and add a check inside useGridKeyboardNavigation
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm referring to

mui-x/packages/grid/x-data-grid/src/hooks/features/keyboardNavigation/useGridKeyboardNavigation.ts

Lines 173 to 181 in 4d77507

case ' ': {
const field = (params as GridCellParams).field;
if (field === GRID_DETAIL_PANEL_TOGGLE_FIELD) {
break;
}
const colDef = (params as GridCellParams).colDef;
if (colDef && colDef.type === 'treeDataGroup') {
break;
}

Copy link
Member

Choose a reason for hiding this comment

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

Maybe create a separate issue for it and link to it in the TODO comment?

event.stopPropagation();
}
apiRef.current.publishEvent('cellKeyDown', props, event);
Expand Down
11 changes: 0 additions & 11 deletions packages/grid/x-data-grid-pro/src/components/GridTreeDataGroupingCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
GridRenderCellParams,
GridGroupNode,
} from '@mui/x-data-grid';
import { isNavigationKey } from '@mui/x-data-grid/internals';
import { useGridRootProps } from '../hooks/utils/useGridRootProps';
import { useGridApiContext } from '../hooks/utils/useGridApiContext';
import { DataGridProProcessedProps } from '../models/dataGridProProps';
Expand Down Expand Up @@ -55,15 +54,6 @@ function GridTreeDataGroupingCell(props: GridTreeDataGroupingCellProps) {
? rootProps.components.TreeDataCollapseIcon
: rootProps.components.TreeDataExpandIcon;

const handleKeyDown = (event: React.KeyboardEvent<HTMLButtonElement>) => {
if (event.key === ' ') {
event.stopPropagation();
Copy link
Member Author

@m4theushw m4theushw Nov 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This event.stopPropagation() can be removed here because the listener already ignores the space key if it comes from the tree data cell.

mui-x/packages/grid/x-data-grid/src/hooks/features/keyboardNavigation/useGridKeyboardNavigation.ts

Lines 178 to 181 in 4d77507

const colDef = (params as GridCellParams).colDef;
if (colDef && colDef.type === 'treeDataGroup') {
break;
}

This also explains its removal from the demos.

}
if (isNavigationKey(event.key) && !event.shiftKey) {
apiRef.current.publishEvent('cellNavigationKeyDown', props, event);
}
};

const handleClick = (event: React.MouseEvent<HTMLButtonElement>) => {
apiRef.current.setRowChildrenExpansion(id, !rowNode.childrenExpanded);
apiRef.current.setCellFocus(id, field);
Expand All @@ -77,7 +67,6 @@ function GridTreeDataGroupingCell(props: GridTreeDataGroupingCellProps) {
<IconButton
size="small"
onClick={handleClick}
onKeyDown={handleKeyDown}
tabIndex={-1}
aria-label={
rowNode.childrenExpanded
Expand Down
20 changes: 8 additions & 12 deletions packages/grid/x-data-grid/src/components/columnSelection/GridCellCheckboxRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
unstable_useForkRef as useForkRef,
} from '@mui/utils';
import { GridRenderCellParams } from '../../models/params/gridCellParams';
import { isNavigationKey, isSpaceKey } from '../../utils/keyboardUtils';
import { isSpaceKey } from '../../utils/keyboardUtils';
import { useGridApiContext } from '../../hooks/utils/useGridApiContext';
import { useGridRootProps } from '../../hooks/utils/useGridRootProps';
import { getDataGridUtilityClass } from '../../constants/gridClasses';
Expand Down Expand Up @@ -76,17 +76,13 @@ const GridCellCheckboxForwardRef = React.forwardRef<HTMLInputElement, GridRender
}
}, [hasFocus]);

const handleKeyDown = React.useCallback(
(event: React.KeyboardEvent<HTMLInputElement>) => {
if (isSpaceKey(event.key)) {
event.stopPropagation();
}
if (isNavigationKey(event.key) && !event.shiftKey) {
apiRef.current.publishEvent('cellNavigationKeyDown', props, event);
}
},
[apiRef, props],
);
const handleKeyDown = React.useCallback((event: React.KeyboardEvent<HTMLInputElement>) => {
if (isSpaceKey(event.key)) {
// We call event.stopPropagation to avoid selecting the row and also scrolling to bottom
// TODO: Remove and add a check inside useGridKeyboardNavigation
event.stopPropagation();
}
}, []);

if (rowNode.type === 'footer' || rowNode.type === 'pinnedRow') {
return null;
Expand Down
7 changes: 1 addition & 6 deletions packages/grid/x-data-grid/src/components/columnSelection/GridHeaderCheckbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { useGridSelector } from '../../hooks/utils/useGridSelector';
import { gridTabIndexColumnHeaderSelector } from '../../hooks/features/focus/gridFocusStateSelector';
import { gridRowSelectionStateSelector } from '../../hooks/features/rowSelection/gridRowSelectionSelector';
import { GridColumnHeaderParams } from '../../models/params/gridColumnHeaderParams';
import { isNavigationKey } from '../../utils/keyboardUtils';
import { useGridApiContext } from '../../hooks/utils/useGridApiContext';
import { getDataGridUtilityClass } from '../../constants/gridClasses';
import { useGridRootProps } from '../../hooks/utils/useGridRootProps';
Expand Down Expand Up @@ -113,12 +112,8 @@ const GridHeaderCheckbox = React.forwardRef<HTMLInputElement, GridColumnHeaderPa
value: !isChecked,
});
}
// TODO v6 remove columnHeaderNavigationKeyDown events which are not used internally anymore
if (isNavigationKey(event.key) && !event.shiftKey) {
apiRef.current.publishEvent('columnHeaderNavigationKeyDown', props, event);
}
},
[apiRef, props, isChecked],
[apiRef, isChecked],
);

const handleSelectionChange = React.useCallback(() => {
Expand Down
Loading