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

[Switch] Fix theme styleOverrides #25776

Closed
wants to merge 2 commits into from

Conversation

sparkpunk
Copy link

@sparkpunk sparkpunk commented Apr 15, 2021

Fixes #25773

@mui-pr-bot
Copy link

mui-pr-bot commented Apr 15, 2021

Details of bundle changes

Generated by 🚫 dangerJS against 7c79e05

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Could you add some context why we need this change? Ideally with a test or codesandbox illustrating the issue.

@eps1lon eps1lon added component: switch This is the name of the generic UI component, not the React module! PR: needs test The pull request can't be merged status: waiting for author Issue with insufficient information labels Apr 15, 2021
@oliviertassinari oliviertassinari changed the title Adding theme attribute to SwitchRoot element [Switch] Fix theme styleOverrides Apr 15, 2021
@mnajdova
Copy link
Member

@eps1lon I've linked the issue that this PR is solving

@eps1lon
Copy link
Member

eps1lon commented Apr 16, 2021

@sparkpunk Could you run yarn prettier and commit the changes to get CI green?

@sparkpunk
Copy link
Author

@eps1lon sure!

@mnajdova
Copy link
Member

I apologize, I spend more time on this issue, seems like in the original codesandbox, the import to the ThemeProvider was not correct! We should import from @material-ui/core/styles, not @material-ui/core! This is easy to be overlooked :( https://codesandbox.io/s/pensive-bhaskara-i7r09?file=/src/theme.js

I am closing the issue and PR as it seems like everything is workingas expected, just by using the wrong provider, the incorrect context was changed.

@mnajdova mnajdova closed this Apr 16, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 16, 2021

While this theme provider issue was a false-positive, the double root, doesn't seem to be one:

Screenshot 2021-04-16 at 14 13 08

We didn't use to forward the root directly (v4):

https://github.com/mui-org/material-ui/blob/d72862cf60b2ea314b5d9e3ed163a0f605e2eef2/packages/material-ui/src/Switch/Switch.js#L181

@mnajdova
Copy link
Member

Looks like regression comming from #24693 the classes object propagated to the SwitchBase is incorrect.

@sparkpunk
Copy link
Author

Thanks @mnajdova! Can't believe it was an import error. D'oh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: switch This is the name of the generic UI component, not the React module! PR: needs test The pull request can't be merged status: waiting for author Issue with insufficient information
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Switch] styleOverrides selector do not appear to work
5 participants