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(core-theming): get-theme-color($theme, $role-color) returns incorrect shades for 4 colors #29577

Closed
1 task
vulture9 opened this issue Aug 14, 2024 · 2 comments · Fixed by #29655
Closed
1 task
Assignees
Labels
area: theming P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@vulture9
Copy link

vulture9 commented Aug 14, 2024

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

No response

Description

For the given function get-theme-color($theme, $role-color) four of the shades are incorrectly mapped:
Light mode:

  • surface . Current implementation is using n-98 , where should be mapped to n-99.

dark mode:

  • surface . Currently using n-10, where should be mapped to n-6.
  • on-surface-variant Currently using nv-80 , where should be mapped to nv-90
  • surface-container-highest Currently using n-22, where should be mapped to n-24.

reference to comparison material theme builder:
https://material-foundation.github.io/material-theme-builder/
image
image

Tested version:
"@angular/material": "18.1.0",

Reproduction

Steps to reproduce:

  1. call get-theme-color($theme, role-color) for role colors 'surface', 'on-surface-variant', 'surface-container-highest'
  2. compare shades of colors from the pallet that are returned. wrong shades are used according to spec

Expected Behavior

correct shades are returned based on the specification. reproduction steps are very clear.

Actual Behavior

slightly wrong color shades are returned than on specification. just compare whatever is here for given mode light/dark mode and you can clearly see that wrong shades are used

Environment

  • Angular: 18.1.0
  • CDK/Material: 18.1.0
  • Browser(s): all, browser independent
  • Operating System (e.g. Windows, macOS, Ubuntu): Mac and Windows
@vulture9 vulture9 added the needs triage This issue needs to be triaged by the team label Aug 14, 2024
@vulture9 vulture9 changed the title bug(core-theming): get-theme-color($theme, $role-color) returns incorrect shades for 2 colors bug(core-theming): get-theme-color($theme, $role-color) returns incorrect shades for 4 colors Aug 14, 2024
@amysorto
Copy link
Contributor

Good catch, thanks for pointing this out!

As mentioned above, the current colors are mapped incorrectly. The rest should be audited as well:

  • (Light theme) surface maps to neutral 99 instead of neutral 98
  • (Dark theme) surface maps to neutral 10 instead of neutral 6
  • (Dark theme) on-surface-variant maps to neutral 80 instead of 90
  • (Dark theme) surface-container-highest maps to neutral 22 instead of 24

@amysorto amysorto self-assigned this Aug 27, 2024
@amysorto amysorto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent area: theming and removed needs triage This issue needs to be triaged by the team labels Aug 27, 2024
@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 Sep 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: theming P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants