[material-ui] Improve color map filter on styles #43579
Conversation
Netlify deploy previewhttps://deploy-preview-43579--material-ui.netlify.app/ @material-ui/core: parsed: -0.07% 😍, gzip: 0.00% 😍 Bundle size reportDetails of bundle changes (Toolpad) |
There was a problem hiding this comment.
We should also update the following components:
- Alert
- ButtonGroup
- Chip
- Fab
- FilledInput
- FormLabel
- IconButton
- Input
- LinearProgress
- Link
- OutlinedInput
- PaginationItem
- Switch
- Typography
Was there a reason these were skipped? I just searched for "...Object.entries(theme.palette)" to find all occurences that needs to be updated.
|
@mnajdova you're correct! Thanks for the review (I initially searched for |
|
@siriwatknp I'm debating whether to add a test for this and how to do it. On the one hand, this is an edge case that we don't document and will stop working when we update the theme structure in v7. So, it feels too much of an edge case to add to On the other hand, a regression was already reported about it, and if we wanted to add a test, it should run on all components, so in theory, it should exist in I lean towards adding a test to |
yes, I agree. |
|
@siriwatknp we already have one 🎉 , so I only had to add the case from #41772: 982213f |
| ...Object.entries(theme.palette) | ||
| .filter(([, palette]) => palette && palette.main) | ||
| .filter(createSimplePaletteValueFilter()) | ||
| .map(([color]) => ({ | ||
| props: { color }, | ||
| style: { |
There was a problem hiding this comment.
I guess we could replace all of these variant generations with CSS variables? https://caniuse.com/?search=css%20variables is compatible with https://mui.com/material-ui/getting-started/supported-platforms/#browser. I guess this could mean for CSS static extraction a smaller CSS output. For emotion, I'm not sure it would bring a lot of benefits.
| .filter(([, palette]) => palette && palette.main) | ||
| .filter(createSimplePaletteValueFilter()) |
There was a problem hiding this comment.
What's the impact on performance? I suspect this should be computed in the theme provider to process the theme and generate a list of compatible values only once, so it's cached.
Now, maybe with memoTheme it doesn't matter so much anymore.
Closes: #42698
Fixes: #41772
Improve the check of colors mapped for styles to avoid errors or invalid styles. Use a shared util for consistency.
Bug fix
Before: https://stackblitz.com/edit/stackblitz-starters-uk6qhx?file=src%2FApp.tsx
After: https://codesandbox.io/p/sandbox/epic-dijkstra-7frj4f?workspaceId=836800c1-9711-491c-a89b-3ac24cbd8cd8