[theme] Fix styleOverrides with nested selectors #25156
Conversation
|
How do I fix for the browsers the tests are failing for? |
oliviertassinari
added
bug 🐛
oliviertassinari
changed the title
There was a problem hiding this comment.
The failing test case come from the downside I have described in the issue with the first option. I think that we can start by keeping the deepmerge and update all the +90 migrated components to the second option. Then, we can chat about the tradeoff with Marija once she comes back and reevaluate.
|
@ruppysuppy I have pushed an update trying the section option. It works. Do you want to update all the other components? I also think that we could consider a test case just for the Slider to assert that the fix works, it won't add a lot of value but could be as some level of defense for future regression. |
oliviertassinari
force-pushed
the
slider-style-customization
branch
from
aa2c8ae
to
c6c9e9a
Compare
oliviertassinari
force-pushed
the
slider-style-customization
branch
from
ee9c45b
to
3745249
Compare
| return deepmerge(styles.root, { | ||
| ...(!styleProps.formControl && styles.formControl), | ||
| ...(styleProps.size === 'small' && styles.sizeSmall), | ||
| ...(styleProps.shrink && styles.shrink), | ||
| ...(!styleProps.disableAnimation && styles.animated), | ||
| ...styles[styleProps.variant], | ||
| [`& .${formLabelClasses.asterisk}`]: styles.asterisk, | ||
| }); | ||
| return deepmerge( | ||
| { | ||
| ...(!styleProps.formControl && styles.formControl), | ||
| ...(styleProps.size === 'small' && styles.sizeSmall), | ||
| ...(styleProps.shrink && styles.shrink), | ||
| ...(!styleProps.disableAnimation && styles.animated), | ||
| ...styles[styleProps.variant], | ||
| [`& .${formLabelClasses.asterisk}`]: styles.asterisk, | ||
| }, | ||
| styles.root || {}, | ||
| ); |
There was a problem hiding this comment.
Hi @oliviertassinari can you revisit this change again? This change causes a regression in our product.
After this change:
Revert this specific change in node_modules and it works well:
I can't tell why this is happening but it seems very strange.
There was a problem hiding this comment.
@Jack-Works we changed how the styleOverrides work in the latest release alpha.32, can you try with this version? Also would be useful if you can share the code, preferably in an issue so that it would help other developers if they stumble upon it.
There was a problem hiding this comment.
Our product is open sourced but it's but hard to build and preview. If you'd like to have a try I can guide you how to setup the project. I'll try alpha 32 later, thanks!
There was a problem hiding this comment.
Let's try alpha.32 and see, if not simple repro would be best for us to debug.
There was a problem hiding this comment.
Hmm, I found we're already on alpha 32... I can try to make a reproduce but I doubt if I can make a simple repro
There was a problem hiding this comment.
It appears to be a problem in our codebase. We overwrite MuiInputLabel in the theme. Remove them solves the problem
Fixes: #25075