Add preview for a11y highlighter's colour contrast indicator.

RESOLVED FIXED in Firefox 65

Status

enhancement
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: yzen, Assigned: yzen)

Tracking

unspecified
Firefox 66
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox65 fixed, firefox66 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

6 months ago
We would like make colour contrast information more contextual within the highlighter tool. We currently only display the value and the pass/failure indicator. To give more context to where the value is coming from we are adding a preview for the colours used to calculate the contrast values.
Assignee

Comment 1

6 months ago
Posted image Victoria's design.
Assignee

Comment 3

5 months ago
MozReview-Commit-ID: 7AAiOIbCfSF

Depends on D14429
Assignee

Comment 4

5 months ago
FYI the latest version is based on Victoria's feedback.

Comment 5

5 months ago
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/294a79174a73
adding overlapping swatch preview for colour contrast indicator in a11y panel. r=pbro
https://hg.mozilla.org/integration/autoland/rev/47727ca2a559
switch from flexbox to grid for a11y highlighter inforbar to support multi-row. r=pbro
Assignee

Comment 6

5 months ago
Comment on attachment 9031142 [details]
Bug 1513557 - adding overlapping swatch preview for colour contrast indicator in a11y panel. r=pbro

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1505848

User impact if declined: Improves the UX for the above feature and addresses all UX polish concerns.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Adds small changes to the colour contrast preview in a11y highlighter in devtools. Small CSS changes mostly.

String changes made/needed: accessibility.contrast.ratio.label2, accessibility.contrast.ratio.large in devtools/shared/locales/en-US/accessibility.properties
Attachment #9031142 - Flags: approval-mozilla-beta?
Assignee

Comment 7

5 months ago
Comment on attachment 9031143 [details]
Bug 1513557 - switch from flexbox to grid for a11y highlighter inforbar to support multi-row. r=pbro

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1505848

User impact if declined: a11y colour contrast preview becomes too busy for users and is especially problematic in Responsive Design Mode.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): CSS changes only to change the layout of the a11y highlighter infobar.

String changes made/needed: none
Attachment #9031143 - Flags: approval-mozilla-beta?

Comment 9

5 months ago
Backout by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0749d8b0f6d9
Backed out 2 changesets for linting failure

Comment 10

5 months ago
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f323d2a186b2
adding overlapping swatch preview for colour contrast indicator in a11y panel. r=pbro
https://hg.mozilla.org/integration/autoland/rev/7800d0bb347b
switch from flexbox to grid for a11y highlighter inforbar to support multi-row. r=pbro

Comment 11

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f323d2a186b2
https://hg.mozilla.org/mozilla-central/rev/7800d0bb347b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
It would be great to start using phabricator, so that I can catch these changes before they land. If nothing else, pinging someone from localization to double check would be good.

accessibility.contrast.ratio.label2=Contrast%S:
accessibility.contrast.ratio.large=\u0020(large text)

What's the point of creating two strings for this

accessibility.contrast.ratio.label2=Contrast%S:
accessibility.contrast.ratio.large=\u0020(large text)

When this would still be two strings (and only one added), and much clearer?

accessibility.contrast.ratio.label=Contrast:
accessibility.contrast.ratio.large=Contrast (large text):

Clearly not happy about an uplift to beta like this.
(In reply to Francesco Lodolo [:flod] from comment #12)
> Clearly not happy about an uplift to beta like this.

To be extremely clear, I would prefer for this to be backed out and land with the correct string changes, because the l10n part is really poor, and landing a follow-up would be quite confusing for people following the history of the file.
Comment on attachment 9031142 [details]
Bug 1513557 - adding overlapping swatch preview for colour contrast indicator in a11y panel. r=pbro

Clearing the approval requests pending the follow-up work requested by Flod.
Attachment #9031142 - Flags: approval-mozilla-beta?
Attachment #9031143 - Flags: approval-mozilla-beta?
Assignee

Comment 15

5 months ago
Ryan, do I just push a backout commit to inbound or can it be backed out directly by someone? Thanks.
Flags: needinfo?(ryanvm)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 66 → ---
Flags: needinfo?(ryanvm)
Assignee

Comment 17

5 months ago
Francesco, I marked you for review to make sure it's all good. Thanks.
Flags: needinfo?(yzenevich) → needinfo?(francesco.lodolo)
Reviewed, thanks for fixing. 

I don't have an issue in uplifting, but I'd like to understand why we have so many from devtools, when their target build (Developer Edition) has been around for a while at this point.
Flags: needinfo?(francesco.lodolo)

Comment 19

5 months ago
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/796051b1e67e
adding overlapping swatch preview for colour contrast indicator in a11y panel. r=pbro,flod
https://hg.mozilla.org/integration/autoland/rev/e8de3ae7a738
switch from flexbox to grid for a11y highlighter inforbar to support multi-row. r=pbro
Assignee

Comment 20

5 months ago
(In reply to Francesco Lodolo [:flod] from comment #18)
> Reviewed, thanks for fixing. 
> 
> I don't have an issue in uplifting, but I'd like to understand why we have
> so many from devtools, when their target build (Developer Edition) has been
> around for a while at this point.

From telemetry, release usage is about 30 times higher than DevEdition. This particular feature was also mentioned on Twitter (devtools account) and we really want to have all the polish.

Comment 21

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/796051b1e67e
https://hg.mozilla.org/mozilla-central/rev/e8de3ae7a738
Status: REOPENED → RESOLVED
Last Resolved: 5 months ago5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Assignee

Comment 22

5 months ago
Comment on attachment 9031142 [details]
Bug 1513557 - adding overlapping swatch preview for colour contrast indicator in a11y panel. r=pbro

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1505848

User impact if declined: Improves the UX for the above feature and addresses all UX polish concerns.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Adds small changes to the colour contrast preview in a11y highlighter in devtools. Small CSS changes mostly.

String changes made/needed: accessibility.contrast.ratio.label.large in devtools/shared/locales/en-US/accessibility.properties
Attachment #9031142 - Flags: approval-mozilla-beta?
Assignee

Comment 23

5 months ago
Comment on attachment 9031143 [details]
Bug 1513557 - switch from flexbox to grid for a11y highlighter inforbar to support multi-row. r=pbro

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1505848

User impact if declined: a11y colour contrast preview becomes too busy for users and is especially problematic in Responsive Design Mode.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): CSS changes only to change the layout of the a11y highlighter infobar.

String changes made/needed: None
Attachment #9031143 - Flags: approval-mozilla-beta?
Comment on attachment 9031142 [details]
Bug 1513557 - adding overlapping swatch preview for colour contrast indicator in a11y panel. r=pbro

[Triage Comment]
Improves the UX for a11y highlighter shipping in 65. String changes approved by Flod. Approved for 65.0b5.
Attachment #9031142 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9031143 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.