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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cant create variant for SVG icon #34827

Open
2 tasks done
AdamZajler opened this issue Oct 19, 2022 · 8 comments
Open
2 tasks done

Cant create variant for SVG icon #34827

AdamZajler opened this issue Oct 19, 2022 · 8 comments
Assignees
Labels
customization: theme Centered around the theming features package: system Specific to @mui/system

Comments

@AdamZajler
Copy link

AdamZajler commented Oct 19, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 馃暪

Use WebStorm & try to create custom color variant for SvgIcon like default and it is #434A54

export const muiTheme = createTheme({
    palette: {
      default: {
        main: '#434A54',
      },
    },
    components: {
      MuiIcon: {
        variants: [
          {
            props: {
              // TODO
              // color: 'default',
            },
            style: {
              color: theme.black,
            },
          },
        ],
      },
    },
    plPL,
  },
);

declare module '@mui/material/styles' {
  interface Palette {
    default: Palette['primary'];
  }

  interface PaletteOptions {
    default: PaletteOptions['primary'];
  }
}

declare module '@mui/material/Icon' {
  interface IconPropsColorOverrides {
    default: true;
  }
}

declare module '@mui/material/SvgIcon' {
  interface SvgIconPropsColorOverrides {
    default: true;
  }
}

Current behavior 馃槸

Cant add color named default to MuiSvgIcon; & I have no tips in component or anything else (only default colors are avilable).

Expected behavior 馃

There should not be any errors in theme & custom color should be visible in tips

Context 馃敠

My friend dosent have such issue on VSCODE only I have it in webstorm :|

Your environment 馃寧

No response

@AdamZajler AdamZajler added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 19, 2022
@zannager zannager added the package: icons Specific to @mui/icons label Oct 19, 2022
@siriwatknp
Copy link
Member

siriwatknp commented Oct 21, 2022

There should not be any errors in theme & custom color should be visible in tips

Can you provide a CodeSandbox that reproduces the error?

@siriwatknp siriwatknp added the customization: theme Centered around the theming features label Oct 21, 2022
@siriwatknp siriwatknp self-assigned this Oct 21, 2022
@siriwatknp siriwatknp added status: waiting for author Issue with insufficient information package: system Specific to @mui/system and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer package: icons Specific to @mui/icons labels Oct 21, 2022
@rajithaw
Copy link

As you can see from this code sandbox custom color variants work on other components but not on Icon component.

https://stackblitz.com/edit/react-zws3or?file=demo.tsx

@github-actions
Copy link

Since the issue is missing key information and has been inactive for 7 days, it has been automatically closed. If you wish to see the issue reopened, please provide the missing information.

@oliviertassinari
Copy link
Member

@rajithaw The code says that MUI will remove the color prop for Icon and SvgIcon components in v6.

// TODO v5 deprecate, v6 remove for sx
color:

So instead, this would works:

-<Icon color="customColor">home</Icon>
+<Icon sx={{ color: 'customColor.main' }}>home</Icon>

https://stackblitz.com/edit/react-zws3or-lndpfm?file=demo.tsx

We also have #19466 about being able to add new props, e.g. the color prop with the theme. I assume this is what would apply for @AdamZajler once we remove the color prop.

@siriwatknp Would this fly with Joy UI? If not, then I would be in favor of aligning Material UI on Joy UI

@oliviertassinari oliviertassinari removed the status: waiting for author Issue with insufficient information label Oct 29, 2022
@siriwatknp
Copy link
Member

siriwatknp commented Nov 7, 2022

@siriwatknp Would this fly with Joy UI? If not, then I would be in favor of aligning Material UI on Joy UI

I would not remove the color prop in Joy UI because it refers to the theme palette as other components do:

<SvgIcon color="primary" />;

// You can't do this
<SvgIcon color="primary.main" />;

However, happy to discuss further to bring consistency to both Material UI and Joy UI.

Ref: we had a discussion about the color prop name in #32141 (comment)

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 7, 2022

in Joy UI because it refers to the theme palette as other components do:

@siriwatknp Why does an icon component needs the notion of a theme palette color? I mean, why is the general sx prop API not enough?

@siriwatknp
Copy link
Member

The sx prop is for instance (local) customization. Having the color prop allows you to create your own theme. For example, in ios theme the color is likely to be the primary color (system blue):

createTheme({ // or extendTheme
  components: {
    SvgIcon: {
      defaultProps: {
        color: 'primary',
      }
    }
  }
})

In some themes, you might want to have the default color, or neutral color (in Joy UI) which you can't do it with sx prop.

// let's not talk about this case because it won't scale.
createTheme({ // or extendTheme
  components: {
    SvgIcon: {
      defaultProps: {
        sx: {
          color: '...',
        }
      }
    }
  }
})

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 7, 2022

which you can't do it with sx prop.

@siriwatknp I was thinking of the sx prop for one-off customizations. For theming, I think that styleOverrides is meant to cover the use case. But even for iOS, there might be too many cases where icons' color needs to be color: inherit that setting blue as a default icon color wouldn't work.

What I fail to understand is why an icon needs special handling for colors, something that justifies that using the normal CSS color customization API is not enough, meaning needing a dedicated color, palette, or colorText prop. For the button, I get why it's needed, it's because there is a lot more than adding one color CSS property.

I think that the initial idea to add the color prop in Material UI around v1 was for API consistency, but we since saw that it can create surprises of behavior #32141 (comment) so confusion and that it creates different ways to do the same thing, so complexity. This makes me wonder: Is it better or worse with the prop?

you might want to have the default color,

I think that removed default as a valid color value in v5, per #13028.

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 package: system Specific to @mui/system
Projects
None yet
Development

No branches or pull requests

5 participants