Adding color contrast information to the color picker
Categories
(DevTools :: Inspector, enhancement, P3)
Tracking
(firefox70 fixed)
Tracking | Status | |
---|---|---|
firefox70 | --- | fixed |
People
(Reporter: mtigley, Assigned: mislam, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(19 files, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details |
Updated•6 years ago
|
Comment 1•6 years ago
|
||
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
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Comment 2•6 years ago
|
||
Assignee | ||
Comment 3•5 years ago
|
||
Assignee | ||
Comment 4•5 years ago
|
||
Depends on D32490
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 5•5 years ago
|
||
Depends on D32491
Assignee | ||
Comment 6•5 years ago
|
||
Depends on D32842
Comment 7•5 years ago
|
||
So excited to see this work happening! The original mockups should still be accurate :) but let me know if any new questions come up.
Assignee | ||
Comment 8•5 years ago
|
||
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
Assignee | ||
Comment 9•5 years ago
|
||
Assignee | ||
Comment 10•5 years ago
|
||
Comment 11•5 years ago
|
||
(In reply to Maliha Islam [:mislam] from comment #8)
Created attachment 9070032 [details]
Annotation CollapsedHi 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.
Assignee | ||
Comment 12•5 years ago
|
||
Depends on D32842
Updated•5 years ago
|
Comment 13•5 years ago
|
||
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
Assignee | ||
Comment 14•5 years ago
|
||
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.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 15•5 years ago
|
||
Depends on D33331
Assignee | ||
Comment 16•5 years ago
|
||
Assignee | ||
Comment 17•5 years ago
|
||
Assignee | ||
Comment 18•5 years ago
|
||
Assignee | ||
Comment 19•5 years ago
|
||
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•5 years ago
|
||
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.
- 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.
-
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.
-
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•5 years ago
|
||
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•5 years ago
|
||
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?
Updated•5 years ago
|
Assignee | ||
Comment 23•5 years ago
|
||
Assignee | ||
Comment 24•5 years ago
|
||
Assignee | ||
Comment 25•5 years ago
|
||
Assignee | ||
Comment 26•5 years ago
|
||
Assignee | ||
Comment 27•5 years ago
|
||
:victoria I have attached a few screenshots of the updated tooltip. Let me know if you have any questions :)
Assignee | ||
Updated•5 years ago
|
Comment 28•5 years ago
|
||
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•5 years ago
|
||
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•5 years ago
|
||
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.
Assignee | ||
Comment 31•5 years ago
|
||
Assignee | ||
Comment 32•5 years ago
|
||
Assignee | ||
Comment 33•5 years ago
|
||
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•5 years ago
|
||
These screenshots look great and that sounds good re: the alt text! Thank you Maliha!
Comment 35•5 years ago
|
||
Comment 36•5 years ago
|
||
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
Assignee | ||
Comment 37•5 years ago
|
||
Test fixed, new try link: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb4411bc8b60f08a8b7e90ae245a5adbc92e111c
Comment 38•5 years ago
|
||
Comment 39•5 years ago
|
||
bugherder |
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•5 years ago
|
||
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.
Updated•5 years ago
|
Comment 41•5 years ago
|
||
Hi flod, are we OK taking this patch with string changes?
Comment 42•5 years ago
|
||
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•5 years ago
|
||
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.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 44•5 years ago
|
||
dev-doc-complete, as color picker screenshots and related text have been updated.
Description
•