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

fix(theming): Adjust theming util to calculate primary element color based on WCAG color contrast #42285

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Dec 14, 2023

Summary

This adjusts the theming utils for calculating the primary color:

  1. Adjust luma calculation to match with the algorithm used by WCAG 2
  2. Add function for calculating color contrast according to WCAG 2
  3. Fix the invertTextColor by not simply checking the luma channel of the HSL color but the real color contrast
  4. Improved primary element color calculation by providing the background color so an element color with high enough contrast can be calculated

Also keep blurry background and hover states in mind while calculating the element color.

Moreover I added a unit test for accessible colors.

Screenshots

Only the hover state has changed:

before after
image image

Checklist

@susnux
Copy link
Contributor Author

susnux commented Dec 14, 2023

/backport to stable28

@susnux susnux changed the title Fix/theming color utils primary fix(theming): Adjust theming util to calculate primary element color based on WCAG color contrast Dec 14, 2023
@susnux susnux force-pushed the fix/theming-color-utils-primary branch from 9691b35 to 3441852 Compare December 15, 2023 15:15
@emoral435 emoral435 force-pushed the fix/theming-color-utils-primary branch from 3441852 to aededaf Compare December 15, 2023 18:14
…contrast and luma calculation)

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
… high contrast themes

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the fix/theming-color-utils-primary branch from aededaf to 84123b3 Compare December 15, 2023 20:09
@susnux
Copy link
Contributor Author

susnux commented Dec 15, 2023

Had to skip the test if the base class was run as a test case on its own as - of cause - the $theme was not set there. So in that case the test need to be skipped and not failed.

@susnux susnux added this to the Nextcloud 29 milestone Dec 15, 2023
@susnux susnux merged commit 8aa91a1 into master Dec 15, 2023
50 checks passed
@susnux susnux deleted the fix/theming-color-utils-primary branch December 15, 2023 20:35
@ShGKme
Copy link
Contributor

ShGKme commented Dec 20, 2023

Is it intended change?

Dark theme:

Before After
image image

@susnux
Copy link
Contributor Author

susnux commented Dec 20, 2023

Is it intended change?

Yes and no.
Yes as the primary-element-text color is adjusted for color contrast.
No as this is caused by using the wrong color variables, the menu is shown on the primary color, thus color-primary-text should be used and not color-primary-element-text.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants