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

bug(button): Palette contrast values are not applied #26056

Closed
1 task done
mlimones-pensare opened this issue Nov 22, 2022 · 62 comments · Fixed by #28664
Closed
1 task done

bug(button): Palette contrast values are not applied #26056

mlimones-pensare opened this issue Nov 22, 2022 · 62 comments · Fixed by #28664
Assignees
Labels
area: material/button P2 The issue is important to a large percentage of users, with a workaround

Comments

@mlimones-pensare
Copy link

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

13.x

Description

When using a custom theme the contrast values from a custom palette are not applied

Reproduction

steps

  • define a custom palette with a "slightly light" color and a white contrast color
$md-secondary: (
  ...
  500 : #f86072,
  ...
contrast: (
  ...
    500 : #ffffff
  ...
  )
);
  • Apply the theme
$theme-primary: mat.define-palette($md-primary,500);
$theme-accent: mat.define-palette($md-secondary,500);
$theme-warn: mat.define-palette(mat.$red-palette);
$theme: mat.define-light-theme((
  color: (
    primary: $theme-primary,
    accent: $theme-accent,
    warn: $theme-warn,
  ),
  typography: mat.define-typography-config(),
));
@include mat.all-component-themes($theme);

example:

https://stackblitz.com/edit/angular-5b8rw5?file=src/theme.scss

Expected Behavior

Text in accent button should be white (contrast color) as specified in the palette

Actual Behavior

Text in accent button is black

Environment

  • Angular:15.0.0
  • CDK/Material:15.0.0
  • Browser(s):chrome, safari
  • Operating System (e.g. Windows, macOS, Ubuntu):Ubuntu
@mlimones-pensare mlimones-pensare added the needs triage This issue needs to be triaged by the team label Nov 22, 2022
@beckerjohannes
Copy link

Also ran into this issue. In my case with a slightly light primary color

  1. Raised Buttons shows primary with black text
  2. Primary Tool Bar shows primary with white text

The issue seems to be, that the button uses the on-primary calculated by the mdc-theme See:

$on-primary: mdc-theme-color.prop-value(on-primary);

Whereas the toolbar uses the contrast color defined by the custom theme. See:
color: theming.get-color-from-palette($palette, default-contrast);

@GeorgDangl
Copy link
Contributor

We're having the same issue, using the cyan 600 color as primary, button color (text and icons) is black. It used to be white before.

However, I've noticed that using a slightly darker cyan 700, the color seems to work again. But, then the primary color is off, of course😀

@sergidt
Copy link

sergidt commented Dec 19, 2022

Same issue, and breaking all my product styles and need to be overwritten, forcing colors hardcoding rules

@inlym
Copy link

inlym commented Jan 6, 2023

Notify me if resolved.

@davidpawar
Copy link

Also facing the same issue.

@johnnygerard
Copy link

Same issue.

@pqt-tmeitz
Copy link

are there any updates yet?

@Frankitch
Copy link

Frankitch commented Feb 1, 2023

Same issue for me, my workaround:

.mat-mdc-unelevated-button.mat-primary {
 --mdc-filled-button-label-text-color: var(--player-primary-contrast-500);
}

.mat-mdc-unelevated-button.mat-accent {
  --mdc-filled-button-label-text-color: var(--player-accent-contrast-500);
}

.mat-mdc-raised-button.mat-primary {
  --mdc-protected-button-label-text-color: var(--player-primary-contrast-500);
}

.mat-mdc-raised-button.mat-accent {
  --mdc-protected-button-label-text-color: var(--player-accent-contrast-500);
}

@MichalK6677
Copy link

The problem is still there. The Angular Material v15 is not usable for more than 2 months.

@bjoernboehnke
Copy link

Just as an information for others facing an issue like this:
There seems to be a change affecting "unthemed" buttons. A button in my application changed text-color after upgrading to angular material 15 from white to black. It is placed on a dark background. I found this issue and assumed it was caused by this. Just give it a try and give your button a color attribute if it has none and select primary, accent or warn depending on your background. You can use mat-flat-button and then the correct contrast color is selected.
Definitely not a solution for everybody but worked for me.

@angelaki
Copy link

angelaki commented Feb 17, 2023

Any chance this will get fixed soon? I need my contrast color to get used because our CI uses a white font on a pretty light blue. Now all the buttons etc. have a black text.

Checkbox' checkmark is affected, too (just like probably most of the compontents).

@mvanderlee
Copy link

More details here: #26153

@sonphnt
Copy link

sonphnt commented Feb 23, 2023

the same issue to me as well, contrast color does not work.

@Geniewave
Copy link

same issue here

@ChristianKohler
Copy link

Same issue. As @beckerjohannes mentioned above, the issue is that for buttons the contrast color is not used. Instead the on-primary color from material/theme is applied.

The on-primary color is used for the button text color.

$on-primary: mdc-theme-color.prop-value(on-primary);

Now on-primary is defined here:

https://github.com/material-components/material-components-web/blob/cedffb44c0c5e2b6aeca591b4e5d19346a52983c/packages/mdc-theme/_theme-color.scss#L137


@function tone($color) {
  $minimumContrast: 3.1;

  $lightContrast: contrast($color, white);
  $darkContrast: contrast($color, rgba(black, 0.87));

  @if ($lightContrast < $minimumContrast) and ($darkContrast > $lightContrast) {
    @return 'light';
  } @else {
    @return 'dark';
  }
}

@function contrast-tone($color) {
  @return if(tone($color) == 'dark', 'light', 'dark');
}


$on-primary: if(contrast-tone($primary) == 'dark', #000, #fff) !default;

As you can see, the minimal contrast is hardcoded. Depending on the contrast the tone is set to light or dark and depending on the tone the text color is set to white or black.

@jnain
Copy link

jnain commented Apr 4, 2023

Same issue here. Hope it'll get fixed soon! :)

Thank you @Frankitch for the workaround!

@the-ult
Copy link

the-ult commented Apr 12, 2023

See: #26179 for the same kind of issues

@ProjectBay
Copy link

Same with checkboxes

@AlvaroP95
Copy link

Angular 16 is already live, and this is not fixed. Any news on this?

@aponski
Copy link

aponski commented May 12, 2023

I have the same problem with buttons and checkboxes. Contrast color of my custom theme is not applied correctly.

@wagnermaciel wagnermaciel added P2 The issue is important to a large percentage of users, with a workaround area: material/button and removed needs triage This issue needs to be triaged by the team labels May 15, 2023
@Leccho
Copy link

Leccho commented Jun 1, 2023

Most of the Angular Material Components don't work properly for the color. I don't know how this as not been fix already. Should have been fix first week after the release of v15, yet here we are at v16 and the problem persist. I'm deceived by this.

@p3pp8
Copy link

p3pp8 commented Jul 4, 2023

Same here! V16.1.3 as well, i had to override text color for raised button as contrast palette wasn't applied.

@nandomegale
Copy link

Same here with v17.0.4

The contrast for 600 is white.

image

But the raised-button applys black

image

@rom111419
Copy link

rom111419 commented Jan 28, 2024

The same error. I want assign my theme and dont want rewriting initial vars, cause it`s bad idea or crutch

@mrctrifork
Copy link

mrctrifork commented Jan 30, 2024

Same here. I've been hitting myself in the head for the past two hours… thank god I found this issue. Thanks for the fix @Plabbee

@Iancovski
Copy link

Same problem in Angular Material 17.1.0. I'm using the package from @Plabbee for workaround, it works perfectly!

@Blafasel3
Copy link

Pls fix this. There is actually a forked repo with a fix. This bug is super painful, because even if you got a workaround, it breaks on every major update eventually and one has to find a new workaround. This is the 3rd time i'm stumbling over this.

@marcguillemdev
Copy link

marcguillemdev commented Feb 20, 2024

I'm exactly at this point and with the same problem:

Same here with v17.0.4

The contrast for 600 is white.

image

But the raised-button applys black

image

@MichalK6677
Copy link

This is a real show stopper for version 17. This version should be considered as beta. I am not able to update without destroyed branding and I have to stay on v16 with legacy components.

@timneil
Copy link

timneil commented Feb 26, 2024

Why are the Angular Material team not reponding to this issue?!

@BelangerB
Copy link

BelangerB commented Feb 28, 2024

Imagine being asked by your manager to update your project to the latest version and having to tell them "sorry I can't, Angular team broke the CSS formatting and won't fix it". I'm sure that'd go over great. Please, fix this already.

Edit: crisbeto has swept in and saved us!

crisbeto added a commit to crisbeto/material2 that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes angular#26056.
crisbeto added a commit to crisbeto/material2 that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes angular#26056.
crisbeto added a commit to crisbeto/material2 that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes angular#26056.
crisbeto added a commit to crisbeto/material2 that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes angular#26056.
crisbeto added a commit to crisbeto/material2 that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes angular#26056.
@noel-invonto
Copy link

This is really painful. Is there no hope? The fact I need a bazooka to change the text color of a button after creating a palette is actually insane. The additional tech debt I need to add to my project for a button text color is really frustrating.

@mvanderlee
Copy link

@noel-invonto I was about to say "No there is no hope" as it's been ignored for over a year. And I get an email at least once a week from the various tickets on this.
BUT I see that crisbeto has started work on this today, so yes.. there is definitely hope!

crisbeto added a commit to crisbeto/material2 that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes angular#26056.
crisbeto added a commit to crisbeto/material2 that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes angular#26056.
crisbeto added a commit to crisbeto/material2 that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes angular#26056.
@crisbeto crisbeto self-assigned this Feb 29, 2024
crisbeto added a commit that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes #26056.
crisbeto added a commit that referenced this issue Feb 29, 2024
During the transition to tokens, we ended up with a bunch of places with hardcoded values in the form of `if($is-dark, #fff, #000)`. This is problematic for custom palettes, because the value is always hardcoded.

These changes attempt to derive the same values from the palette directly.

Fixes #26056.

(cherry picked from commit cfdfa9a)
@nandomegale
Copy link

nandomegale commented Mar 1, 2024

@crisbeto when the fix will be avaible?

@crisbeto
Copy link
Member

crisbeto commented Mar 1, 2024

It will be in the release next week.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/button P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.