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 missing style overrides #16272

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

KenanYusuf
Copy link
Member

Fixes #16159

There were some missing classes in the GridRootStyles overrides. Some overrides were moved to the component styled where it was straightforward to do so.

@KenanYusuf KenanYusuf added component: data grid This is the name of the generic UI component, not the React module! customization: theme Centered around the theming features labels Jan 20, 2025
@mui-bot
Copy link

mui-bot commented Jan 20, 2025

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

Generated by 🚫 dangerJS against 9fdd9c2

@KenanYusuf KenanYusuf requested a review from a team January 21, 2025 09:18
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'd avoid the changes in this file, some classes really don't have a viable use-case to be style-overridden (like virtualScroller-hasScrollX).

Copy link
Member Author

Choose a reason for hiding this comment

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

some classes really don't have a viable use-case to be style-overridden

It's unlikely, but I don't think there is any harm in making it possible. As a user, I would assume that styles are overridable for any MUI class.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that class publicly documented? Some of them are just for internal use.

The downside is that it makes the code here more verbose.

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed in team meeting today, the main points were:

  • There's no such thing as a private/internal class really because they are visible when inspecting the DOM
  • We could come up with a different naming convention for classes that are only intended to be used internally

We came to a consensus that all classes should be added to style overrides for consistency.

@KenanYusuf KenanYusuf merged commit 6b0c8f4 into mui:master Jan 27, 2025
18 checks passed
@KenanYusuf KenanYusuf deleted the missing-style-overrides branch January 27, 2025 18:56
KenanYusuf added a commit to KenanYusuf/mui-x that referenced this pull request Jan 27, 2025
A-s-h-o-k pushed a commit to A-s-h-o-k/mui-x that referenced this pull request Feb 4, 2025
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! customization: theme Centered around the theming features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Theme styleOverrides are ignored
4 participants