Skip to content

Customizing the color of a raised button is way too complex #10010

@xaviergonz

Description

@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 😄

Metadata

Metadata

Assignees

No one assigned

    Labels

    breaking changeIntroduces changes that are not backward compatible.scope: buttonChanges related to the button.

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions