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.

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 input element in custom header #3624

Merged
merged 29 commits into from
Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4c3f29d
fix the regression
alexfauquette Jan 25, 2022
bdc8bcf
add tests
alexfauquette Jan 25, 2022
e3d553d
feedback
alexfauquette Jan 27, 2022
7492d5e
Merge remote-tracking branch 'upstream/master' into issue-3599
alexfauquette Jan 27, 2022
e1355e8
test to prevent focus
alexfauquette Feb 7, 2022
90be856
prettier
alexfauquette Feb 7, 2022
6fa46ae
Merge remote-tracking branch 'upstream/master' into issue-3599
alexfauquette Feb 7, 2022
1cc0e76
Merge remote-tracking branch 'upstream/master' into issue-3599
alexfauquette Feb 17, 2022
25c21cb
Merge remote-tracking branch 'upstream/master' into issue-3599
alexfauquette Feb 23, 2022
bf009e0
fix className
alexfauquette Feb 24, 2022
01e4a78
remove one test
alexfauquette Feb 24, 2022
6c4a2bc
prettier
alexfauquette Feb 24, 2022
5bb32e1
keep innput test with exmplaination
alexfauquette Feb 24, 2022
70137b8
prettier
alexfauquette Feb 24, 2022
a927737
remove buttons from the container
alexfauquette Feb 25, 2022
cc67c2d
feedbacks
alexfauquette Feb 28, 2022
69c41ec
docs:api
alexfauquette Feb 28, 2022
e06da3d
Merge remote-tracking branch 'upstream/master' into issue-3599
alexfauquette Feb 28, 2022
dda1b40
add the 'not' operator
alexfauquette Feb 28, 2022
8345824
fix style
alexfauquette Feb 28, 2022
492f7ce
renaming
alexfauquette Feb 28, 2022
701d17c
fix tests
alexfauquette Feb 28, 2022
6ba52c1
move eslint line
alexfauquette Feb 28, 2022
39094de
remove isGridHeaderCellRoot
alexfauquette Feb 28, 2022
95c41b8
add test
alexfauquette Feb 28, 2022
9368b11
Merge remote-tracking branch 'upstream/master' into issue-3599
alexfauquette Mar 1, 2022
e6bde22
fix import
alexfauquette Mar 1, 2022
27c80e5
Update packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx
alexfauquette Mar 2, 2022
4ccbbc7
Trigger CI
alexfauquette Mar 2, 2022
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
1 change: 1 addition & 0 deletions docs/pages/api-docs/data-grid/data-grid-pro.json
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@
"columnHeaderDropZone",
"columnHeaderTitle",
"columnHeaderTitleContainer",
"columnHeaderTitleContainerContent",
"columnHeaders",
"columnHeadersInner",
"columnHeadersInner--scrollable",
Expand Down
1 change: 1 addition & 0 deletions docs/pages/api-docs/data-grid/data-grid.json
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@
"columnHeaderDropZone",
"columnHeaderTitle",
"columnHeaderTitleContainer",
"columnHeaderTitleContainerContent",
"columnHeaders",
"columnHeadersInner",
"columnHeadersInner--scrollable",
Expand Down
1 change: 1 addition & 0 deletions docs/pages/x/api/data-grid/data-grid-pro.json
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@
"columnHeaderDropZone",
"columnHeaderTitle",
"columnHeaderTitleContainer",
"columnHeaderTitleContainerContent",
"columnHeaders",
"columnHeadersInner",
"columnHeadersInner--scrollable",
Expand Down
1 change: 1 addition & 0 deletions docs/pages/x/api/data-grid/data-grid.json
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@
"columnHeaderDropZone",
"columnHeaderTitle",
"columnHeaderTitleContainer",
"columnHeaderTitleContainerContent",
"columnHeaders",
"columnHeadersInner",
"columnHeadersInner--scrollable",
Expand Down
4 changes: 4 additions & 0 deletions docs/translations/api-docs/data-grid/data-grid-pro-pt.json
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title container element"
},
"columnHeaderTitleContainerContent": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title excepted buttons"
},
"columnHeaders": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column headers"
Expand Down
4 changes: 4 additions & 0 deletions docs/translations/api-docs/data-grid/data-grid-pro-zh.json
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title container element"
},
"columnHeaderTitleContainerContent": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title excepted buttons"
},
"columnHeaders": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column headers"
Expand Down
4 changes: 4 additions & 0 deletions docs/translations/api-docs/data-grid/data-grid-pro.json
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title container element"
},
"columnHeaderTitleContainerContent": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title excepted buttons"
},
"columnHeaders": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column headers"
Expand Down
4 changes: 4 additions & 0 deletions docs/translations/api-docs/data-grid/data-grid-pt.json
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title container element"
},
"columnHeaderTitleContainerContent": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title excepted buttons"
},
"columnHeaders": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column headers"
Expand Down
4 changes: 4 additions & 0 deletions docs/translations/api-docs/data-grid/data-grid-zh.json
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title container element"
},
"columnHeaderTitleContainerContent": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title excepted buttons"
},
"columnHeaders": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column headers"
Expand Down
4 changes: 4 additions & 0 deletions docs/translations/api-docs/data-grid/data-grid.json
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title container element"
},
"columnHeaderTitleContainerContent": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column header's title excepted buttons"
},
"columnHeaders": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the column headers"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const useUtilityClasses = (ownerState: OwnerState) => {
],
draggableContainer: ['columnHeaderDraggableContainer'],
titleContainer: ['columnHeaderTitleContainer'],
titleContainerContent: ['columnHeaderTitleContainerContent'],
};

return composeClasses(slots, getDataGridUtilityClass, classes);
Expand Down Expand Up @@ -232,13 +233,15 @@ function GridColumnHeaderItem(props: GridColumnHeaderItemProps) {
{...draggableEventHandlers}
>
<div className={classes.titleContainer}>
{headerComponent || (
<GridColumnHeaderTitle
label={column.headerName ?? column.field}
description={column.description}
columnWidth={width}
/>
)}
<div className={classes.titleContainerContent}>
{headerComponent || (
<GridColumnHeaderTitle
label={column.headerName ?? column.field}
description={column.description}
columnWidth={width}
/>
)}
</div>

{columnTitleIconButtons}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ export const GridRootStyles = styled('div', {
whiteSpace: 'nowrap',
overflow: 'hidden',
},
[`& .${gridClasses.columnHeaderTitleContainerContent}`]: {
overflow: 'hidden',
display: 'flex',
alignItems: 'center',
},
[`& .${gridClasses.sortIcon}, & .${gridClasses.filterIcon}`]: {
fontSize: 'inherit',
},
Expand Down
5 changes: 5 additions & 0 deletions packages/grid/x-data-grid/src/constants/gridClasses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ export interface GridClasses {
* Styles applied to the column header's title container element.
*/
columnHeaderTitleContainer: string;
/**
* Styles applied to the column header's title excepted buttons.
*/
columnHeaderTitleContainerContent: string;
/**
* Styles applied to the column headers.
*/
Expand Down Expand Up @@ -394,6 +398,7 @@ export const gridClasses = generateUtilityClasses('MuiDataGrid', [
'columnHeaderDropZone',
'columnHeaderTitle',
'columnHeaderTitleContainer',
'columnHeaderTitleContainerContent',
'columnHeaders',
'columnHeadersInner',
'columnHeadersInner--scrollable',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { useGridApiEventHandler } from '../../utils/useGridApiEventHandler';
import { DataGridProcessedProps } from '../../../models/props/DataGridProps';
import { gridVisibleSortedRowEntriesSelector } from '../filter/gridFilterSelector';
import { useCurrentPageRows } from '../../utils/useCurrentPageRows';
import { GRID_CHECKBOX_SELECTION_COL_DEF } from '../../../colDef/gridCheckboxSelectionColDef';
import { gridClasses } from '../../../constants/gridClasses';

/**
* @requires useGridPage (state)
Expand Down Expand Up @@ -166,12 +168,21 @@ export const useGridKeyboardNavigation = (
);

const handleColumnHeaderKeyDown = React.useCallback<
GridEventListener<GridEvents.columnHeaderNavigationKeyDown>
GridEventListener<GridEvents.columnHeaderKeyDown>
>(
(params, event) => {
if (!params.field) {
const headerTitleNode = event.currentTarget.querySelector(
`.${gridClasses.columnHeaderTitleContainerContent}`,
);
const isFromInsideContent =
!!headerTitleNode && headerTitleNode.contains(event.target as Node | null);

if (isFromInsideContent && params.field !== GRID_CHECKBOX_SELECTION_COL_DEF.field) {
// When focus is on a nested input, keyboard events have no effect to avoid conflicts with native events.
// There is one exception for the checkBoxHeader
return;
}

const dimensions = apiRef.current.getRootDimensions();
if (!dimensions) {
return;
Expand Down
119 changes: 87 additions & 32 deletions packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { createRenderer, fireEvent, screen, createEvent } from '@mui/monorepo/test/utils';
import { createRenderer, fireEvent, screen } from '@mui/monorepo/test/utils';
import Portal from '@mui/material/Portal';
import { spy } from 'sinon';
import { expect } from 'chai';
Expand Down Expand Up @@ -365,42 +365,97 @@ describe('<DataGrid /> - Keyboard', () => {
fireEvent.keyDown(document.activeElement!, { key: 'PageDown' });
expect(getActiveCell()).to.equal(`5-1`);
});
});
/* eslint-enable material-ui/disallow-active-element-as-key-event-target */

it('should be able to type in an child input', () => {
const handleInputKeyDown = spy((event) => event.defaultPrevented);
it('should move focus when the focus is on a column header button', function test() {
if (isJSDOM) {
// This test is not relevant if we can't choose the actual height
this.skip();
}

const columns = [
{
field: 'name',
headerName: 'Name',
width: 200,
renderCell: () => (
<input type="text" data-testid="custom-input" onKeyDown={handleInputKeyDown} />
),
},
];
render(<NavigationTestCaseNoScrollX />);

const rows = [
{
id: 1,
name: 'John',
},
];
// get the sort button in column header 1
const columnMenuButton = getColumnHeaderCell(1).querySelector(
`button[title="Sort"]`,
) as HTMLElement;

render(
<div style={{ width: 300, height: 300 }}>
<DataGrid rows={rows} columns={columns} />
</div>,
);
const input = screen.getByTestId('custom-input');
fireClickEvent(input);
const keydownEvent = createEvent.keyDown(input, {
key: 'a',
// Simulate click on this button
fireEvent.mouseUp(columnMenuButton);
fireEvent.click(columnMenuButton);
columnMenuButton.focus();

fireEvent.keyDown(document.activeElement!, { key: 'ArrowDown' });
expect(getActiveCell()).to.equal(`0-1`);
});

/* eslint-enable material-ui/disallow-active-element-as-key-event-target */
it('should be able to type in an child input', () => {
const columns = [
{
field: 'name',
headerName: 'Name',
width: 200,
renderCell: () => <input type="text" data-testid="custom-input" />,
},
];

const rows = [
{
id: 1,
name: 'John',
},
];

render(
<div style={{ width: 300, height: 300 }}>
<DataGrid rows={rows} columns={columns} />
</div>,
);
const input = screen.getByTestId('custom-input');
fireEvent.mouseUp(input);
fireEvent.click(input);
input.focus();
m4theushw marked this conversation as resolved.
Show resolved Hide resolved

// This does not work with navigation keys.
// For now, the workaround for developers is to stop the propagation
// But adding input is discouraged, action column or edit mode are better options
expect(fireEvent.keyDown(input, { key: 'a' })).to.equal(true);
Copy link
Member

@m4theushw m4theushw Mar 2, 2022

Choose a reason for hiding this comment

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

I don't know if we should have this test. An input inside a cell in view mode is not encouraged because in this mode cells are supposed to be read-only. If you fire a ArrowDown event the assertion will fail correctly because any keydown event should move the focus to the next cell. However, in edit mode, the assertion (once changed to ArrowDown) is correct. I would move this test to the cell editing suite and render the input inside a cell in edit mode.

const isEditMode = cellParams.cellMode === GridCellModes.Edit;
if (isEditMode) {
return;
}

This can be done in another PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I will open another PR for this one 👍

});

it('should be able to use keyboard in a columnHeader child input', () => {
const columns = [
{
field: 'name',
headerName: 'Name',
width: 200,
renderHeader: () => <input type="text" data-testid="custom-input" />,
},
];

const rows = [
{
id: 1,
name: 'John',
},
];

render(
<div style={{ width: 300, height: 300 }}>
<DataGrid rows={rows} columns={columns} />
</div>,
);
const input = screen.getByTestId('custom-input');
fireEvent.mouseUp(input);
fireEvent.click(input);
input.focus();

// Verify that the event is not prevented during the bubbling.
// fireEvent.keyDown return false if it is the case
// For more info, see the related discussion: https://github.com/mui-org/material-ui-x/pull/3624/files#r787767632
alexfauquette marked this conversation as resolved.
Show resolved Hide resolved
expect(fireEvent.keyDown(input, { key: 'a' })).to.equal(true);
expect(fireEvent.keyDown(input, { key: ' ' })).to.equal(true);
expect(fireEvent.keyDown(input, { key: 'ArrowLeft' })).to.equal(true);
});
fireEvent(input, keydownEvent);
expect(handleInputKeyDown.returnValues).to.deep.equal([false]);
});

it('should ignore events coming from a portal inside the cell', () => {
Expand Down