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] Support column virtualization with dynamic row height #15541

Merged

Conversation

cherniavskii
Copy link
Member

@cherniavskii cherniavskii commented Nov 21, 2024

@cherniavskii cherniavskii added component: data grid This is the name of the generic UI component, not the React module! feature: Row height Related to the data grid Row height features enhancement This is not a bug, nor a new feature needs cherry-pick The PR should be cherry-picked to master after merge v7.x labels Nov 21, 2024
@mui-bot
Copy link

mui-bot commented Nov 21, 2024

@cherniavskii cherniavskii force-pushed the column-virtualization-dynamic-height branch from 028dfe6 to 29c8454 Compare November 21, 2024 18:05
@cherniavskii cherniavskii marked this pull request as ready for review November 22, 2024 10:47
@cherniavskii cherniavskii requested a review from a team November 22, 2024 10:47
* If `true`, the Data Grid doesn't disable column virtualization when `getRowHeight` is set to `() => 'auto'`.
* By default, column virtualization is disabled when dynamic row height is enabled to measure the row height correctly.
* For datasets with a large number of columns, this can cause performance issues.
* The downside of enabling this prop is that the row height will be estimated based the cells that are currently rendered, which can cause row height change when scrolling horizontally.
Copy link
Member

Choose a reason for hiding this comment

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

Love that, giving the control of the UX tradeoff in the hands of designers.

Copy link
Member

@oliviertassinari oliviertassinari Nov 24, 2024

Choose a reason for hiding this comment

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

It looks like AG Grid doesn't support this option https://www.ag-grid.com/javascript-data-grid/row-height/#auto-row-height. Still, experiencing this https://codesandbox.io/p/sandbox/focused-rain-zxyt6p?file=%2Fdemo.js%3A21%2C35, I almost wonder if this shouldn't be the default. As a developer or Product Manager, it's easier to search for "layout shift with auto height" than "grid slow performance" 😄.

Anyway, we have this in the docs. We will see how this goes.

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like AG Grid doesn't support this option ag-grid.com/javascript-data-grid/row-height#auto-row-height.

I think they don't need it – they took an interesting approach of per-column row auto height.
So they can exclude specific columns from virtualization (i.e. always render them) if they have auto height enabled.
It's smart.

@cherniavskii cherniavskii merged commit 00c0b2b into mui:master Nov 23, 2024
18 checks passed
@cherniavskii cherniavskii deleted the column-virtualization-dynamic-height branch November 23, 2024 13:04
Copy link

Cherry-pick PRs will be created targeting branches: v7.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature feature: Row height Related to the data grid Row height features needs cherry-pick The PR should be cherry-picked to master after merge v7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DataGrid] Possibility to virtualize columns with dynamic row height
4 participants