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

Allow palettes to specify a separate hue for uncontained text #17212

Closed
1 task done
mjadczak opened this issue Aug 28, 2019 · 6 comments
Closed
1 task done

Allow palettes to specify a separate hue for uncontained text #17212

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

Comments

@mjadczak
Copy link

This is the same issue as angular/components#15148, and has been released in Angular Material v8.1.0.

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

Summary 💡

When using custom primary / secondary colours, it is sometimes difficult to choose a colour (or shade thereof) which works well both as a solid background colour, as well as as a text colour over a neutral background. It would be good to allow custom palettes to specify the exact shade to be used for text in cases where e.g. color="primary" is used.

Examples 🌈

I propose adding another key to PaletteColor, namely text. This is "the colour which is used when a component (e.g. a button) wants to display plain text without its own background, and want to display it using the colour of a particular palette". This is different from contrastText, which is the text colour used when using this PaletteColor as a background, and it is different from theme.palette.text.primary, which has nothing to do with the "primary" palette and is rather for "primary neutral text".

To preserve backwards compatibility, this new text field would be added to SimplePaletteColorOptions as an optional field and would default to using the main colour.

This new key would then be used instead of main in cases like this.

@oliviertassinari
Copy link
Member

My intuition is that we should keep the theme structure as simple as possible, hence reject the proposal.

@mjadczak
Copy link
Author

This is a genuine issue which affects accessibility, which is why it was implemented in Angular Material. The issue manifests itself when using a light colour for a primary or secondary hue - when this colour is used for a button regardless of whether it is a text button or filled button, suddenly the colour must contrast well with two colours, not one.

Namely, in the filled case, the colour must contrast well with the dark text in order to remain accessible, but in the text case, it must also contrast well with the light background, which it almost certainly will not. In fact, this can clearly be seen in the docs for color:
image
where the "Reset docs colors" button is very illegible becasue of this.

If this were an issue only with the button, I could see your point about not introducing a special-case key into the theme, as it can be worked around using overrides (which is what I am currently doing). However, this affects any component which displays primary or secondary or error text against a light background: for instance the links in the docs when the secondary (not primary) colour is set this way:
image

All in all, I do think this new property does apply to more than just Buttons, and so I think it should belong in the theme, and be used by any component which draws "just text" without its own background. As well as that, it is quite easy to add in a backwards-compatible way.

@oliviertassinari
Copy link
Member

@mjadczak I don't understand the example case. The primary color is not suitable for being a primary color in the first place, e.g. the slider.

@oliviertassinari
Copy link
Member

Did you have a look at how the problem is solved in https://github.com/material-components/material-components-web? I do agree that they might be an opportunity to support a wider range of colors.

@oliviertassinari oliviertassinari added the design: material This is about Material Design, please involve a visual or UX designer in the process label Aug 29, 2019
@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

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

3 participants