Make theme.palette.augmentColor() pure

[designjudo]

We love the theme object, but starting to see how it could be extended.

• In most cases, a primary, secondary, error and grey color palette will support most applications.
• I am having to add custom colors to the theme to cover a warning situation.

Rather than extend the theme to custom code new palettes, why not use the power of this theme to handle warning color just like error?

"warning": {
      "light": "#",
      "main": "#",
      "dark": "#",
      "contrastText": "#fff"
    }

Also, I would like to see warning and error as color props for the Button component:

<Button color='primary' variant='raised'>Text</Button>
<Button color='secondary' variant='raised'>Text</Button>
<Button color='error' variant='raised'>Text</Button>
<Button color='warning' variant='raised'>Text</Button>

This will greatly reduce the amount of code developers are having to write for more use cases for buttons, and use the power of the theme to consistently style applications.

Great work!!

[oliviertassinari]

@designjudo Here is how we handle the problem on our product.
Material-UI is exposing the theme.palette.augmentColor function for this use case. The API isn't perfect (mutable) nor it's documented yet 🙈. So, you should be able to add a warning color into your palette like this:

import { createMuiTheme } from '@material-ui/core/styles'
import deepmerge from 'deepmerge'; // < 1kb payload overhead when lodash/merge is > 3kb.

const rawTheme = createMuiTheme()
const warning = {
  main: '#',
}
rawTheme.platte.augmentColor(warning)

const theme = deepmerge(rawTheme, {
  palette: {
    warning,
  },
})

Also, I would like to see warning and error as color props for the Button component:

For this one, we have been wrapping most of the Material-UI components to add or remove some properties.

[designjudo]

Currently we've been rewriting the theme object with an extended palette, so:

import lightTheme from './lightTheme'
import { createMuiTheme } from '@material-ui/core/styles';

const rawTheme = createMuiTheme(lightTheme)

...

// lightTheme.js //

import orange from '@material-ui/core/colors/orange';

palette: {
 warning: orange
}

So, I guess I could just call the shades when I do this?

palette: {
warning = {
  main: 'orange[500]'
 }
}

[oliviertassinari]

@designjudo I'm not sure to understand your second point. I think that I have provided guidance on the best strategy for now in #12390 (comment). Using an extended palette is fine. The only sad thing about augmentColor is that it's mutable. We will sort that out in v4.

[sveyret]

Regarding the typings of augmentColor, there is an error. The method, as shown in @oliviertassinari example may accept a single parameter, and this parameter can be a SimplePaletteColorOptions, as specified, but also a full Color. The definition should be:

    augmentColor: (
      color: PaletteColorOptions,
      mainShade?: number | string,
      lightShade?: number | string,
      darkShade?: number | string,
    ) => void;

(note the color type and the question marks in the parameters)

File: core/styles/createPalette.d.ts

Please comment if you prefer a separate issue and/or a PR.

[oliviertassinari]

@sveyret I confirm. Do you want to submit a pull request with the fix? :)

[sveyret]

OK, I'll do it in a few minutes…

[sveyret]

Here it is: #13376

[jacobweber]

It would probably be better if augmentColor was a stand-alone exported function, so you didn't have to create a dummy rawTheme object first.

[oliviertassinari]

@jacobweber I'm not sure it will be possible, the augmentColor function relies on two theme variables you can provide as options:

const theme = {
  palette: {
    // Used by `getContrastText()` to maximize the contrast between the background and
    // the text.
    contrastThreshold = 3,
    // Used by the functions below to shift a color's luminance by approximately
    // two indexes within its tonal palette.
    // E.g., shift from Red 500 to Red 300 or Red 700.
    tonalOffset = 0.2,
  },
};

[ryancogswell]

@oliviertassinari Would the scope of this change to augmentColor include eliminating side effects from calling createMuiTheme? If not, I can create a separate issue for that, but the mutations from augmentColor are the root of the problem.

Currently if I pass in an object to createMuiTheme with a palette that only defines the main value for a color, it has a side effect of adding the light, dark, and contrastText values to my original object. This code sandbox demonstrates the issue.

[oliviertassinari]

include eliminating side effects from calling createMuiTheme?

@ryancogswell What side effects do you have in mind? Oh yes, it's because of augmentColor :/.
A pull request is welcomed! We will merge it for Material-UI v4.0.0.

[ryancogswell]

@oliviertassinari I'll get a pull request for this sometime in the next week.