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 minHeight and maxHeight on flex parent container #14614

Merged
merged 40 commits into from
Sep 25, 2024

Conversation

cherniavskii
Copy link
Member

@cherniavskii cherniavskii commented Sep 13, 2024

@cherniavskii cherniavskii added component: data grid This is the name of the generic UI component, not the React module! feature: Rendering layout Related to the data grid Rendering engine labels Sep 13, 2024
@mui-bot
Copy link

mui-bot commented Sep 13, 2024

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

Updated pages:

Generated by 🚫 dangerJS against 6a9c2fa

function ContainerMeasurements({ containerRef }) {
const [containerHeight, setContainerHeight] = React.useState(0);

useResizeObserver(containerRef, (entries) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this component to visualize the DataGrid height (no need to check it in the dev tools):

It works fine locally, but the resize observer callback is not called in the deploy preview for some reason 🫠
I'll look into it later, but the PR in its current form is ready for review.

Copy link
Member Author

@cherniavskii cherniavskii Sep 20, 2024

Choose a reason for hiding this comment

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

@cherniavskii cherniavskii marked this pull request as ready for review September 18, 2024 18:21
@cherniavskii cherniavskii requested a review from a team September 18, 2024 18:21
Copy link
Contributor

Choose a reason for hiding this comment

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

Small issue with the scrollbar on chrome/linux (which usually behaves like chrome/windows):

Screencast.from.2024-09-18.17-48-40.webm

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I can't reproduce it in Chromium on Ubuntu:

Screen.Recording.2024-09-19.at.01.36.24.mov

Copy link
Contributor

Choose a reason for hiding this comment

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

It only happens when the zoom is not 100%.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, with non-100% zoom, I can reproduce this on MacOS as well 👍🏻
Will look into it!

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, and the fix also solves another issue that I didn't notice before: #14614 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Could this be related to #14814 @romgrk ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so, I can reproduce the issue with both 7.19.0 and 7.18.0 (before this PR was merged)

@cherniavskii cherniavskii merged commit 1751693 into mui:master Sep 25, 2024
18 checks passed
@cherniavskii cherniavskii deleted the support-maxHeight branch September 25, 2024 07:21
arthurbalduini pushed a commit to arthurbalduini/mui-x that referenced this pull request Sep 30, 2024
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! feature: Rendering layout Related to the data grid Rendering engine
Projects
None yet
6 participants