From 4c3f29dbeb2d4dcaa19beb8e0fe21f28d22a1579 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Tue, 25 Jan 2022 16:22:12 +0100 Subject: [PATCH 01/23] fix the regression --- .../keyboard/useGridKeyboardNavigation.ts | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts index 069f2f42b1ba2..a19368174e174 100644 --- a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -9,6 +9,9 @@ import { useGridApiEventHandler } from '../../utils/useGridApiEventHandler'; import { DataGridProcessedProps } from '../../../models/props/DataGridProps'; import { gridVisibleSortedRowEntriesSelector } from '../filter/gridFilterSelector'; import { useCurrentPageRows } from '../../utils/useCurrentPageRows'; +import { isNavigationKey } from '../../../utils/keyboardUtils'; +import { isGridHeaderCellRoot } from '../../../utils/domUtils'; +import { GRID_CHECKBOX_SELECTION_COL_DEF } from '../../../models/colDef/gridCheckboxSelectionColDef'; /** * @requires useGridPage (state) @@ -158,10 +161,21 @@ export const useGridKeyboardNavigation = ( ); const handleColumnHeaderKeyDown = React.useCallback< - GridEventListener + GridEventListener >( (params, event) => { - event.preventDefault(); + if ( + !isGridHeaderCellRoot(event.target as HTMLElement) && + !(params.field === GRID_CHECKBOX_SELECTION_COL_DEF.field && isNavigationKey(event.key)) + ) { + // When focus is on a nested input, keyboard events have no effect to avoid conflicts with native events. + // There is on exception for the checkBoxHeader + return; + } + + if (isNavigationKey(event.key)) { + event.preventDefault(); + } if (!params.field) { return; } @@ -226,11 +240,6 @@ export const useGridKeyboardNavigation = ( } break; } - - case ' ': { - event.preventDefault(); // prevent Space event from scrolling - break; - } } }, [apiRef, colCount, currentPage, goToCell, goToHeader], From bdc8bcf6622fafa6dde06226a0659de5b5207afe Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Tue, 25 Jan 2022 16:22:41 +0100 Subject: [PATCH 02/23] add tests --- .../src/tests/keyboard.DataGrid.test.tsx | 48 ++++++++++++++++--- 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx index 80e647f00310e..aae385b20bdb1 100644 --- a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx +++ b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx @@ -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 { spy } from 'sinon'; import { expect } from 'chai'; import { @@ -325,6 +325,39 @@ describe(' - Keyboard', () => { fireEvent.keyDown(document.activeElement!, { key: 'PageDown' }); expect(getActiveCell()).to.equal(`5-1`); }); + + it('should be able to use keyboard in an child input', () => { + const columns = [ + { + field: 'name', + headerName: 'Name', + width: 200, + renderHeader: () => , + }, + ]; + + const rows = [ + { + id: 1, + name: 'John', + }, + ]; + + render( +
+ +
, + ); + const input = screen.getByTestId('custom-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 + 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); + }); }); /* eslint-enable material-ui/disallow-active-element-as-key-event-target */ @@ -356,11 +389,14 @@ describe(' - Keyboard', () => { ); const input = screen.getByTestId('custom-input'); fireClickEvent(input); - const keydownEvent = createEvent.keyDown(input, { - key: 'a', - }); - fireEvent(input, keydownEvent); - expect(handleInputKeyDown.returnValues).to.deep.equal([false]); + 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 + 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); }); it('should ignore key shortcuts if activeElement is not a cell', () => { From e3d553d023d81a41d5f8db2f46964f5774124cfc Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Thu, 27 Jan 2022 12:55:34 +0100 Subject: [PATCH 03/23] feedback --- .../hooks/features/keyboard/useGridKeyboardNavigation.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts index a19368174e174..af87bf1546f6b 100644 --- a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -166,7 +166,7 @@ export const useGridKeyboardNavigation = ( (params, event) => { if ( !isGridHeaderCellRoot(event.target as HTMLElement) && - !(params.field === GRID_CHECKBOX_SELECTION_COL_DEF.field && isNavigationKey(event.key)) + 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 on exception for the checkBoxHeader @@ -176,9 +176,7 @@ export const useGridKeyboardNavigation = ( if (isNavigationKey(event.key)) { event.preventDefault(); } - if (!params.field) { - return; - } + const dimensions = apiRef.current.getRootDimensions(); if (!dimensions) { return; From e1355e8d5db171dd173841af35ea7d1a201c272e Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 7 Feb 2022 15:21:31 +0100 Subject: [PATCH 04/23] test to prevent focus --- .../features/keyboard/useGridKeyboardNavigation.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts index 1eeb794b3033c..60042a87c89bc 100644 --- a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -170,10 +170,20 @@ export const useGridKeyboardNavigation = ( GridEventListener >( (params, event) => { + let isFromCustomHeader = false + if (params.colDef.renderHeader) { + const headerTitleNode = event.currentTarget.querySelector(".MuiDataGrid-columnHeaderTitle") + isFromCustomHeader = !!headerTitleNode && headerTitleNode.contains(event.target as Node | null) + } + if ( !isGridHeaderCellRoot(event.target as HTMLElement) && params.field !== GRID_CHECKBOX_SELECTION_COL_DEF.field ) { + if (!isFromCustomHeader) { + // prevent page scroll from keyboard when focus on our buttons + event.preventDefault() + } // When focus is on a nested input, keyboard events have no effect to avoid conflicts with native events. // There is on exception for the checkBoxHeader return; From 90be8566a0736bc591e2cc7d4f295dc406c5967c Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 7 Feb 2022 15:23:25 +0100 Subject: [PATCH 05/23] prettier --- .../hooks/features/keyboard/useGridKeyboardNavigation.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts index 60042a87c89bc..cf239a2ca3e52 100644 --- a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -170,10 +170,11 @@ export const useGridKeyboardNavigation = ( GridEventListener >( (params, event) => { - let isFromCustomHeader = false + let isFromCustomHeader = false; if (params.colDef.renderHeader) { - const headerTitleNode = event.currentTarget.querySelector(".MuiDataGrid-columnHeaderTitle") - isFromCustomHeader = !!headerTitleNode && headerTitleNode.contains(event.target as Node | null) + const headerTitleNode = event.currentTarget.querySelector('.MuiDataGrid-columnHeaderTitle'); + isFromCustomHeader = + !!headerTitleNode && headerTitleNode.contains(event.target as Node | null); } if ( @@ -182,7 +183,7 @@ export const useGridKeyboardNavigation = ( ) { if (!isFromCustomHeader) { // prevent page scroll from keyboard when focus on our buttons - event.preventDefault() + event.preventDefault(); } // When focus is on a nested input, keyboard events have no effect to avoid conflicts with native events. // There is on exception for the checkBoxHeader From bf009e064ecd606b3fc6b760be0f3cd13584127b Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Thu, 24 Feb 2022 17:00:04 +0100 Subject: [PATCH 06/23] fix className --- .../hooks/features/keyboard/useGridKeyboardNavigation.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts index 4123517607cac..0a5544043a83f 100644 --- a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -11,6 +11,7 @@ import { gridVisibleSortedRowEntriesSelector } from '../filter/gridFilterSelecto import { useCurrentPageRows } from '../../utils/useCurrentPageRows'; import { isGridHeaderCellRoot } from '../../../utils/domUtils'; import { GRID_CHECKBOX_SELECTION_COL_DEF } from '../../../models/colDef/gridCheckboxSelectionColDef'; +import { gridClasses } from '../../../gridClasses'; /** * @requires useGridPage (state) @@ -174,7 +175,7 @@ export const useGridKeyboardNavigation = ( (params, event) => { let isFromCustomHeader = false; if (params.colDef.renderHeader) { - const headerTitleNode = event.currentTarget.querySelector('.MuiDataGrid-columnHeaderTitle'); + const headerTitleNode = event.currentTarget.querySelector(`.${gridClasses.columnHeaderTitleContainer}`); isFromCustomHeader = !!headerTitleNode && headerTitleNode.contains(event.target as Node | null); } From 01e4a78a339cdbac70cfecd61960ac6153a001d9 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Thu, 24 Feb 2022 17:00:13 +0100 Subject: [PATCH 07/23] remove one test --- .../src/tests/keyboard.DataGrid.test.tsx | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx index fbcc489579353..a2f59bfee6f7c 100644 --- a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx +++ b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx @@ -401,43 +401,6 @@ describe(' - Keyboard', () => { }); /* 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); - - const columns = [ - { - field: 'name', - headerName: 'Name', - width: 200, - renderCell: () => ( - - ), - }, - ]; - - const rows = [ - { - id: 1, - name: 'John', - }, - ]; - - render( -
- -
, - ); - const input = screen.getByTestId('custom-input'); - fireClickEvent(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 - 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); - }); it('should ignore events coming from a portal inside the cell', () => { const handleCellKeyDown = spy(); From 6c4a2bc79e3e14639d62bcbaba8cd92b29e57e97 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Thu, 24 Feb 2022 17:37:25 +0100 Subject: [PATCH 08/23] prettier --- .../hooks/features/keyboard/useGridKeyboardNavigation.ts | 4 +++- .../grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts index 0a5544043a83f..3e83a2a0ddbf5 100644 --- a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -175,7 +175,9 @@ export const useGridKeyboardNavigation = ( (params, event) => { let isFromCustomHeader = false; if (params.colDef.renderHeader) { - const headerTitleNode = event.currentTarget.querySelector(`.${gridClasses.columnHeaderTitleContainer}`); + const headerTitleNode = event.currentTarget.querySelector( + `.${gridClasses.columnHeaderTitleContainer}`, + ); isFromCustomHeader = !!headerTitleNode && headerTitleNode.contains(event.target as Node | null); } diff --git a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx index a2f59bfee6f7c..76ee8688c25f7 100644 --- a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx +++ b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx @@ -401,7 +401,6 @@ describe(' - Keyboard', () => { }); /* eslint-enable material-ui/disallow-active-element-as-key-event-target */ - it('should ignore events coming from a portal inside the cell', () => { const handleCellKeyDown = spy(); render( From 5bb32e1009072d69ae5ee997e3722c79c50a5bc3 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Thu, 24 Feb 2022 17:57:21 +0100 Subject: [PATCH 09/23] keep innput test with exmplaination --- .../src/tests/keyboard.DataGrid.test.tsx | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx index 76ee8688c25f7..1761676699d2b 100644 --- a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx +++ b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx @@ -365,8 +365,42 @@ describe(' - 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 use keyboard in an child input', () => { + it('should be able to type in an child input', () => { + const columns = [ + { + field: 'name', + headerName: 'Name', + width: 200, + renderCell: () => ( + + ), + }, + ]; + + const rows = [ + { + id: 1, + name: 'John', + }, + ]; + + render( +
+ +
, + ); + const input = screen.getByTestId('custom-input'); + input.focus(); + + // 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); + }); + + it('should be able to use keyboard in a columnHeader child input', () => { const columns = [ { field: 'name', @@ -399,7 +433,6 @@ describe(' - Keyboard', () => { expect(fireEvent.keyDown(input, { key: 'ArrowLeft' })).to.equal(true); }); }); - /* eslint-enable material-ui/disallow-active-element-as-key-event-target */ it('should ignore events coming from a portal inside the cell', () => { const handleCellKeyDown = spy(); From 70137b8daf8ebe4663882ef754e819e92f77d3e4 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Thu, 24 Feb 2022 18:33:45 +0100 Subject: [PATCH 10/23] prettier --- .../grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx index 1761676699d2b..969fb2841cc72 100644 --- a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx +++ b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx @@ -373,9 +373,7 @@ describe(' - Keyboard', () => { field: 'name', headerName: 'Name', width: 200, - renderCell: () => ( - - ), + renderCell: () => , }, ]; From a9277373d4b0fae0886dd5e113270656770d5c40 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Fri, 25 Feb 2022 13:48:17 +0100 Subject: [PATCH 11/23] remove buttons from the container --- .../columnHeaders/GridColumnHeaderItem.tsx | 17 ++++++++++------- .../x-data-grid/src/internals/gridClasses.ts | 5 +++++ .../keyboard/useGridKeyboardNavigation.ts | 2 +- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/grid/x-data-grid/src/internals/components/columnHeaders/GridColumnHeaderItem.tsx b/packages/grid/x-data-grid/src/internals/components/columnHeaders/GridColumnHeaderItem.tsx index fd6cc0fa8553b..2b041b82f7530 100644 --- a/packages/grid/x-data-grid/src/internals/components/columnHeaders/GridColumnHeaderItem.tsx +++ b/packages/grid/x-data-grid/src/internals/components/columnHeaders/GridColumnHeaderItem.tsx @@ -64,6 +64,7 @@ const useUtilityClasses = (ownerState: OwnerState) => { ], draggableContainer: ['columnHeaderDraggableContainer'], titleContainer: ['columnHeaderTitleContainer'], + titleContentContainer: ['columnHeaderTitleContentContainer'], }; return composeClasses(slots, getDataGridUtilityClass, classes); @@ -232,13 +233,15 @@ function GridColumnHeaderItem(props: GridColumnHeaderItemProps) { {...draggableEventHandlers} >
- {headerComponent || ( - - )} +
+ {headerComponent || ( + + )} +
{columnTitleIconButtons}
diff --git a/packages/grid/x-data-grid/src/internals/gridClasses.ts b/packages/grid/x-data-grid/src/internals/gridClasses.ts index d26ca99392448..afc2a083ed937 100644 --- a/packages/grid/x-data-grid/src/internals/gridClasses.ts +++ b/packages/grid/x-data-grid/src/internals/gridClasses.ts @@ -101,6 +101,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. + */ + columnHeaderTitleContentContainer: string; /** * Styles applied to the column headers. */ @@ -389,6 +393,7 @@ export const gridClasses = generateUtilityClasses('MuiDataGrid', [ 'columnHeaderDropZone', 'columnHeaderTitle', 'columnHeaderTitleContainer', + 'columnHeaderTitleContentContainer', 'columnHeaders', 'columnHeadersInner', 'columnHeadersInner--scrollable', diff --git a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts index 3e83a2a0ddbf5..769524ce97ab8 100644 --- a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -176,7 +176,7 @@ export const useGridKeyboardNavigation = ( let isFromCustomHeader = false; if (params.colDef.renderHeader) { const headerTitleNode = event.currentTarget.querySelector( - `.${gridClasses.columnHeaderTitleContainer}`, + `.${gridClasses.columnHeaderTitleContentContainer}`, ); isFromCustomHeader = !!headerTitleNode && headerTitleNode.contains(event.target as Node | null); From cc67c2da205abcef7c977eeabf200e408eaa7a2b Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 28 Feb 2022 09:36:22 +0100 Subject: [PATCH 12/23] feedbacks --- .../columnHeaders/GridColumnHeaderItem.tsx | 4 ++-- .../x-data-grid/src/internals/gridClasses.ts | 4 ++-- .../keyboard/useGridKeyboardNavigation.ts | 16 +++++++--------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/grid/x-data-grid/src/internals/components/columnHeaders/GridColumnHeaderItem.tsx b/packages/grid/x-data-grid/src/internals/components/columnHeaders/GridColumnHeaderItem.tsx index 2b041b82f7530..c944427c70565 100644 --- a/packages/grid/x-data-grid/src/internals/components/columnHeaders/GridColumnHeaderItem.tsx +++ b/packages/grid/x-data-grid/src/internals/components/columnHeaders/GridColumnHeaderItem.tsx @@ -64,7 +64,7 @@ const useUtilityClasses = (ownerState: OwnerState) => { ], draggableContainer: ['columnHeaderDraggableContainer'], titleContainer: ['columnHeaderTitleContainer'], - titleContentContainer: ['columnHeaderTitleContentContainer'], + titleContainerContent: ['columnHeaderTitleContainerContent'], }; return composeClasses(slots, getDataGridUtilityClass, classes); @@ -233,7 +233,7 @@ function GridColumnHeaderItem(props: GridColumnHeaderItemProps) { {...draggableEventHandlers} >
-
+
{headerComponent || ( ( (params, event) => { let isFromCustomHeader = false; - if (params.colDef.renderHeader) { - const headerTitleNode = event.currentTarget.querySelector( - `.${gridClasses.columnHeaderTitleContentContainer}`, - ); - isFromCustomHeader = - !!headerTitleNode && headerTitleNode.contains(event.target as Node | null); - } + const headerTitleNode = event.currentTarget.querySelector( + `.${gridClasses.columnHeaderTitleContainerContent}`, + ); + isFromCustomHeader = + !!headerTitleNode && headerTitleNode.contains(event.target as Node | null); if ( - !isGridHeaderCellRoot(event.target as HTMLElement) && + event.target != null && + (event.target as HTMLElement).classList.contains(gridClasses.columnHeader) && params.field !== GRID_CHECKBOX_SELECTION_COL_DEF.field ) { if (!isFromCustomHeader) { From 69c41ec267cda8c74fac18b85ba13b74b2bc2a65 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 28 Feb 2022 09:41:10 +0100 Subject: [PATCH 13/23] docs:api --- docs/pages/api-docs/data-grid/data-grid-pro.json | 1 + docs/pages/api-docs/data-grid/data-grid.json | 1 + docs/pages/x/api/data-grid/data-grid-pro.json | 1 + docs/pages/x/api/data-grid/data-grid.json | 1 + docs/translations/api-docs/data-grid/data-grid-pro-pt.json | 4 ++++ docs/translations/api-docs/data-grid/data-grid-pro-zh.json | 4 ++++ docs/translations/api-docs/data-grid/data-grid-pro.json | 4 ++++ docs/translations/api-docs/data-grid/data-grid-pt.json | 4 ++++ docs/translations/api-docs/data-grid/data-grid-zh.json | 4 ++++ docs/translations/api-docs/data-grid/data-grid.json | 4 ++++ 10 files changed, 28 insertions(+) diff --git a/docs/pages/api-docs/data-grid/data-grid-pro.json b/docs/pages/api-docs/data-grid/data-grid-pro.json index 06d17f5401b39..0d4f3c3e3a2ac 100644 --- a/docs/pages/api-docs/data-grid/data-grid-pro.json +++ b/docs/pages/api-docs/data-grid/data-grid-pro.json @@ -310,6 +310,7 @@ "columnHeaderDropZone", "columnHeaderTitle", "columnHeaderTitleContainer", + "columnHeaderTitleContainerContent", "columnHeaders", "columnHeadersInner", "columnHeadersInner--scrollable", diff --git a/docs/pages/api-docs/data-grid/data-grid.json b/docs/pages/api-docs/data-grid/data-grid.json index e7354ca833e8d..67cb54014d64f 100644 --- a/docs/pages/api-docs/data-grid/data-grid.json +++ b/docs/pages/api-docs/data-grid/data-grid.json @@ -257,6 +257,7 @@ "columnHeaderDropZone", "columnHeaderTitle", "columnHeaderTitleContainer", + "columnHeaderTitleContainerContent", "columnHeaders", "columnHeadersInner", "columnHeadersInner--scrollable", diff --git a/docs/pages/x/api/data-grid/data-grid-pro.json b/docs/pages/x/api/data-grid/data-grid-pro.json index faf911b4c7b54..6124450f7177b 100644 --- a/docs/pages/x/api/data-grid/data-grid-pro.json +++ b/docs/pages/x/api/data-grid/data-grid-pro.json @@ -310,6 +310,7 @@ "columnHeaderDropZone", "columnHeaderTitle", "columnHeaderTitleContainer", + "columnHeaderTitleContainerContent", "columnHeaders", "columnHeadersInner", "columnHeadersInner--scrollable", diff --git a/docs/pages/x/api/data-grid/data-grid.json b/docs/pages/x/api/data-grid/data-grid.json index ddd8698fd063a..91cf7d1084463 100644 --- a/docs/pages/x/api/data-grid/data-grid.json +++ b/docs/pages/x/api/data-grid/data-grid.json @@ -257,6 +257,7 @@ "columnHeaderDropZone", "columnHeaderTitle", "columnHeaderTitleContainer", + "columnHeaderTitleContainerContent", "columnHeaders", "columnHeadersInner", "columnHeadersInner--scrollable", diff --git a/docs/translations/api-docs/data-grid/data-grid-pro-pt.json b/docs/translations/api-docs/data-grid/data-grid-pro-pt.json index 819d717438351..cc72fdfb42723 100644 --- a/docs/translations/api-docs/data-grid/data-grid-pro-pt.json +++ b/docs/translations/api-docs/data-grid/data-grid-pro-pt.json @@ -236,6 +236,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" diff --git a/docs/translations/api-docs/data-grid/data-grid-pro-zh.json b/docs/translations/api-docs/data-grid/data-grid-pro-zh.json index 819d717438351..cc72fdfb42723 100644 --- a/docs/translations/api-docs/data-grid/data-grid-pro-zh.json +++ b/docs/translations/api-docs/data-grid/data-grid-pro-zh.json @@ -236,6 +236,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" diff --git a/docs/translations/api-docs/data-grid/data-grid-pro.json b/docs/translations/api-docs/data-grid/data-grid-pro.json index 819d717438351..cc72fdfb42723 100644 --- a/docs/translations/api-docs/data-grid/data-grid-pro.json +++ b/docs/translations/api-docs/data-grid/data-grid-pro.json @@ -236,6 +236,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" diff --git a/docs/translations/api-docs/data-grid/data-grid-pt.json b/docs/translations/api-docs/data-grid/data-grid-pt.json index cb1c3661f875e..ea36cf6347d8d 100644 --- a/docs/translations/api-docs/data-grid/data-grid-pt.json +++ b/docs/translations/api-docs/data-grid/data-grid-pt.json @@ -205,6 +205,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" diff --git a/docs/translations/api-docs/data-grid/data-grid-zh.json b/docs/translations/api-docs/data-grid/data-grid-zh.json index cb1c3661f875e..ea36cf6347d8d 100644 --- a/docs/translations/api-docs/data-grid/data-grid-zh.json +++ b/docs/translations/api-docs/data-grid/data-grid-zh.json @@ -205,6 +205,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" diff --git a/docs/translations/api-docs/data-grid/data-grid.json b/docs/translations/api-docs/data-grid/data-grid.json index cb1c3661f875e..ea36cf6347d8d 100644 --- a/docs/translations/api-docs/data-grid/data-grid.json +++ b/docs/translations/api-docs/data-grid/data-grid.json @@ -205,6 +205,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" From dda1b40dfb5a2461d88d9241951db6308130b764 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 28 Feb 2022 11:07:05 +0100 Subject: [PATCH 14/23] add the 'not' operator --- .../features/keyboard/useGridKeyboardNavigation.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts index ad030ee6d2e41..207c42c09e0b9 100644 --- a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -171,24 +171,22 @@ export const useGridKeyboardNavigation = ( GridEventListener >( (params, event) => { - let isFromCustomHeader = false; const headerTitleNode = event.currentTarget.querySelector( `.${gridClasses.columnHeaderTitleContainerContent}`, ); - isFromCustomHeader = + const isFromCustomHeader = !!headerTitleNode && headerTitleNode.contains(event.target as Node | null); - - if ( + const isGridHeaderCellRoot = event.target != null && - (event.target as HTMLElement).classList.contains(gridClasses.columnHeader) && - params.field !== GRID_CHECKBOX_SELECTION_COL_DEF.field - ) { + (event.target as HTMLElement).classList.contains(gridClasses.columnHeader); + + if (!isGridHeaderCellRoot && params.field !== GRID_CHECKBOX_SELECTION_COL_DEF.field) { if (!isFromCustomHeader) { // prevent page scroll from keyboard when focus on our buttons event.preventDefault(); } // When focus is on a nested input, keyboard events have no effect to avoid conflicts with native events. - // There is on exception for the checkBoxHeader + // There is one exception for the checkBoxHeader return; } From 83458241938cf55287d56015e463f3be255d0a7c Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 28 Feb 2022 11:34:58 +0100 Subject: [PATCH 15/23] fix style --- .../src/internals/components/containers/GridRootStyles.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/grid/x-data-grid/src/internals/components/containers/GridRootStyles.ts b/packages/grid/x-data-grid/src/internals/components/containers/GridRootStyles.ts index 2dc7d94ceb9b8..ee4e1604f45d2 100644 --- a/packages/grid/x-data-grid/src/internals/components/containers/GridRootStyles.ts +++ b/packages/grid/x-data-grid/src/internals/components/containers/GridRootStyles.ts @@ -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', }, From 492f7cef1e973a8aed7a75255fffee6fe0c69286 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 28 Feb 2022 16:35:24 +0100 Subject: [PATCH 16/23] renaming --- .../hooks/features/keyboard/useGridKeyboardNavigation.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts index 207c42c09e0b9..9a8f9bbb10ec0 100644 --- a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -174,14 +174,14 @@ export const useGridKeyboardNavigation = ( const headerTitleNode = event.currentTarget.querySelector( `.${gridClasses.columnHeaderTitleContainerContent}`, ); - const isFromCustomHeader = + const isFromInsideContent = !!headerTitleNode && headerTitleNode.contains(event.target as Node | null); const isGridHeaderCellRoot = event.target != null && (event.target as HTMLElement).classList.contains(gridClasses.columnHeader); if (!isGridHeaderCellRoot && params.field !== GRID_CHECKBOX_SELECTION_COL_DEF.field) { - if (!isFromCustomHeader) { + if (!isFromInsideContent) { // prevent page scroll from keyboard when focus on our buttons event.preventDefault(); } From 701d17c6ed47b1cf1ae1610eaf9e9d78ea735518 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 28 Feb 2022 16:44:13 +0100 Subject: [PATCH 17/23] fix tests --- .../grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx index 969fb2841cc72..86526e39e272c 100644 --- a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx +++ b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx @@ -390,6 +390,8 @@ describe(' - Keyboard', () => {
, ); const input = screen.getByTestId('custom-input'); + fireEvent.mouseUp(input); + fireEvent.click(input); input.focus(); // This does not work with navigation keys. @@ -421,6 +423,8 @@ describe(' - Keyboard', () => {
, ); const input = screen.getByTestId('custom-input'); + fireEvent.mouseUp(input); + fireEvent.click(input); input.focus(); // Verify that the event is not prevented during the bubbling. From 6ba52c10e133ad53236c2d0e30fe1bbb96651ad2 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 28 Feb 2022 16:45:48 +0100 Subject: [PATCH 18/23] move eslint line --- packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx index 86526e39e272c..3468ecac11664 100644 --- a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx +++ b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx @@ -365,8 +365,8 @@ describe(' - Keyboard', () => { fireEvent.keyDown(document.activeElement!, { key: 'PageDown' }); expect(getActiveCell()).to.equal(`5-1`); }); - /* eslint-enable material-ui/disallow-active-element-as-key-event-target */ + /* eslint-enable material-ui/disallow-active-element-as-key-event-target */ it('should be able to type in an child input', () => { const columns = [ { From 39094de27cdce37256d881691c4eeefa991aeff4 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 28 Feb 2022 16:56:42 +0100 Subject: [PATCH 19/23] remove isGridHeaderCellRoot --- .../hooks/features/keyboard/useGridKeyboardNavigation.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts index 9a8f9bbb10ec0..cd8ee45829aa5 100644 --- a/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/x-data-grid/src/internals/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -176,15 +176,8 @@ export const useGridKeyboardNavigation = ( ); const isFromInsideContent = !!headerTitleNode && headerTitleNode.contains(event.target as Node | null); - const isGridHeaderCellRoot = - event.target != null && - (event.target as HTMLElement).classList.contains(gridClasses.columnHeader); - if (!isGridHeaderCellRoot && params.field !== GRID_CHECKBOX_SELECTION_COL_DEF.field) { - if (!isFromInsideContent) { - // prevent page scroll from keyboard when focus on our buttons - event.preventDefault(); - } + 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; From 95c41b886ebf1fae9213d8f288bc0c630c6c8a24 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Mon, 28 Feb 2022 18:29:17 +0100 Subject: [PATCH 20/23] add test --- .../src/tests/keyboard.DataGrid.test.tsx | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx index 3468ecac11664..308ed3be42408 100644 --- a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx +++ b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx @@ -366,6 +366,28 @@ describe(' - Keyboard', () => { expect(getActiveCell()).to.equal(`5-1`); }); + 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(); + } + + render(); + + // get the sort button in column header 1 + const columnMenuButton = getColumnHeaderCell(1).querySelector( + `button[title="Sort"]`, + ) as HTMLElement; + + // 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 = [ From e6bde22628f4081df82f10d6a26aa5f60b53acba Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Tue, 1 Mar 2022 11:17:43 +0100 Subject: [PATCH 21/23] fix import --- .../src/hooks/features/keyboard/useGridKeyboardNavigation.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grid/x-data-grid/src/hooks/features/keyboard/useGridKeyboardNavigation.ts b/packages/grid/x-data-grid/src/hooks/features/keyboard/useGridKeyboardNavigation.ts index cd8ee45829aa5..f619849ebaf8c 100644 --- a/packages/grid/x-data-grid/src/hooks/features/keyboard/useGridKeyboardNavigation.ts +++ b/packages/grid/x-data-grid/src/hooks/features/keyboard/useGridKeyboardNavigation.ts @@ -8,8 +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 '../../../models/colDef/gridCheckboxSelectionColDef'; -import { gridClasses } from '../../../gridClasses'; +import { GRID_CHECKBOX_SELECTION_COL_DEF } from '../../../colDef/gridCheckboxSelectionColDef'; +import { gridClasses } from '../../../constants/gridClasses'; /** * @requires useGridPage (state) From 27c80e5b2050b8078b26b1e5cea68891096a3f21 Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com> Date: Wed, 2 Mar 2022 14:14:42 +0100 Subject: [PATCH 22/23] Update packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx Co-authored-by: Matheus Wichman --- packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx index 308ed3be42408..844bb87d079b4 100644 --- a/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx +++ b/packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx @@ -451,7 +451,7 @@ describe(' - Keyboard', () => { // 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 + // For more info, see the related discussion: https://github.com/mui/mui-x/pull/3624#discussion_r787767632 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); From 4ccbbc75e580e74be1282d14b128192f643bbe6b Mon Sep 17 00:00:00 2001 From: Alexandre Fauquette Date: Wed, 2 Mar 2022 14:33:37 +0100 Subject: [PATCH 23/23] Trigger CI