[DataGrid] Variable row height #3218

DanailH · 2021-11-18T21:35:51Z

Fixes #438

Preview -> https://deploy-preview-3218--material-ui-x.netlify.app/components/data-grid/rows/#variable-row-height

CHANGELOG entry for the release

🚣 Introduce variable row height (#438) @DanailH

Allow for setting a row-specific height. Before all rows had the same height, now you can set the height on a per-row basis.

<DataGrid
  getRowHeight={({ id }: GridRowHeightParams) => id % 2 === 0 ? 100 : null}
/>

ToDo:

Tests
Docs

…DataGrid-variable-row-height

github-actions · 2021-11-24T15:10:54Z

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

github-actions · 2021-12-03T18:03:08Z

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

github-actions · 2021-12-09T13:39:58Z

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

DanailH · 2021-12-09T13:47:09Z

Ok ... I would really like a review of what has been done so far. The functionality is almost ready but I'm struggling with fixing the failing tests. I'm probably missing a key interaction because this is closely tight to the virtualization and is impacted by every functionality that changes the rows in a way (sorting, filtering, pagination, treeData, density, etc). If someone can spare a pair of eyes to check it I would really appreciate it.

I skipped 2 tests (both are the same, related to the translation using the Theme) - for some reason it enters an infinite loop there.

Another thing that I don't really get is how are the tests from yarn test:karma passing and the ones from yarn test:coverage failing?

m4theushw

I didn't look into the functionality yet. However, I'm already using some parts in #3387.

I don't think we should treat variable row height as a feature, the column variable width is not too.

For the remaining tests that are failing only with yarn test:unit, you can fix them by using disableVirtualization. They fail because in jsdom rootRef.current!.clientHeight! is 0. You can refer to the master/detail PR because the CI is green there and I'm reusing most part of your work here.

packages/grid/_modules_/grid/hooks/features/rowHeight/useGridRowHeight.ts

flaviendelangle · 2022-01-21T10:50:12Z

packages/grid/_modules_/grid/hooks/features/dimensions/useGridDimensions.ts

-    if (!dimensions) {
-      return 0;
-    }
+    const renderContext = apiRef.current.unstable_getRenderContext();


I'm not a huge fan of exposing virtualization internals upper in the DataGrid component tree.
But I don't have a good solution either.

If @m4theushw has a better idea.
Otherwise, I'm fine with merging as is and taking time later to improve.

Actually the keyboard navigation won't work as expected if disableVirtualization=true. With virtualization disabled, the last row index in the render context points to the last row in the entire row set. I'm waiting for #3667 to be merged to able to test if there're no regressions. In the worst scenario, I would keep the old behavior if getRowHeight is not being used.

I tried calculating it without the renderContext -> the problem is that because of the variable row height you need to know the indexes of the first and last rows in the viewport to calculate the total number of visible rows. If anyone have any ideas I'm open to them.

There's a regression here. Pressing Page Up is jumping directly to the column header while not on the first row yet. I don't mind if we ship this small bug only affecting those users using getRowHeight, but keeping the old behavior for those who don't use it. I think the correct way would be to base on rowsMeta.positions and the current scroll position.

github-actions · 2022-01-21T17:00:24Z

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

cherniavskii · 2022-01-21T17:21:30Z

I've resolved a conflict caused by merging #3667

github-actions · 2022-01-24T13:59:47Z

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

…ailH/material-ui-x into feature/DataGrid-variable-row-height

m4theushw

Well done.

…e/DataGrid-variable-row-height

DanailH · 2022-01-25T08:00:31Z

Merged. I'll update the PR description for the release.

m4theushw · 2022-01-25T12:11:52Z

Although we merged, there's still room for improvement. There're 3 things we could do in the future:

Add an apiRef.current.setRowHeight method to the API to imperatively change the row height, similar to the one available to control the column width
Fix keyboard navigation, e.g. Page Up, to use the scroll position and the position of each row
Add a resize handle to the rows to allow to change their height. It's the same thing we have for the columns. We need apiRef.current.setRowHeight for that.

flaviendelangle · 2022-01-25T12:15:09Z

I created an issue to keep track of those subjects
#3737

oliviertassinari · 2022-06-13T20:04:33Z

A thought on the getRowHeight API signature: mui/material-ui#32766 (comment)

DanailH added 2 commits November 18, 2021 22:28

initial version

682700c

Merge branch 'next' of github.com:mui-org/material-ui-x into feature/…

191136f

…DataGrid-variable-row-height

DanailH added component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request labels Nov 18, 2021

DanailH self-assigned this Nov 18, 2021

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

resolve conflicts

e9786a1

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

DanailH added 7 commits November 30, 2021 10:53

Fix typings and names

f08aa74

update docs and typings

247866f

formatting

d47b438

formatting

ed71d1b

use paginated visible sorted rows

60739bc

Fix implementation to fix tests

d07c11d

test

9fa2777

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

get latest

51b6770

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

move rowsMeta to state

ce0f02d

github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Dec 9, 2021

DanailH requested review from m4theushw and flaviendelangle December 9, 2021 13:47

m4theushw reviewed Dec 10, 2021

View reviewed changes

packages/grid/_modules_/grid/hooks/features/rowHeight/useGridRowHeight.ts Outdated Show resolved Hide resolved

update solution

ca386fa

DanailH force-pushed the feature/DataGrid-variable-row-height branch from 6aef23a to ca386fa Compare December 10, 2021 13:56

Fix getViewportPageSize behaviour

0f77b7b

DanailH requested review from flaviendelangle and m4theushw January 20, 2022 15:51

flaviendelangle reviewed Jan 21, 2022

View reviewed changes

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

Merge branch 'master' into feature/DataGrid-variable-row-height

ac23071

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

empty

fdaf249

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

DanailH added 3 commits January 24, 2022 16:04

revert logic

f544342

resove conflicts

98c5b06

Merge branch 'feature/DataGrid-variable-row-height' of github.com:Dan…

1f0f937

…ailH/material-ui-x into feature/DataGrid-variable-row-height

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

DanailH requested a review from flaviendelangle January 24, 2022 15:43

m4theushw approved these changes Jan 25, 2022

View reviewed changes

Merge branch 'master' of github.com:mui-org/material-ui-x into featur…

51f52c2

…e/DataGrid-variable-row-height

DanailH merged commit e7ac270 into mui:master Jan 25, 2022

flaviendelangle mentioned this pull request Jan 25, 2022

[data grid] Variable row height follow-up #3737

Closed

3 tasks

cherniavskii mentioned this pull request Jan 27, 2022

[DataGrid] Keyboard navigation with variable row height #3758

Closed

joserodolfofreitas mentioned this pull request Jan 31, 2022

[data grid] Row resize handle #3801

Open

m4theushw mentioned this pull request Feb 2, 2022

[DataGridPro] Add unstable_setRowHeight method to apiRef #3751

Merged

m4theushw mentioned this pull request Feb 25, 2022

[DataGrid] Sorting in a DataGrid with many columns can shift rows in relation to the column headers #3890

Closed

2 tasks

m4theushw mentioned this pull request Nov 22, 2022

[data grid] Remove headerHeight and rowHeight from state #5620

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Variable row height #3218

[DataGrid] Variable row height #3218

DanailH commented Nov 18, 2021 •

edited

Loading

github-actions bot commented Nov 24, 2021

github-actions bot commented Dec 3, 2021

github-actions bot commented Dec 9, 2021

DanailH commented Dec 9, 2021

m4theushw left a comment •

edited

Loading

flaviendelangle Jan 21, 2022

m4theushw Jan 21, 2022 •

edited

Loading

DanailH Jan 21, 2022

m4theushw Jan 24, 2022

DanailH Jan 24, 2022

github-actions bot commented Jan 21, 2022

cherniavskii commented Jan 21, 2022

github-actions bot commented Jan 24, 2022

m4theushw left a comment

DanailH commented Jan 25, 2022

m4theushw commented Jan 25, 2022

flaviendelangle commented Jan 25, 2022

oliviertassinari commented Jun 13, 2022

[DataGrid] Variable row height #3218

[DataGrid] Variable row height #3218

Conversation

DanailH commented Nov 18, 2021 • edited Loading

CHANGELOG entry for the release

github-actions bot commented Nov 24, 2021

github-actions bot commented Dec 3, 2021

github-actions bot commented Dec 9, 2021

DanailH commented Dec 9, 2021

m4theushw left a comment • edited Loading

Choose a reason for hiding this comment

flaviendelangle Jan 21, 2022

Choose a reason for hiding this comment

m4theushw Jan 21, 2022 • edited Loading

Choose a reason for hiding this comment

DanailH Jan 21, 2022

Choose a reason for hiding this comment

m4theushw Jan 24, 2022

Choose a reason for hiding this comment

DanailH Jan 24, 2022

Choose a reason for hiding this comment

github-actions bot commented Jan 21, 2022

cherniavskii commented Jan 21, 2022

github-actions bot commented Jan 24, 2022

m4theushw left a comment

Choose a reason for hiding this comment

DanailH commented Jan 25, 2022

m4theushw commented Jan 25, 2022

flaviendelangle commented Jan 25, 2022

oliviertassinari commented Jun 13, 2022

DanailH commented Nov 18, 2021 •

edited

Loading

m4theushw left a comment •

edited

Loading

m4theushw Jan 21, 2022 •

edited

Loading