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] Add disableVirtualization to disable virtualization completely #2326

Merged
merged 11 commits into from
Aug 31, 2021

Conversation

m4theushw
Copy link
Member

@m4theushw m4theushw commented Aug 12, 2021

Preview https://deploy-preview-2326--material-ui-x.netlify.app/components/data-grid/virtualization/#disable-virtualization

Closes #1781

I created a separate hook to not mix concerns and to touch as little as possible the virtualization logic.

  • cf7ac77 moves the scrollToIndexes and scroll to a dedicated hook because they're not tied to the virtualization
  • 624352e better name the hook
  • fe60f82 adds a useGridNoVirtualization hook to sync the state and handle the scroll when virtualization is off

Preview: https://deploy-preview-2326--material-ui-x.netlify.app/storybook/?path=/story/x-grid-tests-rows--disable-virtualization&globals=measureEnabled:false

@m4theushw m4theushw added component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request labels Aug 12, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 12, 2021

Thoughts

@m4theushw
Copy link
Member Author

There's a bug in this PR. Filter and try to navigate with the keyboard, it doesn't work.


We are the only one in the benchmark to not directly rely on the native scroll feature rendering.

@oliviertassinari Yeah, I'll investigate if it's easier to use the native scroll instead. Basically, we need to remove the two divs between the window and the rendering zone.

image

Do we disable column virtualization?

Yes

Should we add documentation about it in https://material-ui.com/components/data-grid/virtualization/?

I added a small section in https://deploy-preview-2326--material-ui-x.netlify.app/components/data-grid/virtualization/#disable-virtualization

@m4theushw m4theushw added the on hold There is a blocker, we need to wait label Aug 12, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 12, 2021

Great, should we explain why developers might want to disable it?

  • Yeah, I'll investigate if it's easier to use the native scroll instead.

I meant for the future, not this PR 😁

Capture d’écran 2021-08-13 à 00 40 04


{{"demo": "pages/components/data-grid/virtualization/VirtualizationApiNoSnap.js", "bg": "inline", "hideToolbar": true}}
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove this section?

Copy link
Member Author

Choose a reason for hiding this comment

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

One of the changes is to move the scroll methods to a separated hook. Since this interface is empty now I removed it. Maybe we should add a page for scrolling with a demo: #1103 (comment). In this new page we could document the API available.

@m4theushw m4theushw removed the on hold There is a blocker, we need to wait label Aug 19, 2021
@flaviendelangle
Copy link
Member

flaviendelangle commented Aug 25, 2021

Probably also closes #1519 and #1151

@m4theushw m4theushw changed the base branch from master to next August 30, 2021 22:56
@m4theushw m4theushw merged commit e1d572f into mui:next Aug 31, 2021
@m4theushw m4theushw deleted the disable-virtualization branch August 31, 2021 22:23
m4theushw added a commit to m4theushw/mui-x that referenced this pull request Sep 28, 2021
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! new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DataGrid] Allow to disable virtualization
4 participants