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

[colorManipulator] Defining a css theme variable with rgb() and var() not working #37901

Open
maxdacruz opened this issue Jul 10, 2023 · 5 comments
Assignees
Labels
customization: css Design CSS customizability customization: theme Centered around the theming features enhancement This is not a bug, nor a new feature package: system Specific to @mui/system

Comments

@maxdacruz
Copy link

maxdacruz commented Jul 10, 2023

Steps to reproduce 🕹

Link to live example:
https://codesandbox.io/s/wizardly-lamarr-6cnsfz?file=/src/MainTheme.ts

import { experimental_extendTheme } from "@mui/material/styles";

export const getTheme = () =>
  experimental_extendTheme({
    colorSchemes: {
      light: {
        palette: {
          primary: {
            main: "rgb(var(--primary))"
          }
        }
      },
      dark: {
        palette: {
          primary: {
            main: "rgb(var(--primary))"
          }
        }
      }
    }
  });

If I define a css variable in the theme with rgb and var keywords, the theme applies the main color correctly but doesn't generate automatically the others like where if I define a hex color it works as intended.

--mui-palette-primary-light: rgb(NaN, NaN, NaN);
--mui-palette-primary-dark: rgb(NaN, NaN, NaN);
--mui-palette-primary-mainChannel: NaN;
--mui-palette-primary-lightChannel: NaN NaN NaN;
--mui-palette-primary-darkChannel: NaN NaN NaN;

Current behavior 😯

The button with variant outlier has no border

Expected behavior 🤔

The button border should be using the primary color defined in the theme

Context 🔦

I'm trying to use our custom colors defined as rgb separated by commas that are being used in other internal libraries with mui's new css theme variables support

Your environment 🌎

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.
@maxdacruz maxdacruz added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 10, 2023
@zannager zannager added package: system Specific to @mui/system customization: theme Centered around the theming features customization: css Design CSS customizability labels Jul 10, 2023
@mj12albert
Copy link
Member

@maxdacruz I'm not sure this is supported 🤔

I can reproduce it locally, and it looks like the main color (55, 196, 62 in your example) is only applied correctly only because "rgb(var(--primary))" is passing through all the way as a string and is able to pick up the --primary variable

@mj12albert mj12albert removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 14, 2023
@mj12albert
Copy link
Member

What's odd is that the palette seems to generating correctly even if you only give it 1 hex token (example sandbox)

We have a docs update here that says for custom colors all the tokens should be provided, the same createPalette as createTheme is involved 🤔

Maybe @DiegoAndai and @siriwatknp are more familiar with this ~

@mj12albert mj12albert assigned DiegoAndai and unassigned mj12albert Jul 14, 2023
@DiegoAndai DiegoAndai added status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it enhancement This is not a bug, nor a new feature labels Jul 17, 2023
@DiegoAndai
Copy link
Member

Hey @maxdacruz, thanks for the report!

Providing CSS variables as the main token for the light, dark, and ...Channel tokens to be created automatically is not supported. We have an error for this:

Screenshot 2023-07-17 at 15 15 57

But you managed to bypass it by providing rgb(--var-primary) instead of --var-primary 😅

In the provided example the main token is working because it's passing through "as is" (as @mj12albert mentioned), but the light, dark, and ...Channel tokens require color manipulation and at this moment we are not able to extract the actual value of the CSS variable to perform such manipulation.

Workaround

If you provide the colors as rgb(55, 196, 62) (or any value you need), it should work as expected. Here's an example of this: https://codesandbox.io/s/weathered-http-2fr443-shared-2fr443

Hopefully, that's a workaround that works for you and your team 😊 Otherwise, or if you have any more questions, let me know

Enhancement (Ready to take)

We should improve the pattern checking to catch cases like this in this error. I'll mark this enhancement as ready to take.

@DiegoAndai DiegoAndai added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Jul 17, 2023
@Sboonny
Copy link

Sboonny commented Dec 15, 2023

changing the conditional statement to check if the color includes var should do the trick, something like if (color.includes('var')) I hope. can I try tackling this issue, if no one is working at solving it already?

@siriwatknp
Copy link
Member

In the near future, I believe that we will replace all of the JS color manipulation with color-mix() which will solve all the issues related to colors.

@DiegoAndai DiegoAndai removed the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Dec 20, 2023
@DiegoAndai DiegoAndai added this to the Material UI: v7 draft milestone Dec 20, 2023
@oliviertassinari oliviertassinari changed the title Defining a css theme variable with rgb() and var() not working [colorManipulator] Defining a css theme variable with rgb() and var() not working Aug 18, 2024
@oliviertassinari oliviertassinari removed the status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it label Aug 18, 2024
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 enhancement This is not a bug, nor a new feature package: system Specific to @mui/system
Projects
None yet
Development

No branches or pull requests

7 participants