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

[Typography] Cannot customize primary/error color #34290

Open
2 tasks done
emlai opened this issue Sep 12, 2022 · 8 comments 路 May be fixed by #34444
Open
2 tasks done

[Typography] Cannot customize primary/error color #34290

emlai opened this issue Sep 12, 2022 · 8 comments 路 May be fixed by #34444
Labels
bug 馃悰 Something doesn't work component: Typography The React component.

Comments

@emlai
Copy link
Contributor

emlai commented Sep 12, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 馃槸

<Typography color="primary"> doesn't allow customizing the color:

  • color="primary" doesn't add any classes that we could use for styling.

  • styleOverrides doesn't support the primary or colorPrimary key:

    styleOverrides: {
      primary: {
        color: palette.primary[palette.mode === "dark" ? "light" : "main"]
      },
    }
  • New component variant has no effect:

    variants: [
      {
        props: { color: "primary" },
        style: {
          color: palette.primary[palette.mode === "dark" ? "light" : "main"]
        },
      },
    ],

Same applies for color="error".

Expected behavior 馃

Typography provides some way to change the color of color="primary" into palette.primary[palette.mode === "dark" ? "light" : "main"], and same for color="error".

Steps to reproduce 馃暪

https://codesandbox.io/s/brave-spence-5wxolh?file=/demo.js

Context 馃敠

We're maintaining a UI library based on MUI, so we can't fix this at the <Typography> call site. So we can't do for example <Typography color={palette.primary[palette.mode === "dark" ? "light" : "main"]}>.

Your environment 馃寧

No response

@emlai emlai added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 12, 2022
@emlai emlai changed the title [Typography] Cannot customize colors [Typography] Cannot customize primary/error color Sep 12, 2022
@michaldudak michaldudak added the component: Typography The React component. label Sep 12, 2022
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Sep 17, 2022

@emlai The Typography component does not have it's own color prop. If you specify a color prop, it is considered as a system prop since Typography directly supports all system props because it being a CSS utility component. See docs.

So for your use case you can do:

<Typography color={`primary${[palette.mode === 'dark' ? 'light' : 'main']}]`}>
  Hello world
</Typography>;

See color system property docs: https://mui.com/system/palette/#color

@ZeeshanTamboli ZeeshanTamboli removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 17, 2022
@emlai
Copy link
Contributor Author

emlai commented Sep 17, 2022

@ZeeshanTamboli Unfortunately, as per the "context" section in the issue description, that wouldn't work for our use case, since it would require users of our library to specify that condition every time they want to use primary or error colors on Typography. The user should be able to simply specify color="primary" and have the correct color be used.

Would it be possible to fix this in a way that requires no changes from our users, such as adding classes like MuiTypography-colorPrimary based on the used system color prop, which would allow us to override it with the correct color?

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Sep 17, 2022

Would it be possible to fix this in a way that requires no changes from our users, such as adding classes like MuiTypography-colorPrimary based on the used system color prop, which would allow us to override it with the correct color?

@emlai In that case, you can override the default palette, like:

const theme = createTheme({
  palette: {
    primary: {
      main: '#085982', // your custom color
    },
    error: {
      main: '#d40808', // your custom color
    },
  },
});

And render it like this:

function App() {
  return (
    <ThemeProvider theme={theme}>
      <Typography color="primary">Hello World</Typography>
      <Typography color="error">Hello World</Typography>
    </ThemeProvider>
  );
}

@emlai
Copy link
Contributor Author

emlai commented Sep 17, 2022

@ZeeshanTamboli Our users are relying on palette.primary.main being what it is now, so we can't really change it in a backwards-compatible manner.

We have successfully customized the color of color="primary" for many MUI components such as SvgIcon, IconButton, Button, Link, FormHelperText, and FormLabel (either via styleOverrides, variants, or colorPrimary classes).

Ideally Typography would support a similar level of customization.

@ZeeshanTamboli
Copy link
Member

@emlai

Our users are relying on palette.primary.main being what it is now, so we can't really change it in a backwards-compatible manner.

Ok I think I understand what you mean. Do you mean that you cannot override the default global palette colors since your users are relying on them? You would like to change the color on component level?

We have successfully customized the color of color="primary" for many MUI components such as SvgIcon, IconButton, Button, Link, FormHelperText, and FormLabel (either via styleOverrides, variants, or colorPrimary classes).

Ideally Typography would support a similar level of customization.

The system property is actually more powerful. styleOverrides, variants, or colorPrimary classes is on the theme level, while the system property can be applied directly with more advantages.

I came up with a solution for your use case where you can apply the hex value directly on the Typography component color prop. See CodeSandbox.

I hope it helps you to customize on Typography component level. Let me know if anything else.

@emlai
Copy link
Contributor Author

emlai commented Sep 19, 2022

@ZeeshanTamboli

Do you mean that you cannot override the default global palette colors since your users are relying on them? You would like to change the color on component level?

Yes.

I came up with a solution for your use case where you can apply the hex value directly on the Typography component color prop. See CodeSandbox.

We don't re-export MUI components from our library, so our users are importing Typography directly from MUI. So adding our own Typography component wouldn't work. (Users would have to migrate to it manually.)

The system property is actually more powerful.

It doesn't seem that way to me. Or at least, it's not helping solve this problem. styleOverrides, variants, and CSS classes would solve this problem, so they seem more powerful to me.

@emlai
Copy link
Contributor Author

emlai commented Sep 19, 2022

I just remembered a fourth way to customize the style (MUI docs): accessing the ownerState from a functional style override:

  styleOverrides: {
    root: ({ ownerState }) => {
      switch (ownerState.color) {
        case "primary.main":
          return {
            color: palette.primary[palette.mode === "dark" ? "light" : "main"],
          };
        case "error.main":
          return {
            color: palette.error[palette.mode === "dark" ? "light" : "main"],
          };
      }
    },

We use this for some other components, but unfortunately it doesn't seem to work for Typography: MUI overrides the color returned from the styleOverrides with the default primary.main:

Screen Shot 2022-09-19 at 15 26 22

I don't think styleOverrides should be overridden? So this seems like a bug.

@ZeeshanTamboli
Copy link
Member

I have researched about this issue and found some related discussions about this in #32574 and #32653 (related to Link which inherits Typography). Also looked at #31857.

This indeed is a bug. styleOverrides for Typography should override system properties directly used on the Typography component as pointed out in #32653 (review).

@emlai Thanks for the detailed discussion. I'm re-opening this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 馃悰 Something doesn't work component: Typography The React component.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants