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

[theme] Allow custom color variants #13875

Closed
2 tasks done
carlgunderson opened this issue Dec 11, 2018 · 72 comments · Fixed by #25934
Closed
2 tasks done

[theme] Allow custom color variants #13875

carlgunderson opened this issue Dec 11, 2018 · 72 comments · Fixed by #25934
Labels
new feature New feature or request priority: important This change can make a difference ready to take Help wanted. Guidance available. There is a high chance the change will be accepted v5.x
Milestone

Comments

@carlgunderson
Copy link

  • This is not a v0.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

The ability to have success in the color palette for Button, or any component w/ a color prop.

Current Behavior 😯

Only error palette option is available.

Context 🔦

Similar to the need for error styling, a success color palette option for any component that has a color prop would be equally valuable. The current functionality to get a green button option, for instance, requires lots of custom styling on the Button component which isn't ideal. If error is warranted in the palette, why not success as well?

Another idea is for some kind of dynamic mapping, so if you created a manual key of success and passed it in as a color prop to a Button, the button would just try to find the theme override with that key in the palette.

Thanks!

@carlgunderson carlgunderson changed the title [Button] Allow for success color, either through props or theme [theme] Allow for success color palette option Dec 11, 2018
@carlgunderson carlgunderson changed the title [theme] Allow for success color palette option [theme] Allow for success color palette option Dec 11, 2018
@oliviertassinari

This comment has been minimized.

@oliviertassinari oliviertassinari added discussion priority: important This change can make a difference labels Dec 12, 2018
@mbrookes

This comment has been minimized.

@carlgunderson

This comment has been minimized.

@oliviertassinari

This comment has been minimized.

@katerlouis
Copy link

what's the status here?
I'd also like to do the following

<Button color="customPaletteColor">Awesome!</Button>

@oliviertassinari
Copy link
Member

@katerlouis It works but you will get a prop-type warning. The next step is to, somehow, loosen the prop-types from a static check to a dynamic check.

@katerlouis
Copy link

katerlouis commented Aug 9, 2019

this doesn't work for me:

// colors.js
export const blue = {
  main: "#00CDE8",
}

export const red = {
  main: "#FF495A",
  light: "#F76E7B",
}

export const purple = {
  main: "#5D2BFF",
}
...
// theme.js
...
palette: {
    primary: blue,
    secondary: purple,
    buy: blue,
    sell: red,
},
...
// SomeComponent.jsx
...
<Button color="sell">This is not red :'(</Button>

What am I doing wrong?

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 9, 2019

@katerlouis Ok, my bad, the current style structure doesn't allow it. We would need to perform the following diff:

diff --git a/packages/material-ui/src/Button/Button.js b/packages/material-ui/src/Button/Button.js
index 958cb4d74..ad737d575 100644
--- a/packages/material-ui/src/Button/Button.js
+++ b/packages/material-ui/src/Button/Button.js
@@ -199,34 +199,20 @@ const Button = React.forwardRef(function Button(props, ref) {
     ...other
   } = props;

-  const text = variant === 'text';
-  const outlined = variant === 'outlined';
-  const contained = variant === 'contained';
-  const primary = color === 'primary';
-  const secondary = color === 'secondary';
-  const className = clsx(
-    classes.root,
-    {
-      [classes.text]: text,
-      [classes.textPrimary]: text && primary,
-      [classes.textSecondary]: text && secondary,
-      [classes.outlined]: outlined,
-      [classes.outlinedPrimary]: outlined && primary,
-      [classes.outlinedSecondary]: outlined && secondary,
-      [classes.contained]: contained,
-      [classes.containedPrimary]: contained && primary,
-      [classes.containedSecondary]: contained && secondary,
-      [classes[`size${capitalize(size)}`]]: size !== 'medium',
-      [classes.disabled]: disabled,
-      [classes.fullWidth]: fullWidth,
-      [classes.colorInherit]: color === 'inherit',
-    },
-    classNameProp,
-  );
-
   return (
     <ButtonBase
-      className={className}
+      className={clsx(
+        classes.root,
+        classes[variant],
+        {
+          [classes[`size${capitalize(size)}`]]: size !== 'medium',
+          [classes.disabled]: disabled,
+          [classes.fullWidth]: fullWidth,
+          [classes[`${variant}${capitalize(color)}`]]: color !== 'inherit',
+          [classes.colorInherit]: color === 'inherit',
+        },
+        classNameProp,
+      )}
       component={component}
       disabled={disabled}
       focusRipple={!disableFocusRipple}

From a bundle size perspective, it's already something worth doing :). Is this something you would like to contribute?

Then, you could do:

import React from 'react';
import { createMuiTheme } from '@material-ui/core/styles';
import { ThemeProvider } from '@material-ui/styles';
import Button from '@material-ui/core/Button';

const theme = createMuiTheme({
  overrides: {
    // Style sheet name ⚛️
    MuiButton: {
      // Name of the rule
      textSell: {
        color: 'red',
      },
    },
  },
});

function OverridesCss() {
  return (
    <ThemeProvider theme={theme}>
      <Button color="sell">Overrides CSS</Button>
    </ThemeProvider>
  );
}

export default OverridesCss;

We would need to move #6115 forward or #16180 first to support theme.palette.x directly.

@JCMais
Copy link

JCMais commented May 16, 2021

If anyone is trying to use a new color palette with a button in v4, here is one way to do it:

  1. Do the changes mentioned by @oliviertassinari above about augmenting @material-ui/core/styles (this works in v4)
  2. Create a new Button component like the following one:
import * as React from "react";
import clsx from "clsx";
import { ButtonProps, Button as MuiButton, withStyles, fade, capitalize } from "@material-ui/core";

// this is the material-ui Theme being used
import { Theme } from "../path-to-theme/theme";

const EXTRA_COLORS = ["danger"];

export const styles = (theme: Theme) => ({
  /* Pseudo-class applied to the root element if `disabled={true}`. */
  disabled: {},
  /* Styles applied to the root element if `variant="text"` and `color="danger"`. */
  textDanger: {
    color: theme.palette.danger.main,
    "&:hover": {
      backgroundColor: fade(theme.palette.danger.main, theme.palette.action.hoverOpacity),
      // Reset on touch devices, it doesn't add specificity
      "@media (hover: none)": {
        backgroundColor: "transparent",
      },
    },
  },
  /* Styles applied to the root element if `variant="outlined"` and `color="danger"`. */
  outlinedDanger: {
    color: theme.palette.danger.main,
    border: `1px solid ${fade(theme.palette.danger.main, 0.5)}`,
    "&:hover": {
      border: `1px solid ${theme.palette.danger.main}`,
      backgroundColor: fade(theme.palette.danger.main, theme.palette.action.hoverOpacity),
      // Reset on touch devices, it doesn't add specificity
      "@media (hover: none)": {
        backgroundColor: "transparent",
      },
    },
    "&$disabled": {
      border: `1px solid ${theme.palette.action.disabled}`,
    },
  },
  /* Styles applied to the root element if `variant="contained"` and `color="danger"`. */
  containedDanger: {
    color: theme.palette.danger.contrastText,
    backgroundColor: theme.palette.danger.main,
    "&:hover": {
      backgroundColor: theme.palette.danger.dark,
      // Reset on touch devices, it doesn't add specificity
      "@media (hover: none)": {
        backgroundColor: theme.palette.danger.main,
      },
    },
  },
});

const _Button = React.forwardRef(function Button(
  props: Omit<ButtonProps, "color"> & { color: ButtonProps["color"] | "danger" },
  ref
) {
  const { classes, disabled, className, color = "default", variant = "text", ...otherProps } = props;

  return (
    // @ts-ignore
    <MuiButton
      className={
        EXTRA_COLORS.includes(color)
          ? clsx(
              {
                // @ts-ignore
                [classes.disabled]: disabled,
                // @ts-ignore
                [classes[`${variant}${capitalize(color)}`]]: color !== "default" && color !== "inherit",
              },
              className
            )
          : className
      }
      disabled={disabled}
      ref={ref}
      // @ts-ignore
      color={EXTRA_COLORS.includes(color) ? undefined : color}
      variant={variant}
      {...otherProps}
    />
  );
});

// @ts-ignore
export const Button = withStyles(styles, { name: "ExtendedMuiButton" })(_Button);

The above example is for adding support to a danger color palette, notice how we have to create 3 styles, one for each button variant:

  • textDanger
  • outlinedDanger
  • containedDanger

@dstoyanoff
Copy link

Hey guys,
What about Typography? It seems we cannot pass custom palette colors as it is with the other components :(
Is there a workaround?

@oliviertassinari
Copy link
Member

@dstoyanoff Are you using v5? Do you have a reproduction?

@dstoyanoff
Copy link

Hello @oliviertassinari,
Yes, this is with v5.

The issue is that the color property of the Typography is actually css color as Typography doesn't have the explicit color property. If I pass a valid CSS color, it would work, but this means I have to use useTheme to get the color.

Bacause of the above, Typography does not provide an overridable type for Typescript - e.g. TypographyPropsColorOverrides and it only has an override for Variants.

Is there a plan to include support for color on Typography?

@oliviertassinari
Copy link
Member

oliviertassinari commented May 27, 2021

@dstoyanoff What you are describing doesn't make sense to what I have conceptually for the component. Typography extends the Box which accepts any color prop ("text.secondary", "foo.bar", "red", etc.) What version are you using? Do you have a reproduction?

Capture d’écran 2021-05-27 à 16 15 42

https://next.material-ui.com/api/typography/

@dstoyanoff
Copy link

Thanks for this. I didn't know that colors work like this when nested in the theme. It's a nice one. Is there a way to make it type-safe in TypeScript? Perhaps you could leverage the new Template Literal Types...

@oliviertassinari
Copy link
Member

@dstoyanoff Please open a new issue for type-safe, it's actually an interesting topic (also to get autocomplete).

@i-tozer
Copy link

i-tozer commented Jun 11, 2021

I am having the same issue as #13875 (comment), expect for with the chip component. The button's I am able to customise thanks to @oliviertassinari's above fix.

I opened an issue here: #26695

@cjol
Copy link

cjol commented Jul 22, 2021

Our latest progress on this feature.

  1. First, developers have to extend the palette: https://next.material-ui.com/customization/palette/#adding-new-colors
import { createMuiTheme } from '@material-ui/core/styles';

const theme = createMuiTheme({
  palette: {
    neutral: {
      main: '#E2410D',
    },
  },
});

declare module '@material-ui/core/styles' {
  interface Palette {
    neutral: Palette['primary'];
  }
  interface PaletteOptions {
    neutral: PaletteOptions['primary'];
  }
}

Should this automatically add the variants for e.g. dark and light as well as just adding main? It seems that it currently doesn't in your sandbox:

        <Button color="neutral">{JSON.stringify(theme.palette.neutral)}</Button>

image

So either the type is incorrect in the documentation, or there's a bug in the implementation.

@oliviertassinari
Copy link
Member

Should this automatically add the variants for e.g. dark and light as well as just adding main?

@cjol It doesn't. You need to use theme.palette.augmentColor(). It doesn't know if the new key is meant to be a color for the components or something else. light/dark are used by some of the components, so maybe we should document it. https://next.material-ui.com/customization/palette/#adding-new-colors. Do you want to open a new issue about it?

@cjol
Copy link

cjol commented Jul 23, 2021

Makes sense, so a more accurate type would be:

type CustomColor = { main: string }
declare module '@material-ui/core/styles' {
  interface Palette {
    neutral: CustomColor
  }
  interface PaletteOptions {
    neutral: CustomColor
  }
}

So that palette.neutral.light is correctly undefined (unless you choose to define it inside your CustomColor)

@pouyarezvani
Copy link

@oliviertassinari Im probably not asking this in the right spot,
but how can I add a new custom prop to the Box element from Mui?
I was able to add custom variant to Typography like this :

declare module '@mui/material/Typography' {
	interface TypographyPropsVariantOverrides {
		bold: true;
		body3: true;
	}
}

I'd like to add a custom prop to the Box element that would be called "variant" and if variant 's value is "Bordered" then the Box element would get a custom border.

@dstoyanoff
Copy link

@pouyarezvani, you can't just add new props. Those overrides are there to allow adding more possible values to an already existing property. You have to use styled for this or create your own component to wrap this one.

@James-Wilkinson-git
Copy link

Can we get this for v4 please, its impossible to upgrade to v5 y'all changed way too much and styled-components is just not the way we will be developing, v4 needs to live forever.

@mnajdova
Copy link
Member

mnajdova commented Mar 2, 2022

Can we get this for v4 please, its impossible to upgrade to v5 y'all changed way too much and styled-components is just not the way we will be developing, v4 needs to live forever.

v5 uses emotion by default, not styled-components, and we don't plan to support v4 anymore, except for security patches if there is need for them. Using emotion instead of JSS has enabled us tp create some of these new APIs which were not possible (too slow) when using JSS (v4)

@socketpair
Copy link

So, in the end:
color="warning" is OK, it works. How to tell Typography to show "disabled" text ?

@kgregory
Copy link
Member

kgregory commented Apr 25, 2022

So, in the end: color="warning" is OK, it works. How to tell Typography to show "disabled" text ?

Typography doesn't have a color prop, but it can be done a few ways. Here is an example using the sx prop:

      <Typography sx={{ color: (theme) => theme.palette.text.disabled }}>
        This thing is disabled right now
      </Typography>

Read more about about MUI system properties

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request priority: important This change can make a difference ready to take Help wanted. Guidance available. There is a high chance the change will be accepted v5.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.