[DataGrid] Allows keyboard navigation in group header #5947

alexfauquette · 2022-08-29T13:18:51Z

Preview: https://deploy-preview-5947--material-ui-x.netlify.app/x/react-data-grid/column-groups/

The structure column group structure was shared between columns definitions and the columnGroups state.

In this PR I move the column.groupPath which contains the list of the group parent into a method unstable_getColumnGroupPath(field) ANd the all group header structure is saved in the state. The idea being to simplify query such as "What is the group on the left of the focused one" and later on for groups drag and drop: "What are the children of their dragged group and the children of the drop group"

mui-bot · 2022-08-29T13:24:36Z

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	500.6	788.2	721.8	661.56	120.799
Sort 100k rows	ms	550.5	1,172.4	550.5	888.56	216.606
Select 100k rows	ms	156.5	272.9	229.7	228.64	42.071
Deselect 100k rows	ms	146.1	221.8	181.7	182.3	28.532

Generated by 🚫 dangerJS against 72f6024

github-actions · 2022-09-01T10:04:40Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

…roup

github-actions · 2022-09-12T09:22:52Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2022-09-26T06:00:08Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

DanailH

Looks great! The only thing I would consider adding is a check if a column header doesn't have content (it's empty) not to be able to navigate in it. Idk how difficult lit would be tho.

DanailH · 2022-09-28T12:48:20Z

packages/grid/x-data-grid/src/hooks/features/columnGrouping/useGridColumnGrouping.ts

+    });
+  }, [apiRef, props.columnGroupingModel]);
+
+  useGridApiEventHandler(apiRef, 'columnOrderChange', handleColumnReorderChange);


Shouldn't that be handled in the useGridColumnReorder hook?

I don't think so, because the useGridColumnReorder trigger the event, then other hooks listen to it, like the useGridcolumnSpanning, or useGridcolumnPinning which already listen tot this event to recompute their state

Did you had a specific idea in mind like performance impact for putting the logic in the useGridColumnReorder?

DanailH · 2022-09-28T12:52:02Z

packages/grid/x-data-grid/src/hooks/features/columnGrouping/useGridColumnGrouping.ts

@@ -103,15 +48,6 @@ const createGroupLookup = (columnGroupingModel: GridColumnNode[]): GridColumnGro

  return { ...groupLookup };
 };
-export const columnGroupsStateInitializer: GridStateInitializer<


I may be lost but does this mean that the groups are no longer persisted in the state? Shouldn't we initialize an empty columnGrouping state?

It's unclear for me when an initializer is necessary. I thaught it was used to initialise the state before pipe processors are called.

Since column grouping is not used by any processors, I removed the initialization. But if it's needed for other purpose, I can bring initializer back

The main purpose of the state initializers was to allow the state to be initialized synchronously before any feature hook. Later, we had to also initialize them before any pre-processor. You can delete the state initializer and have the feature working normally. The React.useEffect in this hook will take care of initializing the state, but it will happen asynchronously now. This will only be a problem if another hook depends on the state of the column grouping.

Ok, since the state is reused by columnReorder and columnHeader I put back the initializer

packages/grid/x-data-grid/src/hooks/features/density/densitySelector.ts

alexfauquette · 2022-09-28T14:49:51Z

@DanailH It was my first it do not navigate to empty cell but then comes the question about where does the focus go. And their is no obvious answer to that, so I looked at other libraries, and they just let navigate in empty column group. For example AG grid column group

m4theushw

Great work! I didn't thing any bug related to keyboard navigation, although I noticed a small problem when the user hides a column in https://deploy-preview-5947--material-ui-x.netlify.app/x/react-data-grid/column-groups/.

msedge_dJb50P4sDK.mp4

I can't reproduce it in https://docs-next--material-ui-x.netlify.app/x/react-data-grid/column-groups/ so it propabily was added in this PR.

packages/grid/x-data-grid/src/tests/keyboard.DataGrid.test.tsx

packages/grid/x-data-grid/src/models/api/gridFocusApi.ts

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

alexfauquette · 2022-10-05T12:06:50Z

@m4theushw the bug is fixed by this commit I was rendering all the column groups between the first and last columns to render. Now I remove groups that have all their columns hidden

github-actions · 2022-10-10T08:49:26Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2022-10-15T14:33:42Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

alexfauquette added 2 commits August 29, 2022 15:16

move params to dedicated file

857385d

basic keyboard interaction

a885c69

alexfauquette added the feature: Column groups Related to the data grid Column groups feature label Aug 29, 2022

alexfauquette self-assigned this Aug 29, 2022

alexfauquette added 5 commits August 29, 2022 15:29

scripts

64049bb

create dedicated file for utils

a654372

add initializer in premium

8ba0117

save group header structure in the state

c88d352

update header structure on column reorder

5391022

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 1, 2022

alexfauquette added 8 commits September 5, 2022 14:10

move groupPath from column lookup to grouping model

1ca84d2

scripts

41003d5

fix tests

65fa822

add tests

28b1df7

avoid fetching out of range column

405d7ed

fix tests

b7a8b10

Merge remote-tracking branch 'upstream/master' into keyboard-column-g…

aeb4d8e

…roup

fix

73c0134

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 7, 2022

prettier

b98974d

alexfauquette changed the base branch from master to next September 9, 2022 12:15

alexfauquette marked this pull request as ready for review September 9, 2022 12:15

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 12, 2022

Merge remote-tracking branch 'upstream/next' into keyboard-column-group

749aea7

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 15, 2022

alexfauquette added 2 commits September 20, 2022 16:31

Merge remote-tracking branch 'upstream/next' into keyboard-column-group

c3f157a

change impoprted method

622f0ba

alexfauquette requested review from DanailH and m4theushw September 20, 2022 15:48

remove commented sections

26f9e5c

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 26, 2022

DanailH reviewed Sep 28, 2022

View reviewed changes

m4theushw reviewed Oct 3, 2022

View reviewed changes

alexfauquette added 4 commits October 5, 2022 11:36

Merge remote-tracking branch 'upstream/next' into keyboard-column-group

f1f0409

small feedbacks

9f149fa

fix empty group bug

369f734

scripts

1c017e6

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 5, 2022

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 10, 2022

alexfauquette requested review from m4theushw and DanailH October 11, 2022 12:42

Merge remote-tracking branch 'upstream/next' into keyboard-column-group

dac5dbf

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 11, 2022

alexfauquette added 2 commits October 14, 2022 19:40

add initializer

1dc6cb0

prettier

0e6cbf8

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 15, 2022

Merge remote-tracking branch 'upstream/next' into keyboard-column-group

72f6024

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 17, 2022

alexfauquette merged commit 955179e into mui:next Oct 18, 2022

alexfauquette deleted the keyboard-column-group branch October 18, 2022 15:16

alexfauquette mentioned this pull request Dec 8, 2022

[data grid] Allows keyboard navigation in column groups #5879

Closed

cherniavskii mentioned this pull request Apr 14, 2023

[data grid pro] Updating columns and columnGroupingModel at once throws error #8602

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Allows keyboard navigation in group header #5947

[DataGrid] Allows keyboard navigation in group header #5947

alexfauquette commented Aug 29, 2022 •

edited by m4theushw

Loading

mui-bot commented Aug 29, 2022 •

edited

Loading

github-actions bot commented Sep 1, 2022

github-actions bot commented Sep 12, 2022

github-actions bot commented Sep 26, 2022

DanailH left a comment •

edited

Loading

DanailH Sep 28, 2022

alexfauquette Oct 5, 2022

DanailH Sep 28, 2022

alexfauquette Oct 5, 2022

m4theushw Oct 13, 2022

alexfauquette Oct 14, 2022

alexfauquette commented Sep 28, 2022

m4theushw left a comment

alexfauquette commented Oct 5, 2022

github-actions bot commented Oct 10, 2022

github-actions bot commented Oct 15, 2022

[DataGrid] Allows keyboard navigation in group header #5947

[DataGrid] Allows keyboard navigation in group header #5947

Conversation

alexfauquette commented Aug 29, 2022 • edited by m4theushw Loading

mui-bot commented Aug 29, 2022 • edited Loading

github-actions bot commented Sep 1, 2022

github-actions bot commented Sep 12, 2022

github-actions bot commented Sep 26, 2022

DanailH left a comment • edited Loading

Choose a reason for hiding this comment

DanailH Sep 28, 2022

Choose a reason for hiding this comment

alexfauquette Oct 5, 2022

Choose a reason for hiding this comment

DanailH Sep 28, 2022

Choose a reason for hiding this comment

alexfauquette Oct 5, 2022

Choose a reason for hiding this comment

m4theushw Oct 13, 2022

Choose a reason for hiding this comment

alexfauquette Oct 14, 2022

Choose a reason for hiding this comment

alexfauquette commented Sep 28, 2022

m4theushw left a comment

Choose a reason for hiding this comment

alexfauquette commented Oct 5, 2022

github-actions bot commented Oct 10, 2022

github-actions bot commented Oct 15, 2022

alexfauquette commented Aug 29, 2022 •

edited by m4theushw

Loading

mui-bot commented Aug 29, 2022 •

edited

Loading

DanailH left a comment •

edited

Loading