[Theme] Inadeguate palette theming primitives

[yuchi]

Disclaimer! This is a feature-request/question hybrid since I believe there are some problems on how colors are treated in v1. If maintainers think this is not the right place to discuss this I’ll aptly move it to Stack Overflow.

Problem Description

While customizing the library with a custom theme I found the way we have to specify the primary and accent shades a little bit controversial. Let me explain this a little bit more thoroughly.

Material Design guidelines define a set of “colors” (or “hues”) beautifully crafted by the Google Design team so that each “color” has a nicely defined set of shades (50, 100 to 900). Once you choose your main starting shade inside the hue, moving up and down helps in creating a visual chromatic hierarchy. Given the length of jumps required to have enough contrast inside the hue, and to give more vibrancy to the design of apps, they suggest to start at 500 and move up and down from there.

But nothing stops you from choosing a different shade as your starting point.

50	200	500	900

In fact these screenshots have been generated by the “official” Color Tool.

The actual problem

In this library when defining the theme for the application we can specify the hue for primary or accent colors, but not the specific shade we want.

To do so we need to create a custom “color” by shifting values up or down in order to move the selected shade to the 500 (and A500) position.

And most of the time this is required for the UI to have some chromatic sense. If you look at the documentation itself you can find an occurrence of a wrongly defined color right in the «Themes» section.

In fact this approach makes it difficult for components to extract the right colors.

Actual	Expected
«Accent» is too light	«Accent» is readable

Proposal

It’s a little but late to change this part of the library but I believe something should be done about it; be it an utility to create new colors automatically based on a shade (shiftColor(greeen, 900)) or a different approach.

[oliviertassinari]

I have been maturing the reflexion about this issue. I think that it should be handled at the configuration time of the palette. We have a small section about it in the documentation.

I like that idea of exposing a palette generation tool, however, it comes with a bundle size challenge. This should be a static tool, I don't want to bloat the bundle with something outputting a small object. Also, this palette generation tool doesn't have to be scoped to Material-UI. In a way, I do think that our best solution going forward is a competitor to one of the linked projects in the documentation. This is not something I want to put my energy one, but It can be a good marketing tool, as well as a wahoo effect if we add a demo in the documentation (something more powerful than the light/dark dynamic theme switch).

[oliviertassinari]

A demo would also be the occasion to fix #2009.

[yuchi]

[…] I think that it should be handled at the configuration time of the palette […]
[…] bundle size challenge […]

This is an issue I completely underestimated.

I think we can split the problem in two then:

1. Making the creation of palettes/themes easier by creating the aforementioned tool.
2. Ensuring the output is (more) intelligently used by components.

What scares me is that when extracting a color from the theme components usually access a specific shade of a hue. But I believe this is not in spec.

As far as I understood the spec, the hues are reference material to choose colors from, it is not a framework to define the colors of an application. In flow terms:

type Shade = string; // #RRGGBB

type Hue = {
    50: Shade,
    100: Shade,
    // ...
    900: Shade,
    A100: Shade,
    A200: Shade,
    A400: Shade,
    A700: Shade
};

type PaletteColor = {
    main: Shade,
    lighter: Shade,
    darker: Shade
};

type Palette = {
    type: 'light' | 'dark',
    primary: PaletteColor,
    accent: PaletteColor
};

type Theme = {
    // ...
    palette: Palette
};

[oliviertassinari]

What scares me is that when extracting a color from the theme components usually access a specific shade of a hue. But I believe this is not in spec.

I have been noticed the opposite on the specification, for instance with the button:

Normal color: 500
Pressed color: 700

So, as far as I see the issue. It's mainly a matter of addressing point number 1. Then regarding point number 2. I can share the usage frequency on the codebase:

primary

• 100: 3 (linear progress)
• 200: 1 (docs)
• 300: 0
• 400: 0
• 500: 40
• 600: 0
• 700: 1 (button)
• 800: 1 (docs)
• 900: 0

secondary

• A100: 5
• A200: 8
• A400: 7
• A700: 2

error

• 500: 1
• A400: 3

[oliviertassinari]

Actually, looking at the source code of material-components-web. I think that we should try to simplify the situation! This would help a lot with point n°1 as we would be able to guide people toward the official color tool

[Floriferous]

Following up on #8383, I've been porting a large app to next this week and the whole color situation is hard to get right.

I was expecting something similar to what material-components-web are doing, provide my main colors, and then the library figures the rest out.

I went through the tools to create my 3 palettes but ended up not getting the colors I wanted on most of the UI elements. If you want to quickly get going and immediately have a theme corresponding to your brand it is currently pretty hard.

For example: If I want my primary buttons to be pure red and secondary/accent buttons to be pure yellow, I have to dig in the code and see that primary buttons get the 500 hue, but accent buttons get the A200 variant, as much as it respects the spec, it isn't very user-friendly.

[emanuellarini]

Well, I'm gonna share my experience (used this lib in 3 projects so far):
In the end I just wished I could set only one variable for primary, another for secondary and another for accent. The palette system seems like a bazooka to kill an ant (for small projects!)
To change the theme button, input and other component primary colors to the same variable I had to do this:

palette: {
  primary: {
      ...deepPurple,
      A400: myPrimaryColor,
      A500: myPrimaryColor,
      A800: myPrimaryColor
  },
  error: red,
},

If I didn't do this, I have to use the override system and change every color of every component...
I don't know if I'm doing it wrong, but it was the easiest solution my team came up

Sorry for bad english.

[vuhrmeister]

I also stumbled upon this issue. I used mdl long time ago, lastly I tried material-component-web.
Now I wanted to achieve the same result, but the need to define a whole palette just confused me a lot. The result is not what I expected, neither what the official color tool generates.

So I really hope this will change. I was just convinced to use material-ui until I came across this issue.

[yuchi]

@vuhrmeister, don't diss the lib just for this. The quality of the abstractions here over shines this issue, even if important.

[vuhrmeister]

@yuchi well, I don't blame the whole lib just due to that single issue. It's not by chance that I chose material-ui over the competitors ;)

[yuchi]

Sorry it sounds harsher than I expected! Didn't mean to suggest that, just wanted to minimize the gravity of this issue 😅

[leMaik]

It's a priority as well as a breaking change. I will take care of this issue.

@oliviertassinari Wouldn't it be possible to implement this in a non-breaking way? E.g. createPalette(primaryColor, secondaryColor) and then that function calculates the shades? Similar to what mdc-theme-(light|dark)-variant(…) does in material-components-web. 🤔

[oliviertassinari]

Alright, let's fix this issue. It's going to be a breaking change. But I think that it's important to simplify the situation. I have been giving a look at how Bootstrap and material-components-web palette work. We can try to copy what works best for them. Here is my proposal:

The new API

import { createMuiTheme } from 'material-ui/styles';
import indigo from 'material-ui/colors/indigo';
import pink from 'material-ui/colors/pink';
import red from 'material-ui/colors/red';
import { darken, lighten } from 'material-ui/styles/colorManipulator';

const themeDefault = createMuiTheme()

// All the following keys are optional.
// We try our best to provide a great default value.
// The following values illustrate the default logic when our users don't provide anything.
const theme = createMuiTheme({
  palette: {
    contrastThreshold: 3.1,
    tonalOffset: 0.07,
    primary: {
      // Looking at material-components-web,
      // lighten() and darken() might not be enough:
      // https://github.com/material-components/material-components-web/blob/ac46b8863c4dab9fc22c4c662dc6bd1b65dd652f/packages/mdc-theme/_functions.scss#L83
      light: lighten(indigo[500], 0.07),
      main: indigo[500],
      dark: darken(indigo[500], 0.07),
      contrastText: themeDefault.palette.getContrastText(indigo[500]),
    },
    secondary: {
      light: lighten(pink.A400, 0.07),
      main: pink.A400,
      dark: darken(pink.A400, 0.07)
      contrastText: themeDefault.palette.getContrastText(pink.A400),
    },
    error: red.A400,
  },
});

The upgrade path

const theme = createMuiTheme({
  palette: {
-   primary: indigo,
+   primary: {
+     light: indigo[200],
+     main: indigo[500],
+     dark: indigo[700],
    },
-   secondary: pink,
+   secondary: {
+     light: pink.A200,
+     main: pink.A400,
+     dark: pink.A700,
    },
-   error: red,
+   error: red.A400,
  },
});

Problems solved

• People can use https://material.io/color/ out of the box.

• But nothing stops you from choosing a different shade as your starting point. of @yuchi

• If you want to quickly get going and immediately have a theme corresponding to your brand it is currently pretty hard of @Floriferous

• Button text color does not respect the themes contrastDefaultColor Button text color does not respect the themes contrastDefaultColor #8183 of @dangerousdan

[mbrookes]

I'm liking this overall, it combines a number of the options we've discussed for various problems into a cohesive whole. Two thoughts:

1. Do we need light & dark variants? We don't use primary[200] (or [300] which I think you meant) anywhere, and only use primary[700] once (in Button).

   We do use secondary.A200 - 15 times, but this could be inferred from the main color (darken, or whatever appropriate color manipulation). With your tonalOffset suggestion, the user has some control over that, which is a great idea. 👍

2. We could potentially avoid a complete breaking change by checking the keys of the object provided, and if it looks like a color object, use the appropriate value (500 for primary, A400 for secondary, etc.)

[oliviertassinari]

Do we need light & dark variants?

@mbrookes I think so. It's not just about the Material Design specification. As far as I have looked at the SASS files of material-web-components, they almost don't use the tones. It's more about providing to users a visual chromatic hierarchy set they can use. It's very common to have designers working with different tones (based on the different projects I have been working on), often 3-5.

We could potentially avoid a complete breaking change

This breaking change should be easily handled by the users. What's the point of making the upgrade path simpler? It will cost people how upgraded bundle size.

[Floriferous]

Looks good.

It'd be great if we can also only provide the main variant of the colors, and let them be overridden by light and dark variants if provided. So the defaults would be exactly what you proposed.

[mbrookes]

@Floriferous That's the intention (or rather, that's what the code already does 😄 ).

[yuchi]

@oliviertassinari: if this covers the spec, it would be awesome.

@mbrookes: we need to ship color parsing/manipulation functions then…

[mbrookes]

@yuchi For example?

[yuchi]

I probably misinterpreted your comment. Did you mean to automatically infer light/dark colors out of main? If that's what you mean we need to have color funcs. If not… what did you mean?

[mbrookes]

Did you mean to automatically infer light/dark colors out of main?

It does, if you provide main, but omit light and/or dark.

If that's what you mean we need to have color funcs.

For example?

PS. Sorry, about the noise. Made the mistake of referencing a comment here from the PR, and as I"ve been fighting the CI gods, it's been spamming this issue.