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

[material-ui][theme] Lower CSS Specificity for color scheme rules in CSS Vars Theme #42893

Open
joebochill opened this issue Jul 10, 2024 · 5 comments
Assignees
Labels
customization: theme Centered around the theming features docs Improvements or additions to the documentation package: material-ui Specific to @mui/material

Comments

@joebochill
Copy link

joebochill commented Jul 10, 2024

Summary

I'm in the process of migrating themes to the new CSS Vars Theme/Provider. I have a light/dark theme which I'm combining per the migration guide into a single theme with dark/light color schemes.

As mentioned in the docs, this is causing a pretty big headache with regards to CSS rule specificity (any of the rules I set for the dark theme cannot be overridden in a component's sx prop without also specifying the color scheme selector.

I'm wondering if there is a better way that I should approach this or if there's a way that color schemes could be implemented without the extra css specificity.

Examples

This is a super simple example (not exactly what I'm doing in my theme, but shows the issue):

Let's say we have a rule in the theme for Button to change the background color:

styleOverrides: {
    root: ({ theme }) => ({
      backgroundColor: 'primary.main',
      [theme.getColorSchemeSelector('dark')]:{
          backgroundColor: 'primary.light'
      }
    }),
}

If I want to override the button background color somewhere in my project:

<Button sx={{backgroundColor: 'red'}} />

it only works for the light theme (with the old ThemeProvider approach, this would work for both).

To get it to work for the dark theme, I have to add an extra rule, which is a bit clunky:

<Button sx={(theme) => ({
    backgroundColor: 'red',
    [theme.getColorSchemeSelector('dark')]:{
        backgroundColor: 'red',
    }
})} />

Motivation

Essentially, I just want it to be easy to override theme styles for a component via sx without having to write a rule for both/all color schemes that I may have.

Search keywords: CssVars Theme getColorSchemeSelector specificity override sx

@joebochill joebochill added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 10, 2024
@aarongarciah aarongarciah added the customization: theme Centered around the theming features label Jul 10, 2024
@ZeeshanTamboli ZeeshanTamboli changed the title Lower CSS Specificity for color scheme rules in CSS Vars Theme [material-ui][theme] Lower CSS Specificity for color scheme rules in CSS Vars Theme Jul 13, 2024
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Jul 13, 2024

A reproduction with v5: https://stackblitz.com/edit/vitejs-vite-ycxrk9?file=src%2FApp.jsx&terminal=dev.

It also gets overriden in v6 with new theme.applyStyles in dark mode: https://stackblitz.com/edit/vitejs-vite-3zw18f?file=src%2FApp.jsx,package.json&terminal=dev due to the attribute selector.

@siriwatknp
Copy link
Member

siriwatknp commented Jul 16, 2024

#42839 will fix this issue. The theme.applyStyles('dark', …styles) will use *:where() & that does not increase CSS specificity.

My mistake, as @ZeeshanTamboli pointed out that the sx prop is still overridden in v6. In this case, I don't think we fix this because it's how Emotion generate styles.

A workaround would be to use :where(*) in sx prop:

<Button
  variant="contained"
  sx={{
    '&:where(*)': {
      backgroundColor: 'red',
    }
  }}
>
  Hello
</Button>

This does not increase CSS specificity (it's still (0, 1, )) but just to ensure that the order to the stylesheet is not in the component styles which will override the theme as expected.

@siriwatknp siriwatknp added package: material-ui Specific to @mui/material and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 16, 2024
@siriwatknp
Copy link
Member

siriwatknp commented Jul 16, 2024

However, I don't think that this is a bug but something that we need to document because in some case, the current behavior is useful so that the styles in dark mode is not impacted.

For example,

<Button
  variant="contained"
  sx={{
    color: 'black',
  }}
>
  Hello
</Button>

In dark mode, the color will not be black which is expected. Normally the color between light and dark are different, so it's normal to specify the selector.

@siriwatknp siriwatknp added the docs Improvements or additions to the documentation label Jul 16, 2024
@joebochill
Copy link
Author

Yes, I think you're probably right — in most cases, someone would need to set the color overrides (via sx) differently for light/dark theme anyway (and maybe having the styles break is a good indicator that they need to do that). The only time when that might be problematic is if, for example, a project only supports a single (non-default) color scheme — e.g., it's a dark mode only application and they have to specify the dark color mode in every style override they want to do via sx.

@joebochill
Copy link
Author

joebochill commented Jul 24, 2024

Another case where this is a bit annoying is if you're trying to override a color via sx and you want to use a theme color (not a arbitrary hex color as in the example above).

E.g.,

<Button sx={{
    backgroundColor: 'primary.light',
}} />

When using values from the theme, I would expect a single rule to handle dark/light theme differences (that's the point of the theme).

You'd have to specify the same rule twice (once with a dark mode selector) or use the where(*) selector mentioned above. Both feel a little icky in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customization: theme Centered around the theming features docs Improvements or additions to the documentation package: material-ui Specific to @mui/material
Projects
None yet
Development

No branches or pull requests

4 participants