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

getColorSchemeSelector returns the wrong selector when multiple are present #43046

Closed
joebochill opened this issue Jul 23, 2024 · 4 comments
Closed
Assignees
Labels
customization: css Design CSS customizability customization: theme Centered around the theming features package: system Specific to @mui/system support: question Community support but can be turned into an improvement

Comments

@joebochill
Copy link

joebochill commented Jul 23, 2024

Steps to reproduce

Link to live example: https://codesandbox.io/s/brave-euler-y5ghwt?file=/src/Demo.tsx

Steps:

  1. Use the new CssVarsProvider
  2. Create two wrapper elements to force the theme mode (with data-mui-color-scheme)
  3. Style a component using the theme selector
<div data-mui-color-scheme={"dark"}>
  <div data-mui-color-scheme={"light"}>
    <Box
      sx={(theme) => ({
        height: 100,
        width: 100,
        backgroundColor: "primary.main",
        [theme.getColorSchemeSelector("dark")]: {
          backgroundColor: "error.main"
        }
      })}
    />
  </div>
</div>

Current behavior

The dark mode styles are being applied (because there is a dark mode ancestor in the tree) even though the nearest parent element is setting light mode.

Expected behavior

The component should be styled using the light theme rules/values since the nearest parent element specifies light theme.

Context

I'm noticing this in a storybook project where I am trying to show the dark mode styles and light mode styles for a component side by side.

Your environment

npx @mui/envinfo

Using Chrome

  System:
    OS: macOS 14.5
  Binaries:
    Node: 20.11.0 - /usr/local/bin/node
    npm: 10.2.4 - /usr/local/bin/npm
    pnpm: 9.0.5 - /opt/homebrew/bin/pnpm
  Browsers:
    Chrome: 126.0.6478.183
    Edge: Not Found
    Safari: 17.5

Search keywords: CssVars Theme getColorSchemeSelector

@joebochill joebochill added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 23, 2024
@zannager zannager added package: system Specific to @mui/system customization: theme Centered around the theming features customization: css Design CSS customizability labels Jul 24, 2024
@joebochill
Copy link
Author

useColorScheme() also does not return the expected mode. Is the appropriate way to do this to use a second CssVarsProvider instead of just using the element attribute on a div?

@siriwatknp
Copy link
Member

This is expected from the styles that you use:

<Box
  sx={(theme) => ({
    height: 100,
    width: 100,
    backgroundColor: "primary.main",
    [theme.getColorSchemeSelector("dark")]: { // << this has higher specificity
      backgroundColor: "error.main"
    }
  })}
/>

I'm noticing this in a storybook project where I am trying to show the dark mode styles and light mode styles for a component side by side.

To achieve this, I suggest attaching light and dark element in the same level:

<div data-mui-color-scheme="light">
  …components
</div>
<div data-mui-color-scheme="dark">
  …components
</div>

@siriwatknp siriwatknp added support: question Community support but can be turned into an improvement and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 21, 2024
@joebochill
Copy link
Author

Thanks for the reply @siriwatknp — I understand what you are saying (and having a bunch of nested containers is probably an edge case that doesn't warrant much effort).

To achieve this, I suggest attaching light and dark element in the same level:

This should work for my particular case, but perhaps not for a more general case where someone needs to nest themes. I think maybe this could be explained better in the documentation. It appears that there is not a way to actually nest themes like there was before with the ThemeProvider.

The docs say by using the data-mui-color-scheme attribute, you can force a particular element/children to appear as if they are in a different mode, but they aren't actually in that mode: https://mui.com/material-ui/experimental-api/css-theme-variables/customization/#force-a-specific-color-scheme

You can see that in this demo as well (Buttons are styled differently because of the attribute, but both report being in the same mode): https://codesandbox.io/p/sandbox/withered-paper-7dxkzh?file=%2Fsrc%2FDemo.tsx%3A8%2C1&workspaceId=e74d666b-0caf-4132-8ad3-e66da26e8e7e

If it's true that you can't actually nest themes, then I think it would help to be more explicit about that in the documentation.

@joebochill
Copy link
Author

The issue with the mode can also be seen in MUI components that use theme.palette.mode (e.g., Breadcrumbs). If you use the breadcrumbs in a container with data-mui-color-scheme="dark" they still use colors from the light mode (because the mode is not actually changed in this container).

Example: https://codesandbox.io/p/sandbox/sharp-waterfall-kx4wtw?file=%2Fsrc%2FDemo.tsx
image

The Breadcrumbs should only be using the grey[100] color if the mode is light, but you can see in the demo it's used for both light and dark containers.

https://github.com/mui/material-ui/blob/next/packages/mui-material/src/Breadcrumbs/BreadcrumbCollapsed.js#L15
image

The Breadcrumbs are just one example, but I've seen a number of other affected components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customization: css Design CSS customizability customization: theme Centered around the theming features package: system Specific to @mui/system support: question Community support but can be turned into an improvement
Projects
None yet
Development

No branches or pull requests

3 participants