-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[DataGridPro] Keep bottom pinned row at the bottom #13313
Conversation
Deploy preview: https://deploy-preview-13313--material-ui-x.netlify.app/ |
packages/x-data-grid/src/components/virtualization/GridVirtualScroller.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Can you add a unit test for this regression?
// In cases where the columns exceed the available width, | ||
// the horizontal scrollbar should be shown even when there're no rows. | ||
// Keeping 1px as minimum height ensures that the scrollbar will visible if necessary. | ||
const height = Math.max(contentHeight, 1); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this code was preventing this 0-height warning from running in some cases, so fixing it broke some tests all over the place that didn't have explicit dimensions on the grid container. I'll need to debug & update those tests before merging this PR. The test runner also doesn't seem to match the warning to which test it originated from precisely, so sometimes it marks one test as failed but the warning came from another test. As in if I run the test with .only
it succeeds, but if I run the test suite as a whole it fails. Lots of fun ahead.
Closes #12925
Before: https://codesandbox.io/p/sandbox/funny-leaf-6fv4qt
After: https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-9vpwwk