Customizing the color of a raised button is way too complex

[xaviergonz]

This is more a rant than a real issue really.

I used to have 4 colors for raised buttons: default - gray, primary - blue, secondary - green, contrast - red

Due to the recent change to the palette that removed the contrast color from I embarked into the task of making an additional custom red button and this is what I ended up with:

import Button, { ButtonClassKey, ButtonProps } from 'material-ui/Button';
import { fade } from 'material-ui/styles/colorManipulator';
import withStyles, { WithStyles } from 'material-ui/styles/withStyles';
import * as React from 'react';
import { errorColor, getContrastText, muiThemeNext } from '../../themes/muiThemeNext';

const styles = {
  raisedPrimary: {
    color: getContrastText(errorColor[600]),
    backgroundColor: errorColor[600],
    '&:hover': {
      backgroundColor: errorColor[800],
      // Reset on mouse devices
      '@media (hover: none)': {
        backgroundColor: errorColor[600],
      },
    },
  },

  flatPrimary: {
    color: errorColor.A400,
    '&:hover': {
      backgroundColor: fade(errorColor.A400, 0.12),
    },
  },
};

class DangerButtonWithStyles extends React.Component<ButtonProps & WithStyles<ButtonClassKey>> {
  render() {
    const classes = {
      ...this.props.classes,
      raisedPrimary: this.props.disabled ? undefined : this.props.classes.raisedPrimary,
      flatPrimary: this.props.disabled ? undefined : this.props.classes.flatPrimary,
    };

    return (
      <Button
        {...this.props}
        classes={classes}
        color="primary"        
      />    
    );
  }
}

export const DangerButton = withStyles(styles)(DangerButtonWithStyles) as React.ComponentType<ButtonProps>;

My first rant is about having to opt-in and out of classes depending on if the button is disabled or not.
If I don't do this then, since the generated classes have more priority than the default ones then the disabled styles are never applied.
Wouldn't it make more sense to make a deep merge of the current theme classes (including overrides) with the custom ones and then create class names based on that merge so that disabled overrides are not lost?

The second rant is about how overly complex this all seems. Couldn't there be something like this instead?

<Button
  colorOverrides={{
    label:"blue", // label color for both raised and flat buttons
    background: "red" // background color for raised and flat buttons
    hoverBackground: "yellow" // background color on hover for raised and flat buttons
    disabledLabel: "blue" // label color for both raised and flat buttons when disabled
    disabledBackground: "red" // background color when disabled for raised and flat buttons
    ripple: "tomato" // ripple color?
  }}
> hi </Button>

each of those properties would default to undefined, meaning no customization applied and therefore the API is backwards compatible

There's not so much of a problem with IconButton for example, since you can just use something like

export class DangerIconButton extends React.Component<IconButtonProps> {
  render() {
    return (
      <IconButton
        {...this.props}        
        color="inherit"
        style={{
          color: this.props.disabled ? undefined : errorColor.A400,
          ...this.props.style
        }}
      />    
    );
  }
}

still though it is funny how you have to be careful with not setting the color when disabled, so there could be something like icon, disabledIcon and ripple inside colorOverrides

And same thing about checkbox, etc.

Just my two cents 😄

[oliviertassinari]

I agree!

[mbrookes]

Couldn't there be something like this instead?

<Button
  colorOverrides={{

The reason that isn't currently possible is the very same reason the color prop is an enum to begin with: it isn't possible to modify the generated CSS classes via props.

Each of the colors you can specify exist as a predefined class (customizable via the palette). Changing the color prop changes which of these classes is applied. If we could inject prop values into the generated styles (as needed for your proposed solution),there wouldn't be any restriction on the colors that you could use, so your solution wouldn't be needed. Catch 22.

I believe JSS has some developments in this regard, so never say never.

[oliviertassinari]

When I said "I agree" I was referring to the opportunity we have to make override simpler. I wasn't backing any specific implementation. There are some areas I want to look into:

• CSS variables, maybe too early given the browser support but this has potential.
• JSS style properties, maybe we can have some specific component variables people can inject to quickly change the "key" styles (the most important style). It covers the same use cases than CSS variables but with a broader browser support and more runtime cost.
• Alternative classes composition like Bootstrap. This is when component style variables aren't enough, it's about designed the style so that people can quickly change it. For instance, relying on color inheritance from the root element down to a nested element that needs the color.

[ianschmitz]

I may have missed something, but why not wrap your custom red button component in a MuiThemeProvider, and provide it a new theme with red being the primary color. You can then just use <Button color="primary" />

[xaviergonz]

@ianschmitz I thought about that too, but I'm weary of performance.

@oliviertassinari couldn't it do something like what https://typestyle.github.io/#/core/-style- does? they basically generate a css class based on some style and with a class name which is the hash of the style. If the class is effectively the same (same attributes) then the same class is reused since the hash is the same. That basically allows you to keep using classes while using styles.

[ianschmitz]

I'd be interested to hear more of the potential performance issues. It's a recommended pattern based on the docs (i use this extensively on work project).

Here is a simplified example:

const customTheme = outerTheme => ({
  ...outerTheme,
  palette: {
    ...outerTheme.palette,
    primary: red,
  },
});

const Button = () => (
    <MuiThemeProvider theme={customTheme}>
        <Button color="primary">Custom Primary</Button>
    </MuiThemeProvider>
);

[Eurythmax]

I have a similar issue. The default color/backgroundcolor of the when focused is #304ffe.
This color comes from the classes .MuiFormLabel-focused-186 and .MuiInput-inkbar-191:after
I want to change these colors for every textfield to #ffa000.

What is the best solution for this?

Thank you

[ianschmitz]

@Eurythmax take a look at https://material-ui.com/customization/themes/#customizing-all-instances-of-a-component-type

[Eurythmax]

Thanks @ianschmitz i have made my own Textfield now.

import React from 'react';
import { MuiThemeProvider, createMuiTheme } from 'material-ui/styles';
import TextField from 'material-ui/TextField';

const theme = createMuiTheme({
	overrides: {
		MuiInput: {
			focused: {
				color: '#ffa000',
			},
			inkbar: {
				'&:after': {
					backgroundColor: '#ffa000',
				},
			},
		},
		MuiFormLabel: {
			focused: {
				color: '#ffa000',
			},
		},
	},
});

function TextFieldOverride(prop) {
	console.log(prop.fullWidth); // eslint-disable-line
	const fullWidth = (prop.fullWidth === 'true');
	const autoFocus = (prop.autoFocus === 'true');

	return (
		<MuiThemeProvider theme={theme}>
			<TextField
				autoFocus={autoFocus}
				margin={prop.margin}
				id={prop.id}
				label={prop.label}
				type={prop.type}
				fullWidth={fullWidth}
			></TextField>
		</MuiThemeProvider>
	);
}

export default TextFieldOverride;

[oliviertassinari]

I'd be interested to hear more of the potential performance issues. It's a recommended pattern based on the docs (i use this extensively on work project).

Here is a simplified example:

const customTheme = outerTheme => ({
  ...outerTheme,
  palette: {
    ...outerTheme.palette,
    primary: red,
  },
});

const Button = () => (
    <MuiThemeProvider theme={customTheme}>
        <Button color="primary">Custom Primary</Button>
    </MuiThemeProvider>
);

@ianschmitz The performance implications of this approach is deeply linked to the JSS work needed behind the scene. The main point: We cache the injected CSS with the following tuple (styles, theme).

• theme: If you provide a new theme at each render, a new CSS object will be injected. So, instead of using the theme callback property of the MuiThemeProvider, it's better to provide a static theme object. So, both for UI consistency and performance, it's better to render a bounded & limited number of theme object.
• styles: The larger the styles object is, the more work is needed. Try having as few component in the children of MuiThemeProvider

It should be fast enough for most of the use cases, but to be cautious. I will document it.

[zsolt-dev]

How come this is closed? There is still no way to change the color and this does not work:

const customTheme = outerTheme => ({
  ...outerTheme,
  palette: {
    ...outerTheme.palette,
    primary: red,
  },
});

Error: index.js:2178 Warning: Material-UI: missing color argument in fade(undefined, 0.12).

[oliviertassinari]

@joesanta555 I have added 2 examples in the docs: https://github.com/mui-org/material-ui/blob/faf1ead52970e2b1d4adafed8dc69aaa625c5763/docs/src/pages/demos/buttons/CustomizedButtons.js

[zsolt-dev]

Thank you. I read the closed issues before and could not find the 2 examples.

Finally a nice way of doing it 🎉 🎈

Thank you again for this

[robbyemmert]

This is why I think the JSS/CSS-Modules solution is way overboard. I understand why you did it, but there are a lot of cases where it adds way more overhead than it is worth. Honestly, I would love to be able to disable it altogether.

This problem is losing me and my clients money as we speak...

[klis87]

My ultimate dream would be to use css-modules with some theming solution like react-toolbox did. For modern browsers we could use CSS variables which would allow even nested theme support. Material-ui is my favourite ui framework by far, but nothing can beat css modules re performance and productivity. However, I am glad u picked JSS from css-in-js solutions, but missing css-modules anyway :) I know I can still use css-modules, but global Mui component theming must be done with JSS, plus I need to define some vars like colors etc in my css files, instead of importing those from default theme object

[klis87]

@robbyemmert why dont u like css-modules? :)

[oliviertassinari]

Honestly, I would love to be able to disable it altogether.

@robbyemmert From an architectural point of view, it should be possible to disable it and extract global CSS. But it would require a bit of effort. What's your issue? Have you seen https://material-ui.com/guides/interoperability/ or https://material-ui.com/customization/themes/#customizing-all-instances-of-a-component-type?

@klis87 We are looking into CSS variables support in #12827. We gonna try to follow the community with the most popular styling solutions.

[klis87]

@oliviertassinari that's fantastic news, can't wait for this!

[TidyIQ]

I agree that changing button colors is too complex. I want to avoid nesting themes due to performance issues so I've made this custom button component, but I'm not sure if this would have even worse performance issues. I'd love some feedback on it.

It works fine and allows me to set any color for any button variants (i.e. either default, inherit, primaryLight, primary, primaryDark, secondaryLight, secondary, secondaryDark or any html color, such as #ff00ff for either outlined, text or contained buttons). I can also override the font color of the button if needed, and the hover no longer depends on what theme.palette.[primary/secondary].dark is - it's calculated using fade or darken instead.

Here's the code:

// ./Button/useColor.ts

import { useTheme } from "@material-ui/core/styles";

const useColor: UseColor = variant => {
  const theme = useTheme();

  switch (variant) {
    case "primaryLight": {
      return theme.palette.primary.light;
    }
    case "primary": {
      return theme.palette.primary.main;
    }
    case "primaryDark": {
      return theme.palette.primary.dark;
    }
    case "secondaryLight": {
      return theme.palette.secondary.light;
    }
    case "secondary": {
      return theme.palette.secondary.main;
    }
    case "secondaryDark": {
      return theme.palette.secondary.dark;
    }
    case "default":
    case "inherit": {
      return undefined;
    }
    default: {
      return variant;
    }
  }
};

export default useColor;

interface UseColor {
  (variant?: string): string | undefined;
}

// ./Button/index.tsx

import React, { FC } from "react";
import { fade, darken } from "@material-ui/core/styles/colorManipulator";
import MuiButton, {
  ButtonProps as MuiButtonProps
} from "@material-ui/core/Button";
import { makeStyles } from "@material-ui/core/styles";
import useColor from "./useColor";

const useStyles = makeStyles(theme => ({
  textPrimary: (props: StylesProps) => {
    if (props.bgColor) {
      const color = props.fontColor || props.bgColor;
      return {
        color,
        "&:hover": {
          backgroundColor: fade(
            props.bgColor,
            theme.palette.action.hoverOpacity
          )
        }
      };
    }
    return {};
  },
  containedPrimary: (props: StylesProps) => {
    if (props.bgColor) {
      const color =
        props.fontColor || theme.palette.getContrastText(props.bgColor);
      return {
        color,
        backgroundColor: props.bgColor,
        "&:hover": {
          backgroundColor: darken(props.bgColor, 0.25),
          // Reset on touch devices, it doesn't add specificity
          "@media (hover: none)": {
            backgroundColor: "transparent"
          }
        }
      };
    }
    return {};
  },
  outlinedPrimary: (props: StylesProps) => {
    if (props.bgColor) {
      const color = props.fontColor || props.bgColor;
      return {
        color,
        border: `1px solid ${fade(color, 0.5)}`,
        "&:hover": {
          backgroundColor: fade(
            props.bgColor,
            theme.palette.action.hoverOpacity
          ),
          border: `1px solid ${color}`
        }
      };
    }
    return {};
  }
}));

const Button: FC<ButtonProps> = ({
  children,
  color = "default",
  fontColor,
  ...props
}) => {
  const { variant } = props;
  const bgColor = useColor(color);
  const classes = useStyles({ bgColor, fontColor });

  let btnClasses: Record<string, string> | undefined;
  let btnColor: MuiButtonProps["color"];
  if (color !== "default" && color !== "inherit") {
    btnColor = "primary";
    switch (variant) {
      case "contained": {
        btnClasses = { containedPrimary: classes.containedPrimary };
        break;
      }
      case "outlined": {
        btnClasses = { outlinedPrimary: classes.outlinedPrimary };
        break;
      }
      case "text":
      case undefined: {
        btnClasses = { textPrimary: classes.textPrimary };
        break;
      }
      default:
        break;
    }
  } else {
    btnColor = color;
  }

  return (
    <MuiButton {...props} classes={btnClasses} color={btnColor}>
      {children}
    </MuiButton>
  );
};

export default Button;

export interface ButtonProps extends Omit<MuiButtonProps, "color"> {
  readonly color?: string;
  readonly fontColor?: string;
}

interface StylesProps {
  readonly bgColor: string | undefined;
  readonly fontColor: string | undefined;
}

[eps1lon]

I want to avoid nesting themes due to performance issues so I've made this custom button component, but I'm not sure if this would have even worse performance issues. I'd love some feedback on it.

Do you have an example that we could profile? I'm not aware of noticeable performance implications when nesting themes.