[DataGrid] Row spanning #14124

MBilalShafi · 2024-08-06T11:15:33Z

🎉 Feature Completed 🎉

Resolves #207

Based on https://www.notion.so/mui-org/xGrid-Row-Spanning-b178fd43907146799eb7a57d9330b68b?pvs=4#160e28c1b4b247d7befe0cbf44acd475

Documentation and Preview: https://deploy-preview-14124--material-ui-x.netlify.app/x/react-data-grid/row-spanning/

Todos

Follow-up Todos

Add test coverage for more features (e.g. row reordering)
Make the Grid remember the last horizontal rowIndex. [DataGrid] Row spanning #14124 (comment)
Validate how the feature works with:
- Cell selection
- Row selection
[TBD] Take into account the column virtualization - only compute row spanning state for rendered columns
[TBD] Pin the value exceeding the currently rendered rows on top

Changelog

💫 Support Row spanning on the Data Grid which automatically merges the consecutive cells in a column

mui-bot · 2024-08-06T11:19:38Z

Deploy preview: https://deploy-preview-14124--material-ui-x.netlify.app/

Updated pages:

Generated by 🚫 dangerJS against 7031634

…en there are repeated values

MBilalShafi · 2024-08-07T14:44:38Z

docs/data/data-grid/row-spanning/RowSpanning.js

I am wondering what the behavior should be with editing.

Should editing a cell that spans multiple rows update all the repeating values? It makes sense to me.
Although is not how colSpan behaves (example: https://stackblitz.com/edit/react-ucq5za?file=Demo.tsx).

What do you think @mui/xgrid ?

I don't understand user expectations very well for this feature, but I was wondering if there is a case where users would want to change the value of individual cells that are part of a row span?

Should editing a cell that spans multiple rows update all the repeating values?

If not, this suggestion makes sense to me.

I see use cases for both

If you have structured data and you use rowspan to show value of the parent once for all rows, you would probably want to update all of them at once

If we connected cells in a column for an age, then you probably don't want to update all of them at once

If we connected cells in a column for an age, then you probably don't want to update all of them at once

I believe such rows shouldn't be spanned in the first place. I introduced a prop rowSpanValueGetter to allow excluding unwanted values from being spanned.

For example with age, as you mentioned, it could be the same yet belong to different person. So if we avoid spanning them, we could go with the assumption: editing a cell that spans multiple rows would update all the repeating values

Consider this example: https://deploy-preview-14124--material-ui-x.netlify.app/x/react-data-grid/row-spanning/#customizing-row-spanned-cells

George Floyd and Cynthia Duke are both 25 years old but rowSpanValueGetter is used to correctly span the values.

Does it make sense?

for our pretty automated feature, this gives enough initial control to cover both cases

After thinking more about this, I'm inclined to not go with this approach for now, mainly because it is a breaking behavioral change.

Currently, on every cell or row update, processRowUpdate prop gets fired once.

If we go with the suggested behavior, the processRowUpdate will get fired more than once each edit (depending on the impacted rows).

The current behavior might also be useful in some cases where the users intend to modify the spanned cells based on the values.

break-spanned-cells.mp4

And the column spanning works in a similar way too, so it should be fine for now.

We might wait for some user feedback around this before deciding to go with the suggested approach. And even if we decide to go with it, we do it at a major, to avoid breaking changes.

Jcronoxer · 2024-09-17T09:33:33Z

@MBilalShafi when is the final release of row spanning on the library expected ?

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

packages/x-data-grid/src/hooks/features/rows/gridRowSpanningUtils.ts

packages/x-data-grid/src/hooks/features/rows/useGridRowSpanning.ts

arminmeh · 2024-09-18T12:39:27Z

Additional points for the follow-up section

row selection with row spanning
cell selection with row spanning

MBilalShafi · 2024-09-18T19:28:52Z

row selection with row spanning

cell selection with row spanning

In my initial testing, the basic cell selection worked fine, did you spot any specific interaction not properly working?
Regarding row selection, I'm not sure tbh if it makes much sense when row spanning is on. IMO we could wait for some feedback to validate if the row selection is actually relevant with the row-spanning feature. Wdyt?

MBilalShafi · 2024-09-18T19:30:07Z

@MBilalShafi when is the final release of row spanning on the library expected ?

It should be released this week or coming week.

arminmeh · 2024-09-18T21:14:59Z

In my initial testing, the basic cell selection worked fine, did you spot any specific interaction not properly working?

Did not test it. I meant more to validate that we want it to behave the way it is (even if it means it can't be used together)

docs/data/data-grid/row-spanning/row-spanning.md

Co-authored-by: Sycamore <71297412+samuelsycamore@users.noreply.github.com> Signed-off-by: Bilal Shafi <bilalshafidev@gmail.com>

oliviertassinari

Very cool 👍

packages/x-data-grid/src/components/cell/GridCell.tsx

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

oliviertassinari · 2024-09-20T21:18:28Z

packages/x-data-grid/src/components/cell/GridCell.tsx

+    return (
+      <div
+        data-colindex={colIndex}
+        role="presentation"


This shouldn't be used, no?

Suggested change

role="presentation"

Looking https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/presentation_role, I see no purpose for the role here.

Actually, this seems needed. If I understand https://www.w3.org/WAI/ARIA/apg/practices/hiding-semantics/ correctly, because the parent is a role="row", it needs each child to have a cell role, since this one doesn't have it, it should be hidden.

Signed-off-by: Bilal Shafi <bilalshafidev@gmail.com> Co-authored-by: Sycamore <71297412+samuelsycamore@users.noreply.github.com>

tomekxoxo · 2024-10-10T08:51:20Z

Hello, can you merge this feature to the v5 version of MUI?
Have you tested if export to excel works properly?

MBilalShafi · 2024-10-10T11:33:36Z

Hey @tomekxoxo,

We only backport critical bug fixes to the previous major versions, I'd suggest you to consider upgrading to a new major.

We have the migration guides available for every major release upgrade. Some of the changes could even be automatically applied using codemods.

Here are the migration guides:
v5 -> v6 migration: mui.com/x/migration/migration-data-grid-v5
v6 -> v7 migration: mui.com/x/migration/migration-data-grid-v6

Have you tested if export to excel works properly

I don't think the row spanning is integrated with Excel export as of now.

[Data Grid] Row spanning POC

5d31a72

MBilalShafi added the component: data grid This is the name of the generic UI component, not the React module! label Aug 6, 2024

MBilalShafi changed the title ~~[Data Grid] Row spanning POC~~ [DataGrid] Row spanning POC Aug 6, 2024

Fix a few issues

958808c

MBilalShafi changed the title ~~[DataGrid] Row spanning POC~~ [DataGrid] Row spanning Aug 7, 2024

MBilalShafi added 7 commits August 7, 2024 18:18

Add basic keyboard navigation

7894afa

Add hook and initializer to other packages

b5d7add

Some housekeeping

bbfad03

Remove planned flag

0c86f39

Add support for valueGetter

0880c1b

Add rowSpanValueGetter to support exclusion from row spanning even wh…

c526ea6

…en there are repeated values

Improve docs a bit

ed97a2d

MBilalShafi commented Aug 7, 2024

View reviewed changes

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

This comment was marked as outdated.

Sign in to view

MBilalShafi added 2 commits August 19, 2024 20:12

Add new feature flag

c458c57

Merge branch 'master' into row-spanning

c4dd3d7

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

MBilalShafi added 3 commits August 20, 2024 00:12

Fix column resize

51bc9d4

Add a couple of demos

e2a4605

Some changes on the demos

bf15c95

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

This comment was marked as outdated.

Sign in to view

MBilalShafi added 3 commits August 20, 2024 19:03

Update docs and add a new demo

427e61d

Support keyboard navigation from column spanned cell to row spanned cell

7ddcc0a

Merge branch 'master' into row-spanning

5dfc6b7

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

MBilalShafi added 2 commits August 21, 2024 18:41

Improvement

7c8dfbc

Lint

5a121b1

Skip JS dom test

f68b1ed

arminmeh reviewed Sep 18, 2024

View reviewed changes

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

arminmeh approved these changes Sep 18, 2024

View reviewed changes

packages/x-data-grid/src/hooks/features/rows/gridRowSpanningUtils.ts Outdated Show resolved Hide resolved

packages/x-data-grid/src/hooks/features/rows/useGridRowSpanning.ts Outdated Show resolved Hide resolved

arminmeh reviewed Sep 18, 2024

View reviewed changes

packages/x-data-grid/src/hooks/features/rows/useGridRowSpanning.ts Outdated Show resolved Hide resolved

Armin's code review comments addressed

4c9a218

samuelsycamore self-requested a review September 18, 2024 23:36

samuelsycamore reviewed Sep 19, 2024

View reviewed changes

docs/data/data-grid/row-spanning/row-spanning.md Outdated Show resolved Hide resolved

MBilalShafi and others added 4 commits September 20, 2024 05:55

Apply suggestions from code review

085b108

Co-authored-by: Sycamore <71297412+samuelsycamore@users.noreply.github.com> Signed-off-by: Bilal Shafi <bilalshafidev@gmail.com>

Move warning and rephrase

fdcf26b

CI

31b421e

Update

9408e2f

MBilalShafi enabled auto-merge (squash) September 20, 2024 01:19

Merge branch 'master' into row-spanning

7031634

MBilalShafi disabled auto-merge September 20, 2024 01:49

MBilalShafi merged commit bb30d3a into mui:master Sep 20, 2024
14 of 15 checks passed

MBilalShafi deleted the row-spanning branch September 20, 2024 01:52

oliviertassinari mentioned this pull request Sep 20, 2024

[website] Add 'Row spanning' mui/material-ui#43831

Merged

oliviertassinari reviewed Sep 20, 2024

View reviewed changes

arthurbalduini pushed a commit to arthurbalduini/mui-x that referenced this pull request Sep 30, 2024

[DataGrid] Row spanning (mui#14124)

e76bebf

Signed-off-by: Bilal Shafi <bilalshafidev@gmail.com> Co-authored-by: Sycamore <71297412+samuelsycamore@users.noreply.github.com>

oliviertassinari mentioned this pull request Sep 30, 2024

[core] Replace minWidth, maxWidth with width #14776

Merged

alelthomas mentioned this pull request Oct 4, 2024

[data grid][docs] Change demo name example #14822

Merged

oliviertassinari mentioned this pull request Oct 6, 2024

[test] Replace waitFor() with act() #14851

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Row spanning #14124

[DataGrid] Row spanning #14124

MBilalShafi commented Aug 6, 2024 •

edited

Loading

mui-bot commented Aug 6, 2024 •

edited

Loading

MBilalShafi Aug 7, 2024

KenanYusuf Aug 14, 2024

arminmeh Aug 15, 2024

MBilalShafi Aug 19, 2024 •

edited

Loading

arminmeh Aug 21, 2024

MBilalShafi Aug 22, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

Jcronoxer commented Sep 17, 2024

arminmeh commented Sep 18, 2024

MBilalShafi commented Sep 18, 2024

MBilalShafi commented Sep 18, 2024 •

edited

Loading

arminmeh commented Sep 18, 2024

oliviertassinari left a comment

oliviertassinari Sep 20, 2024

oliviertassinari Oct 6, 2024

tomekxoxo commented Oct 10, 2024 •

edited

Loading

MBilalShafi commented Oct 10, 2024

[DataGrid] Row spanning #14124

[DataGrid] Row spanning #14124

Conversation

MBilalShafi commented Aug 6, 2024 • edited Loading

Todos

Follow-up Todos

Changelog

mui-bot commented Aug 6, 2024 • edited Loading

MBilalShafi Aug 7, 2024

Choose a reason for hiding this comment

KenanYusuf Aug 14, 2024

Choose a reason for hiding this comment

arminmeh Aug 15, 2024

Choose a reason for hiding this comment

MBilalShafi Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

arminmeh Aug 21, 2024

Choose a reason for hiding this comment

MBilalShafi Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

Jcronoxer commented Sep 17, 2024

arminmeh commented Sep 18, 2024

MBilalShafi commented Sep 18, 2024

MBilalShafi commented Sep 18, 2024 • edited Loading

arminmeh commented Sep 18, 2024

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari Sep 20, 2024

Choose a reason for hiding this comment

oliviertassinari Oct 6, 2024

Choose a reason for hiding this comment

tomekxoxo commented Oct 10, 2024 • edited Loading

MBilalShafi commented Oct 10, 2024

MBilalShafi commented Aug 6, 2024 •

edited

Loading

mui-bot commented Aug 6, 2024 •

edited

Loading

MBilalShafi Aug 19, 2024 •

edited

Loading

MBilalShafi Aug 22, 2024 •

edited

Loading

MBilalShafi commented Sep 18, 2024 •

edited

Loading

tomekxoxo commented Oct 10, 2024 •

edited

Loading