mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
94.03k stars 32.3k forks source link

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

Open maxdacruz opened 1 year ago

maxdacruz commented 1 year ago

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. ```
mj12albert commented 1 year ago

@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 commented 1 year ago

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 ~

DiegoAndai commented 1 year ago

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.

Sboonny commented 11 months ago

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 commented 11 months ago

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.