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

[data grid] Get correct width for invisible iconContainer #16399

Merged
merged 5 commits into from
Jan 31, 2025

Conversation

michelengelen
Copy link
Member

@michelengelen michelengelen commented Jan 30, 2025

Fixes #15388

This is a fix to get the scrollWidth instead of the clientWidth when the column has sorting applied. This is to calculate the columns width on autosize. Previously it was not respecting the sorting button.

The problem before was that we get the headerContainer from the ref stored in the apiRef which was not holding the current state (and therefore visibility) of the sorting button. This resulted in the value stored in clientWidth to always be 0.

For the fix I would much rather rely on the sortModelLookup, but since this is outside of a component or hook we cannot use the grids selectors.

Any ideas how to improve this are welcome!

@michelengelen michelengelen added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Sorting Related to the data grid Sorting feature needs cherry-pick The PR should be cherry-picked to master after merge v7.x labels Jan 30, 2025
@michelengelen michelengelen self-assigned this Jan 30, 2025
@mui-bot
Copy link

mui-bot commented Jan 30, 2025

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

Generated by 🚫 dangerJS against cf165cc

@arminmeh
Copy link
Contributor

Instead of all of this, you can also just remove these styles
https://github.com/mui/mui-x/blob/master/packages/x-data-grid/src/components/containers/GridRootStyles.ts#L306

They are actually hiding the icon while autosizing which makes the icon container width 0
@romgrk added those some time ago here https://github.com/mui/mui-x/pull/12013/files#diff-e1db53193a627d36d1cd3f651a954ee1e32ecb93caa020446631d722bb1be75cR232

@mui/xgrid anyone else knows if we need them for something else?

Quick UI test didn't show me anything wrong after removing it and autosize includes the active sort icon

@michelengelen michelengelen enabled auto-merge (squash) January 30, 2025 17:06
@michelengelen
Copy link
Member Author

Sry for the inconvenience, but I did check the suggestion from @arminmeh and could not find a case where that was not working, so I instead applied that, since it is cleaner.

If we can make this into the release of v7 (when cherry-picking is done) that would be awesome!

@michelengelen michelengelen mentioned this pull request Jan 31, 2025
@michelengelen michelengelen merged commit 0736d31 into mui:master Jan 31, 2025
16 checks passed
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
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Sorting Related to the data grid Sorting feature 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.

[data grid] Column header labels still abbreviated after Autosize Columns
4 participants