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

Implement WCAG contrast ratio calculations #164

Merged
merged 1 commit into from Jan 31, 2020
Merged

Conversation

okaneco
Copy link
Contributor

@okaneco okaneco commented Jan 25, 2020

Add RelativeContrast trait for Luma, add tests

Implements contrast ratio calculation for W3C Web Content Accessibility Guidelines (WCAG). Success Criterion 1.4.3 Contrast (Minimum) and Success Criterion 1.4.6 Contrast (Enhanced) suggest text color and background color contrast ratios accessible for people with low vision or color deficiencies. Success Criterion 1.4.11 Non-text Contrast suggests a minimum contrast ratio for adjacent colors graphical elements such as UI components.

Success Criterion 1.4.3 Contrast (Minimum)

Success Criterion 1.4.6 Contrast (Enhanced)

Success Criterion 1.4.11 Non-text Contrast

Contrast ratio

Relative luminance

@okaneco
Copy link
Contributor Author

okaneco commented Jan 25, 2020

This saves users from having to implement it themselves and is useful for generating color palettes with acceptable contrast between text and background colors. The calculation is easy because of palette but can be improperly implemented if Luma is used for contrast calculation instead of Luma<Linear>.

Copy link
Owner

@Ogeon Ogeon left a comment

Choose a reason for hiding this comment

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

I think this is an excellent idea! It's a quality of life thing that will hopefully make it feel more approachable. That's part of why I'm considering that this may be good to blanket implement for as many types as possible. It makes it easier to discover and it's more trivial than something like hue shift.

palette/src/luma/relative_contrast.rs Outdated Show resolved Hide resolved
palette/src/luma/relative_contrast.rs Outdated Show resolved Hide resolved
impl<S, T> RelativeContrast for Luma<S, T>
where
T: FloatComponent,
S: LumaStandard,
Copy link
Owner

Choose a reason for hiding this comment

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

An alternative could be to force this to be linear. But I'm not sure that's the way to go. Maybe it's better to go the opposite way this time and implement this trait for anything that can be converted to linear Luma. I think a blanket implementation would be possible.

@Ogeon
Copy link
Owner

Ogeon commented Jan 26, 2020

There's apparently some subtleties when it comes to non-text elements: https://www.w3.org/WAI/WCAG21/Understanding/non-text-contrast.html
May be good to somehow clarify that it's for text and also maybe add the 3:1 ratio for other graphical elements.

@okaneco
Copy link
Contributor Author

okaneco commented Jan 27, 2020

I added Success Criterion 1.4.11 Non-text Contrast, tried to clarify the methods were for text and non-text, and implemented the trait on types that can convert to linear luma.

For the original commit, I imagined the primary users would be people using RGB/HSV/HSL/HWB for picking out colors, but it can definitely be convenient for other types too. My concern was that in some color models the contrast ratio calculated isn't guaranteed to be the same when the colors are clipped during conversion to sRGB. I'm not sure how to stop people from assuming contrast will remain the same or if it's that much of a problem.


For the cargo fmt run, cargo +nightly fmt is needed in order to get comment wrapping as specified in the rustfmt.toml. I believe there are 14 files that need formatting still.

@Ogeon
Copy link
Owner

Ogeon commented Jan 27, 2020

These criteria are more guidelines and recommendations than hard rules, as I understand it, so one will have to use manual tests either way. I suppose all we can do there is to recommend that it's used on a color space that is the intended output space. So even if the input is HSV (still in the RGB family), the conversion to RGB is going to be lossy in some way, so it's probably best to convert to RGB before checking the contrast. This does also become an ergonomics v.s. correctness issue that's connected to the whole conversion system. The most correct way would perhaps be to always return a Result when converting, but that's not the convenient way. It's not exactly a straight forward choice.

The fact that we can't know the output format for sure (we can only guess that it's probably sRGB, using statistics) leaves us with very few options. I think restricting it to only be available for Luma will only add friction. What it could do, at best, is to make people aware that some conversion is required, but every color space outside the RGB family can be converted to Luma without going through RGB. That, together with the fact that we don't know what people will do with their colors, makes me lean towards a more enabling than restricting approach. That's why I suggested a blanket implementation like impl<T: IntoColor> RelativeContrast for T, to make it included "for free".

The rest is a matter of education. The trait documentation can have examples that clearly demonstrate the best practices. It can use variable naming to emphasize that it's the output color, for example, and the example scenario can put it in a context where it's even more clear.

For the cargo fmt run, cargo +nightly fmt is needed in order to get comment wrapping as specified in the rustfmt.toml. I believe there are 14 files that need formatting still.

I forgot that there may be problems if someone is developing on the nightly channel, although that's not recommended for compatibility reasons. That config file doesn't do much good really...

palette/src/luma/relative_contrast.rs Outdated Show resolved Hide resolved
/// [Success Criterion 1.4.11 Non-text Contrast (Level AA)](https://www.w3.org/WAI/WCAG21/Understanding/non-text-contrast.html)
pub trait RelativeContrast {
/// The return type for the contrast ratio.
type Scalar: FloatComponent;
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can tone this requirement down a bit. 🙂 All we need is PartialOrd + FromF64, as far as I can tell, and it doesn't have to be useful as a color component. The rest is implementation details. Part of why I'm commenting on this is that I'm considering reducing the trait requirements further, if possible. Float, for example, implies a lot of additional mathematical properties!

@Ogeon
Copy link
Owner

Ogeon commented Jan 27, 2020

Interesting with API design on the fly. I hope you don't feel like I'm pushing too much. I have just been thinking a lot about how to improve the trait situation. Hence my mind dump earlier. I have some ideas regarding how things may become more convenient. Some of it involves doing more "under the hood". For example trying to combine component conversion with color space conversion, so it becomes possible to go from Srgb<u8> to Lch<f32> in a single method call. But there's still some distance to go before it's there yet...

As for this, I think a final thing that probably should be done is to move this trait out of the luma module. It's no longer a special thing for Luma, after all.

@okaneco
Copy link
Contributor Author

okaneco commented Jan 27, 2020

It's fine, no worries. For this PR and the color difference PR I wasn't sure where they exactly fit in because they extend functionality in different ways than most of the operations already in the library. I expected a lot of reworking.

For example trying to combine component conversion with color space conversion

That would be excellent.

@Ogeon
Copy link
Owner

Ogeon commented Jan 28, 2020

Good to hear I'm not overly annoying! Things like these are not always straight forward. There's always that balance I mentioned and other factors. In the end we are all trying to improve this thing as much as we can.

@okaneco
Copy link
Contributor Author

okaneco commented Jan 30, 2020

I submitted a rough draft of documentation to educate about the usage of the trait. I just wasn't sure about what the doc test should explicitly show.

The rest is a matter of education. The trait documentation can have examples that clearly demonstrate the best practices. It can use variable naming to emphasize that it's the output color, for example, and the example scenario can put it in a context where it's even more clear.

Copy link
Owner

@Ogeon Ogeon left a comment

Choose a reason for hiding this comment

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

Awesome! And thanks for sticking with this through all the twists and turns. I wrote a comment for the documentation part. Otherwise I think it's about time this goes in.

Although, an improvement I don't know why I didn't think of earlier would actually be to do a similar thing as what we did for the color difference. We could have a standalone function that calculates the contrast ratio only, with the luma as input:

fn contrast_ratio<T: Add + Div + FromF64>(luma1: T, luma2: T) -> T {
    if luma1 > luma2 {
        (luma1 + from_f64(0.05)) / (luma2 + from_f64(0.05))
    } else {
        (luma2 + from_f64(0.05)) / (luma1 + from_f64(0.05))
    }
}

I must have been too preoccupied with figuring out the blanket implementation.

Also, the import grouping used to be pretty arbitrary, so I appreciate the cleanup! 👍 But I'm afraid I have mostly moved away from doing it recently (the old files are just lagging behind), because I feel like it's not something I want to require people to follow. I'm not going to make you undo it, though.

This repo has some old dust that would be good to clean out, really. Migrating to the 2018 edition wouldn't hurt, either. But that's a different topic!

palette/src/relative_contrast.rs Outdated Show resolved Hide resolved
@Ogeon
Copy link
Owner

Ogeon commented Jan 30, 2020

Or all the other color types could just call the method on Luma. Converting to it is always going to be as cheap as possible, anyway. 🤷🏻‍♂️

@Ogeon
Copy link
Owner

Ogeon commented Jan 30, 2020

Nice! I think that gets the message across. Let me know when you feel like it's ready to be merged.

@okaneco
Copy link
Contributor Author

okaneco commented Jan 30, 2020

Twists and turns indeed.

I liked the idea of moving the calculation into the relative_contrast module which simplifies all the calls and should help make it more maintainable. It is quite obvious in retrospect but I completely missed it as well while trying to handle the blanket impl.

I've fixed some minor grammar issues I just noticed but I believe this is finished.

Also, I'm not sure if this is just for Travis but the Mac Nightly builds seem to have gotten a bit slower and now the Beta as well taking 11-12 mins to complete.

Add RelativeContrast trait
Add contrast_ratio function
Implement trait for types that convert to linear luma
@Ogeon
Copy link
Owner

Ogeon commented Jan 31, 2020

Looks great! Thanks!

There was a new Rust release just yesterday, so there may have been a regression there. Or just high load on their servers. I'll have a look at the numbers in a moment and merge after that.

@Ogeon
Copy link
Owner

Ogeon commented Jan 31, 2020

The Mac runners can be slow sometimes and this seem to be within the ballpark, so it may have been just bad luck. But we'll have to see if it stays consistent.

Either way, time to merge!

bors r+

bors bot added a commit that referenced this pull request Jan 31, 2020
164: Implement WCAG contrast ratio calculations r=Ogeon a=okaneco

Add RelativeContrast trait for Luma, add tests

Implements contrast ratio calculation for W3C Web Content Accessibility Guidelines (WCAG). Success Criterion 1.4.3 Contrast (Minimum) and Success Criterion 1.4.6 Contrast (Enhanced) suggest text color and background color contrast ratios accessible for people with low vision or color deficiencies. Success Criterion 1.4.11 Non-text Contrast suggests a minimum contrast ratio for adjacent colors graphical elements such as UI components.

[Success Criterion 1.4.3 Contrast \(Minimum\)](https://www.w3.org/TR/WCAG/#contrast-minimum)

[Success Criterion 1.4.6 Contrast \(Enhanced\)](https://www.w3.org/TR/WCAG/#contrast-enhanced)

[Success Criterion 1.4.11 Non-text Contrast](https://www.w3.org/TR/WCAG/#non-text-contrast)

[Contrast ratio](https://www.w3.org/TR/WCAG/#dfn-contrast-ratio)

[Relative luminance](https://www.w3.org/TR/WCAG/#dfn-relative-luminance)

Co-authored-by: okaneco <47607823+okaneco@users.noreply.github.com>
@Ogeon
Copy link
Owner

Ogeon commented Jan 31, 2020

bors r+

bors bot added a commit that referenced this pull request Jan 31, 2020
164: Implement WCAG contrast ratio calculations r=Ogeon a=okaneco

Add RelativeContrast trait for Luma, add tests

Implements contrast ratio calculation for W3C Web Content Accessibility Guidelines (WCAG). Success Criterion 1.4.3 Contrast (Minimum) and Success Criterion 1.4.6 Contrast (Enhanced) suggest text color and background color contrast ratios accessible for people with low vision or color deficiencies. Success Criterion 1.4.11 Non-text Contrast suggests a minimum contrast ratio for adjacent colors graphical elements such as UI components.

[Success Criterion 1.4.3 Contrast \(Minimum\)](https://www.w3.org/TR/WCAG/#contrast-minimum)

[Success Criterion 1.4.6 Contrast \(Enhanced\)](https://www.w3.org/TR/WCAG/#contrast-enhanced)

[Success Criterion 1.4.11 Non-text Contrast](https://www.w3.org/TR/WCAG/#non-text-contrast)

[Contrast ratio](https://www.w3.org/TR/WCAG/#dfn-contrast-ratio)

[Relative luminance](https://www.w3.org/TR/WCAG/#dfn-relative-luminance)

Co-authored-by: okaneco <47607823+okaneco@users.noreply.github.com>
@Ogeon
Copy link
Owner

Ogeon commented Jan 31, 2020

Seems like Bors isn't feeling well today.

@Ogeon
Copy link
Owner

Ogeon commented Jan 31, 2020

Let's see if this works now.

bors r+

bors bot added a commit that referenced this pull request Jan 31, 2020
164: Implement WCAG contrast ratio calculations r=Ogeon a=okaneco

Add RelativeContrast trait for Luma, add tests

Implements contrast ratio calculation for W3C Web Content Accessibility Guidelines (WCAG). Success Criterion 1.4.3 Contrast (Minimum) and Success Criterion 1.4.6 Contrast (Enhanced) suggest text color and background color contrast ratios accessible for people with low vision or color deficiencies. Success Criterion 1.4.11 Non-text Contrast suggests a minimum contrast ratio for adjacent colors graphical elements such as UI components.

[Success Criterion 1.4.3 Contrast \(Minimum\)](https://www.w3.org/TR/WCAG/#contrast-minimum)

[Success Criterion 1.4.6 Contrast \(Enhanced\)](https://www.w3.org/TR/WCAG/#contrast-enhanced)

[Success Criterion 1.4.11 Non-text Contrast](https://www.w3.org/TR/WCAG/#non-text-contrast)

[Contrast ratio](https://www.w3.org/TR/WCAG/#dfn-contrast-ratio)

[Relative luminance](https://www.w3.org/TR/WCAG/#dfn-relative-luminance)

Co-authored-by: okaneco <47607823+okaneco@users.noreply.github.com>
@bors
Copy link
Contributor

bors bot commented Jan 31, 2020

Build succeeded

@bors bors bot merged commit 1abba19 into Ogeon:master Jan 31, 2020
@okaneco okaneco deleted the wcag branch January 31, 2020 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants