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

Add stronger guarantee of readability/contrast for palette background/text color pairs #2376

Merged

Conversation

Davidster
Copy link
Contributor

@Davidster Davidster commented Apr 7, 2024

If we run the todo example with the SolarizedDark theme, we can see that the text color selected for display on top of a cyan background is white ('All' button):

image

which brutally fails the WCAG contrast guidelines.

This change guarantees that the text color passes the test, and passes the guidelines by selecting black text for the example in question.

@hecrj hecrj added this to the 0.13 milestone Apr 7, 2024
@hecrj hecrj added bug Something isn't working styling fix labels Apr 7, 2024
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I wonder if there is a way to eventually adapt the original text color smartly for better contrast, instead of defaulting to white/black.

@hecrj hecrj merged commit 6304235 into iced-rs:master Apr 7, 2024
12 checks passed
@hecrj
Copy link
Member

hecrj commented Apr 7, 2024

Ended up reverting this because the solution here is just trading some bad cases for others.

The fallback.inverse() case still does not guarantee readability and it is affecting the Light and Dark theme variants, which are the most common.

We need to find a smarter way to generate a readable color.

hecrj added a commit that referenced this pull request Apr 7, 2024
…color_contrast"

This reverts commit 6304235, reversing
changes made to 31d1d5f.
@Davidster
Copy link
Contributor Author

Davidster commented Apr 7, 2024

Darn, well surely if swapping to the complete other end of the luminance spectrum doesn't fix the contrast then there is no possible way to pass the WCAG test for that particular background color, right? Maybe if neither white nor black passes the test then we could take the one that's closest to passing?

.. and log a warning if no good option was found?

@Davidster
Copy link
Contributor Author

Davidster commented Apr 7, 2024

we could also use the AA contrast ratio (4.5:1) instead of AAA (7:1) as a minimum

@hecrj
Copy link
Member

hecrj commented Apr 7, 2024

Maybe if neither white nor black passes the test then we could take the one that's closest to passing?

@Davidster Good idea. Did that in 72b975e and seems to work well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix styling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants