Material-ui: [Button] Remove default color, use primary by default

[wysird]

It'd be useful to be able to override the 'default' palette value and it actually take effect.

e.g.

const theme = createMuiTheme({
    palette: {
        ...
        default: {
            dark: '#000000',
            light: '#111111',
            main: '#100008',
            contrastText: '#ffffff',
        },
        ...
    },
    typography: {
        useNextVariants: true
    },
});

At the moment some components (like Button) use specific colours (like theme.palette.grey[300]) instead of theme.palette.default. Switching this round would allow us to make use of the default them in the palette.

e.g.

diff --git a/packages/material-ui/src/Button/Button.js b/packages/material-ui/src/Button/Button.js
index 4e1304ff1..cbfef3994 100644
--- a/packages/material-ui/src/Button/Button.js
+++ b/packages/material-ui/src/Button/Button.js
@@ -107,8 +107,8 @@ export const styles = theme => ({
   },
   /* Styles applied to the root element if `variant="contained"`. */
   contained: {
-    color: theme.palette.getContrastText(theme.palette.grey[300]),
-    backgroundColor: theme.palette.grey[300],
+    color: theme.palette.default.contrastText,
+    backgroundColor: theme.palette.default.main,
     boxShadow: theme.shadows[2],
     '&$focusVisible': {
       boxShadow: theme.shadows[6],
@@ -122,10 +122,10 @@ export const styles = theme => ({
       backgroundColor: theme.palette.action.disabledBackground,
     },
     '&:hover': {
-      backgroundColor: theme.palette.grey.A100,
+      backgroundColor: theme.palette.default.dark,
       // Reset on touch devices, it doesn't add specificity
       '@media (hover: none)': {
-        backgroundColor: theme.palette.grey[300],
+        backgroundColor: theme.palette.default.main,
       },
       '&$disabled': {
         backgroundColor: theme.palette.action.disabledBackground,

[mbrookes]

The reason that the components use grey from the theme, rather than from core/colors (and the reason grey is in the theme) is for just this reason - so that you can override it.

Perhaps grey isn't the best name (what if you want to turn grey into pink?) but it at least describes the default.

[wysird]

I understand that you can 'override' the default value, however as 'default' is part of the Enum-type definitions, I thought it might be more useful to instead define it in the palette.

[mbrookes]

It is already defined in the palette – theme.palette.grey.

[wysird]

I see, so you suggest that I remap grey to be the "default", replacing 'default' with 'grey' in the example I gave?

[mbrookes]

I'm suggesting that if you want to redefine the color of components that use grey from the theme, you can.

[CoericK]

+1 Im also facing the same issue, @wysird, I love this library, but for instance, I have a simple UI where i want to display a simple button but using theme colors:

<Button variant="contained">Button contained</Button>

To look like this:

In order to it would be nice to have something like :

const theme = createMuiTheme({
  palette: {
    default: {
      main: blue[500],
      light: blue[500],
      dark: blue[500],
      contrastText: "#ffffff",
    },
  },
});

But instead we have to do this:

<Button variant="contained" color="primary">Button contained primary</Button>

```js
const theme = createMuiTheme({
palette: {
primary: {
main: blue[500],
light: blue[500],
dark: blue[500],
contrastText: "#ffffff",
},
},
});

And finally if you want to override the default button you would need to override with:

```js
{
overrides: {
 MuiButton:{
contained:{
//...do override of colors and background here.
}
}
}
}

[CoericK]

@mbrookes how did you workaround this?

[wysird]

@CoericK so according to @mbrookes you're supposed to do something like this:

const theme = createMuiTheme({
  palette: {
    grey: {
      main: blue[500],
      light: blue[500],
      dark: blue[500],
      contrastText: "#ffffff",
    },
  },
});

[wysird]

@CoericK I do totally agree though, I feel like overriding the MuiButtons (and _everything_ else) is not really right, given that you refer to theme.palette.primary and theme.palette.secondary (as you pass in the value to the color="primary"/color="secondary", respectively).

For this reason, it would still make sense to match color="default" with:

palette: {
  default: {
    main: blue[500],
    light: blue[500],
    dark: blue[500],
    contrastText: '#ffffff',
  },
},

Either that, or if (as I suggested) the name provided to color changes from color="default" to color="grey", to match up with:

palette: {
    grey: {
      main: blue[500],
      light: blue[500],
      dark: blue[500],
      contrastText: "#ffffff",
  },
},

Thoughts, @mbrookes?

[oliviertassinari]

What about we drop this default color in v5?

diff --git a/packages/material-ui/src/Button/Button.js b/packages/material-ui/src/Button/Button.js
index 7a780777a..bf1c7c19c 100644
--- a/packages/material-ui/src/Button/Button.js
+++ b/packages/material-ui/src/Button/Button.js
@@ -264,7 +264,7 @@ const Button = React.forwardRef(function Button(props, ref) {
     children,
     classes,
     className,
-    color = 'default',
+    color = 'primary',
     component = 'button',
     disabled = false,
     disableElevation = false,
@@ -346,7 +346,7 @@ Button.propTypes = {
   /**
    * The color of the component. It supports those theme colors that make sense for this component.
    */
-  color: PropTypes.oneOf(['default', 'inherit', 'primary', 'secondary']),
+  color: PropTypes.oneOf(['inherit', 'primary', 'secondary']),
   /**
    * The component used for the root node.
    * Either a string to use a HTML element or a component.

Pros:

• The valuedefault can feel inconsistent: #13028 by @eps1lon. This issue.
• I think that it was added for an older version of the specification. There is no more mention to is anymore. Also, having looked at more design systems, it seems that we are an exception.
• Do people even use this default value (without overrides)? Personally, I have avoided as much as possible, it looks strange.

[wysird]

What about we drop this default color in v5?

diff --git a/packages/material-ui/src/Button/Button.js b/packages/material-ui/src/Button/Button.js
index 7a780777a..bf1c7c19c 100644
--- a/packages/material-ui/src/Button/Button.js
+++ b/packages/material-ui/src/Button/Button.js
@@ -264,7 +264,7 @@ const Button = React.forwardRef(function Button(props, ref) {
     children,
     classes,
     className,
-    color = 'default',
+    color = 'primary',
     component = 'button',
     disabled = false,
     disableElevation = false,
@@ -346,7 +346,7 @@ Button.propTypes = {
   /**
    * The color of the component. It supports those theme colors that make sense for this component.
    */
-  color: PropTypes.oneOf(['default', 'inherit', 'primary', 'secondary']),
+  color: PropTypes.oneOf(['inherit', 'primary', 'secondary']),
   /**
    * The component used for the root node.
    * Either a string to use a HTML element or a component.

Pros:

• The valuedefault can feel inconsistent: #13028 by @eps1lon. This issue.
• I think that it was added for an older version of the specification. There is no more mention to is anymore. Also, having looked at more design systems, it seems that we are an exception.
• Do people even use this default value (without overrides)? Personally, I have avoided as much as possible, it looks strange.

I think inheriting from above is probably a better idea; the real problem is when trying to style the colours to work with your new backgrounds and the default just doesn't change; it sticks with whatever it has. I'm totally behind just removing the default as a compromise to needing to override it.

[wysird]

Actually as an extension to what you suggested @oliviertassinari, it might be worth changing all 'root' styles (e.g. MuiButton-root, FormControl-root, etc) for the color variable to be inherit, as this is usually a better solution (meaning you can set the color at the top level in a single place, rather than overriding literally every MUI element.

[oliviertassinari]

@wysird I fear the full support of color="inherit" with CSS inheritance is not possible. How would you style the hover, focused, etc styles?

[wysird]

Sorry, I don't mean to use color="inherit" I mean the default styling provided by the theme provider should be setting the CSS style of color to inherit. At the moment there are values like: color: rgba(0, 0, 0, 0.54); which means that if you change (for instance) the background color to a dark color, anything that has this value will have to be overridden. So you overrides ends up looking like this:

{
  ...
  overrides: {
    MuiButton: {
      color: 'inherit',
    },
    MuiSvgIcon: {
      color: 'inherit',
    },
    MuiPaper: {
      color: 'inherit',
    },
  },
  ...
}

etc.

I'm suggesting that the default for all these is 'inherit', meaning there should not really be any reason to override, and makeStyles is utilised in situations that a change is required.

Does this make sense, or am I muddying the water?

[oliviertassinari]

@wysird The color CSS property inherits by default. We override it, sometimes, to use the values from the theme; it doesn't seem there is any change required here;

[wysird]

Oh yeah, my bad, I see what you mean.