Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Theme] support for 'on' colors? #17241

Closed
1 task done
petegivens opened this issue Aug 30, 2019 · 12 comments
Closed
1 task done

[Theme] support for 'on' colors? #17241

petegivens opened this issue Aug 30, 2019 · 12 comments
Labels
design: material This is about Material Design, please involve a visual or UX designer in the process discussion

Comments

@petegivens
Copy link
Contributor

petegivens commented Aug 30, 2019

The Material Design docs advocate for theme palette values for "on" colors like onPrimary, onSurface, etc. Are there plans to support these in the theme.palette object?

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

I'm just looking to understand the philosophy and if this is on the roadmap. I'm working on a white label application that needs to support a robust theming solution. We have embraced the (newer) MD theme model where svg strokes and typography are colored based on these "on" colors. If text appears on top of a primary colored surface, it uses the onPrimary color.

We have also extended this to the color prop on small components. E.G. our Button can take color="primary" or color="onPrimary".
image

When I look through Material UI's default theme, it looks like the palette shape hasn't changed since MD1. The palette still supports accent colors, for example, which are no longer part of the MD2 color palette recommendations. I assume that the palette.primary.contrastText value is supposed to represent this?

Can we expect to see support for these 'on' colors in the theme.palette object or in the component APIs in the future?

Examples 🌈

image

image

@oliviertassinari oliviertassinari added the design: material This is about Material Design, please involve a visual or UX designer in the process label Aug 30, 2019
@oliviertassinari
Copy link
Member

Yes, contrastText is supposed to serve this exact purpose, as far as I understand the specification. So, I think that we are all good.

@mbrookes
Copy link
Member

mbrookes commented Aug 30, 2019

contrastText is supposed to serve this exact purpose

It is, (although it predates the spec having such a concept), but it isn't possible to apply it to typography (using the color prop). Also, while the v2 spec doesn't distinguish between primary and primary variant when it comes to the contrast color, it may be that it needs to be different for main / light / dark.

We could add theme.palette.primary.onMain', theme.palette.primary.onLight, theme.palette.primary.onDark` etc to the theme, and, where it makes sense, to the component color API (principally typography, but the button example is interesting).

@petegivens For the moment you have access to theme.palette.getContrastText, and to set the the related theme.palette.contrastThreshold if needed. And of course you can always add the on colors to the palette yourself, but you would still have to wrap Typography to take advantage of it.

@petegivens
Copy link
Contributor Author

petegivens commented Sep 3, 2019

Also, while the v2 spec doesn't distinguish between primary and primary variant when it comes to the contrast color, it may be that it needs to be different for main / light / dark.

This is true. There's no way to know in advance which level of the primary palette will require white or black text on top of it. Even MaterialUI's system of having primary.main, primary.light, and primary.dark doesn't really work for us. In some of our clients' color palettes, their main primary color is primary900. MaterialUI seems to assume that all color palettes will use primary500 as the main primary so you can always step 2 levels lighter or darker. But that's a bit of a digression I suppose.

@petegivens For the moment you have access to theme.palette.getContrastText, and to set the the related theme.palette.contrastThreshold if needed. And of course you can always add the on colors to the palette yourself, but you would still have to wrap Typography to take advantage of it.

That's the approach we'll take for now. I think adding the on colors to the typography and icon component APIs (<Typography variant="h1" color="onPrimary" />) is an idea worth exploring as it will ensure tighter alignment with the MD spec. It also helps a ton to support a truly theme-able application. In a multi-tenant situation, you never know in advance if white or black is the appropriate color for text on top of the theme's primary color.

The Button example I listed above is not part of MD spec AFAIK--they seem to only advocate using on colors for typography and svg strokes.

@mbrookes
Copy link
Member

mbrookes commented Sep 3, 2019

Even MaterialUI's system of having primary.main, primary.light, and primary.dark doesn't really work for us. In some of our clients' color palettes, their main primary color is primary900

Light and dark do work with 900 colors. You can try it in the docs. If the calculated light and dark values don't work for you, you can adjust theme.palette.tonalOffset, or you can provide all three colors directly.

@petegivens
Copy link
Contributor Author

Light and dark do work with 900 colors. You can try it in the docs. If the calculated light and dark values don't work for you, you can adjust theme.palette.tonalOffset, or you can provide all three colors directly.

I don't understand how that's working. If I set my primary.main color to primary900, it does indeed generate a "darker" primary color, but it won't be a color that exists in my primary color palette (p50-p900). Does it just auto generate a color that's some percentage darker than primary.main?

@petegivens
Copy link
Contributor Author

petegivens commented Sep 3, 2019

Looked up the source code and it seems that my assumption is correct. The color tool calls theme.palette.augmentColor to create the dark and light variants. augmentColor itself calls lighten and darken on primary.main. So my concern is that generating a dark primary color in this way (when our main primary color is primary900) won't work for us since it won't be an approved theme palette color.

I wonder if our understanding of the material color palette is wrong, and our main primary color should always be primary500?

@mbrookes
Copy link
Member

mbrookes commented Sep 3, 2019

it won't be an approved theme palette color

Even if you provide a 500 color for main, light and dark won't be "approved" colors, as they are calculated rather than using the palette colors. You have to provide specific colors for light, main and dark if you want to strictly follow the v1 spec colors.

Google have opened up to non v1 palette colors anyway:

2014 Material Design color palettes

These color palettes, originally created by Material Design in 2014, are comprised of colors designed to work together harmoniously, and can be used to develop your brand palette. To generate your own harmonious palettes, use either the palette generation tool or Material Theme Editor.

Custom colors in the palette tool: https://material.io/resources/color/#!/?view.left=0&view.right=1&primary.color=7f2644

@petegivens
Copy link
Contributor Author

@mbrookes Great point. I did a quick fuzzy search for primary.dark and primary.light in the MUI repo and it seems that they aren't used nearly as frequently as I would have guessed. The way that MD's color picker tool displays them, it seems like they are meant to serve as accent colors (or what they are not calling primary variant / secondary variant?). So I think my whole understanding of their purpose was flawed.

I appreciate the discussion. I don't have any more questions at this point so feel free to close this issue. I do think that adding onPrimary, onSecondary, onBackground, etc. might be a good addition to the typography and icon APIs, but this is easily accommodated on my end by thinly wrapping those components.

@mbrookes
Copy link
Member

mbrookes commented Sep 4, 2019

I have no objection to them being added.

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 6, 2019

I do think that adding onPrimary, onSecondary, onBackground, etc. might be a good addition to the typography and icon APIs, but this is easily accommodated on my end by thinly wrapping those components.

@petegivens The latest strategy is to allow people to use any color from their palette in the core component, moving the system in the core #15561: https://material-ui.com/system/palette/#color . I would imagine that primary could be aliased to primary.main? 🤔 In the case of these colors, I would propose primary.contrastText to reflect the theme structure.

@siriwatknp
Copy link
Member

This issue is outdated (MD2 related). We aim to have a fresh look for Material UI in the near future, so I’m closing this.

Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@petegivens How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design: material This is about Material Design, please involve a visual or UX designer in the process discussion
Projects
None yet
Development

No branches or pull requests

4 participants