1478156 - Adding color contrast information to the color picker

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Micah [:mtigley] (she/her)]

Contrast ratio information of the selected color will be added in this task according to the new design: https://mozilla.invisionapp.com/share/CGMV7JMRV36#/screens

Much of the groundwork for this component has already been developed in Bug1332090 and all that needs to be done is porting this code over to the current picker and polished.

Comment 1

[Yura Zenevich [:yzen] (please use "needinfo")]

Hi Victoria, my new intern, Maliha, will be picking this up starting next week and I was wondering if you had any comments/updates to the wireframes that were originally considered for this bug? Thanks

Comment 2

[Maliha Islam [:mislam]]

Attachment: Bug 1478156 - Adding color contrast information to the color picker

Comment 3

[Maliha Islam [:mislam]]

Attachment: Bug 1478156 - Move shared code to be used by color contrast feature, r=yzen

Comment 4

[Maliha Islam [:mislam]]

Attachment: Bug 1478156 - Add color contrast span to color picker, r=yzen

Depends on D32490

Comment 5

[Maliha Islam [:mislam]]

Attachment: Bug 1478156 - Add contrast ratio annotation title and info link to color picker, r=yzen

Depends on D32491

Comment 6

[Maliha Islam [:mislam]]

Attachment: Bug 1478156 - Make color picker tooltip keyboard accessible, r=yzen

Depends on D32842

Comment 7

[Victoria Wang [:victoria]]

So excited to see this work happening! The original mockups should still be accurate :) but let me know if any new questions come up.

Comment 8

[Maliha Islam [:mislam]]

Attachment: Annotation Collapsed

Hi Victoria,

We decided to do an expandable annotation section instead of showing it on hover. I attached some screenshots. Please let us know what you think!

Maliha

Comment 9

[Maliha Islam [:mislam]]

Attachment: Annotation Expanded

Comment 10

[Maliha Islam [:mislam]]

Attachment: Error: No Annotation

Comment 11

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Maliha Islam [:mislam] from comment #8)

Created attachment 9070032 [details]
Annotation Collapsed

Hi Victoria,

We decided to do an expandable annotation section instead of showing it on hover. I attached some screenshots. Please let us know what you think!

Maliha

To add more context, we are considering the expandable section as it is more inclusive than hover activatable tooltip + leaves an option for potentially add an eye-dropper button for the background colour.

Comment 12

[Maliha Islam [:mislam]]

Attachment: Bug 1478156 - Move shareable focus methods to be used for color picker accessibility, r=yzen

Depends on D32842

Comment 13

[Victoria Wang [:victoria]]

This looks great! And that reasoning for the expandable UI totally makes sense.

Minor visual tweak: would you mind adding a little extra bottom padding to the whole tooltip? (4px extra space between the contrast info and the bottom of the tooltip, so that the contrast section looks more vertically centered in that space).

Related good news: All the tooltips are being updated to a design that is no longer translucent, so everything will be easier to read https://bugzilla.mozilla.org/show_bug.cgi?id=1553472

Comment 14

[Maliha Islam [:mislam]]

Will do! Thanks, Victoria :) It's great that we are planning to make the tooltip backgrounds a solid color, I always found it a little distracting to see the devtools body through it.

Comment 15

[Maliha Islam [:mislam]]

Attachment: Bug 1478156 - Make color picker tooltip screen reader accessible, r=yzen

Depends on D33331

Comment 16

[Maliha Islam [:mislam]]

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=56942508041c8a9300060aa75cbddfe8528c7947

Comment 17

[Maliha Islam [:mislam]]

Attachment: contrast-ratio-success-state-final

Comment 18

[Maliha Islam [:mislam]]

Attachment: contrast-ratio-fail-state-final

Comment 19

[Maliha Islam [:mislam]]

Attachment: contrast-ratio-dragger-hover-state-final

Hi Victoria,

We decided to make some changes to the color picker I showed you last time (please check the last few attachments). There is no expandable section anymore and we decided to add a color name (or "closest to" color name) title when the user hovers over the spectrum dragger or the color preview circle. I think it would also be nice to make this color name more visible in the color picker as a span, but not sure how this will fit in with the current design. What are your thoughts?

Comment 20

[Victoria Wang [:victoria]]

Hi Maliha!

I talked to Yura a bit to learn more about the reasoning behind the changes, and here's what I've come up with. See this new mockup for a visual reference.

1. I think the info text is a bit too much to have always showing, since it's more of a notice that most people won't need after seeing it for the first time. Since we can't put the info text in a expandable section anymore (without a lot of extra work, sounds like) I'd suggest we instead move it back into the tooltip. However, we can also add an info icon which the user can click to learn more about color contrast.

The simplest link to use is this MDN page, though its tutorial-like format doesn't really fit our use case. A better approach could be to make a new section in the DevTools color picker page that gives a compact overview of the WCAG specs. I'll ping some folks from MDN to see if they have thoughts on this. For reference, Chrome's color contrast page which is linked to from their color picker seems really nice.

2. I think it would be best to show the color name tooltip only in the color preview circle, and not over the whole spectrum rectangle, as it seems a little distracting in the spectrum since it's such a large target.

3. Here are a few small visual tweaks:

• A little more left margin on the Contrast text would be great to line up with the eyedropper icon (about 12px looks like)
• Move the first of the horizontal sliders up a bit so it lines up with the color preview circle. The extra spacing between the two sliders should help usability as well.

Thanks - let me know if anything is unclear!

Comment 21

[Victoria Wang [:victoria]]

Attachment: image.png

Hi Irene and Chris! For this new color picker, we want to add a link to an MDN page that explains color contrast for accessibility, including the WCAG ratio numbers and "AA/AAA" terminology.

This MDN page on color contrast would be the easiest solution, though its tutorial-like format doesn't really fit our need, which is for a more compact informational page. A better approach could be to make a new section in the DevTools color picker page. For reference, Chrome's color contrast page which is linked to from their color picker seems very nice. Would love to hear your thoughts on how to proceed.

Comment 22

[Chris Mills [:cmills]]

Hi Victoria!

The color contrast page I wrote is designed to be linked to from the color contrast accessibility check results in the a11y inspector. It is not really a tutorial, but more of an informational page. I think I'd prefer to keep information on how to fix color contrast issues and explanation of the related WCAG criteria in that page, and link to it from the color picker page, which should be more focused on just covering the tool features.

I do think that https://developer.mozilla.org/en-US/docs/Web/Accessibility/Understanding_WCAG/Perceivable/Color_contrast#Solution could be expanded to include better information on what the contrast ratios mean, and the WCAG criteria, in the same sort of way as the Google article does.

Would this work for you?

Comment 23

[Maliha Islam [:mislam]]

Attachment: max-score-large-text-new

Comment 24

[Maliha Islam [:mislam]]

Attachment: info-button-hover-state-new

Comment 25

[Maliha Islam [:mislam]]

Attachment: circle-color-preview-hover-state-final

Comment 26

[Maliha Islam [:mislam]]

Attachment: contrast-ratio-hover-state-final

Comment 27

[Maliha Islam [:mislam]]

:victoria I have attached a few screenshots of the updated tooltip. Let me know if you have any questions :)

Comment 28

[Victoria Wang [:victoria]]

Hi Maliha, these look beautiful! Thanks so much for making the changes.

One little tweak: Right side of the spectrum rectangle looks like it needs a bit more margin to match the left side.

And one slightly bigger question about title attr usability for both you and Yura:
In the mockup I had a light gray underline under the contrast ratio/icon to hint that there's extra info to be found there (since it's not just alt text). It's inspired by the traditional dotted-underline styling for abbreviations. We could add that in, but now that I'm looking at your screenshots, I wonder if we should combine the two title tags into just one, which would be found by hovering over the (?). It could say (assuming ability to use new lines):

Does not meet WCAG standards for accessible text
Calculated against background: #ffffff
Learn more on MDN

What do you think?

Comment 29

[Victoria Wang [:victoria]]

Hi Chris!

The color contrast page I wrote is designed to be linked to from the color contrast accessibility check results in the a11y inspector. It is not really a tutorial, but more of an informational page. I think I'd prefer to keep information on how to fix color contrast issues and explanation of the related WCAG criteria in that page, and link to it from the color picker page, which should be more focused on just covering the tool features.

I do think that https://developer.mozilla.org/en-US/docs/Web/Accessibility/Understanding_WCAG/Perceivable/Color_contrast#Solution could be expanded to include better information on what the contrast ratios mean, and the WCAG criteria, in the same sort of way as the Google article does.

Thanks for the info! I see how that page makes sense when linked to from the a11y inspector. For linking from the color picker, I feel it would help to have maybe 1-2 sentences on contrast ratios and WCAG criteria near the top of the page, as a quick summary, since I feel there's a more time-sensitive need to know what the UI means rather than just the concept of fixing contrast issues. (I guess we could also link to the Solution section)

Here's a quick attempt at the sort of thing I'm thinking of:
"When designing readable interfaces for different vision capabilities, a good rule of thumb is the WCAG guidelines which recommend an AA contrast ratio of 4.5:1 or higher for text. You can increase the contrast to an AAA rating of 7:1 to cover a greater range of deficiencies. Very large text (120-150% larger than the body text) can go down to 3:1."

Comment 30

[Victoria Wang [:victoria]]

I thought a little more about the combined title attributes and now think it’s a bad idea 😅. Poor title tooltip would be doing too much work. I think if we keep it as is but just add a subtle underline indicator to the contrast info, that will be fine for now.

Comment 31

[Maliha Islam [:mislam]]

Attachment: max-score-large-text-new-2

Comment 32

[Maliha Islam [:mislam]]

Attachment: info-button-hover-state-new-2

Comment 33

[Maliha Islam [:mislam]]

Victoria, thanks a lot for your feedback! I have made the changes you suggested. We have decided to shorten the title attr for the info button to "Learn more" to reuse localization string and be consistent with the accessibility panel. Let me know if there is any final thing I need to update :)

Comment 34

[Victoria Wang [:victoria]]

These screenshots look great and that sounds good re: the alt text! Thank you Maliha!

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/041f51c9d4d1
Move shared code to be used by color contrast feature, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/314855eab887
Add color contrast span to color picker, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/a363971fffb0
Add contrast ratio annotation title and info link to color picker, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/83a10c635fe9
Move shareable focus methods to be used for color picker accessibility, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/fcc9123589fd
Make color picker tooltip keyboard accessible, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/5ca694230ffa
Make color picker tooltip screen reader accessible, r=yzen,gl

Comment 36

[Narcis Beleuzu [:NarcisB]]

Backed out for dt failures on browser_html_tooltip_xul-wrapper.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/810ed823eef6829d90592ac05a1c0db54dd3acca
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256406786&repo=autoland&lineNumber=10216

Comment 37

[Maliha Islam [:mislam]]

Test fixed, new try link: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb4411bc8b60f08a8b7e90ae245a5adbc92e111c

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e16036e3719c
Move shared code to be used by color contrast feature, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/1a57ac83b59c
Add color contrast span to color picker, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/2bfcab598ff4
Add contrast ratio annotation title and info link to color picker, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/71c74b7d0e45
Move shareable focus methods to be used for color picker accessibility, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/c3bb4cfd04dc
Make color picker tooltip keyboard accessible, r=yzen,gl
https://hg.mozilla.org/integration/autoland/rev/83881815f007
Make color picker tooltip screen reader accessible, r=yzen,gl

Comment 39

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/e16036e3719c
https://hg.mozilla.org/mozilla-central/rev/1a57ac83b59c
https://hg.mozilla.org/mozilla-central/rev/2bfcab598ff4
https://hg.mozilla.org/mozilla-central/rev/71c74b7d0e45
https://hg.mozilla.org/mozilla-central/rev/c3bb4cfd04dc
https://hg.mozilla.org/mozilla-central/rev/83881815f007

Comment 40

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 9067377 [details]
Bug 1478156 - Move shared code to be used by color contrast feature, r=yzen

Beta/Release Uplift Approval Request

• User impact if declined: Users will not have a color contrast information available for text color pickers which would help accessibility effort.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): These patches include improves the way keyboard and screen reader users can use the color picker , some of the color picker components had to be substantially updated. This is all front end code within the devtools inspector panel (no platform changes)
• String changes made/needed: 6 strings added to devtools/client/locales/en-US/inspector.properties and 1 string added to devtools/client/locales/en-US/accessibility.properties in total.

Comment 41

[Ryan VanderMeulen [:RyanVM]]

Hi flod, are we OK taking this patch with string changes?

Comment 42

[Francesco Lodolo [:flod]]

I think at this point it's a question for Release Management to answer, more than l10n.

We keep getting in this situation (DevTools patches with strings in need of uplift, IIRC a few of them from accessibility), and I wonder what's the point of having a rule if we keep making exception to it. This looks like feature development (and the bug is over a year old), not bug fixing.

Last note: these strings landed less than 48h ago, they're not even exposed to localization yet, let alone localized.

Comment 43

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9067377 [details]
Bug 1478156 - Move shared code to be used by color contrast feature, r=yzen

I agree that this is pushing it, especially when we're already nearly halfway into the Beta cycle. Let's let this ride the trains.

Comment 44

[Janet Swisher]

dev-doc-complete, as color picker screenshots and related text have been updated.