Improve color contrast for cached resource rows

RESOLVED FIXED in Firefox 63

Status

enhancement
P3
normal
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: fvsch, Assigned: fvsch)

Tracking

63 Branch
Firefox 63

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(3 attachments, 2 obsolete attachments)

Assignee

Description

10 months ago
In the table view, cached resources are grayed out. This is achieved with `opacity: 0.6` on the cell content, as far as I can tell.

In the light theme, this results in text-to-background contrast that is below the WCAG 2 recommended ratio (4.5:1), often around 3.0:1.

Contrasted enough:
- cached response on a pure white background

Not contrasted enough:
- cached response on a gray background (every other row)
- cached response on a blue background (when selected)

In particular, the selection style is originally #FFFFFF on #0074E8. This is an exact 4.5:1 contrast:
https://webaim.org/resources/contrastchecker/?fcolor=FFFFFF&bcolor=0074E8

This means that any partial opacity for the text will end up below 4.5:1, and should be avoided.

Proposed fix:
1. Change the "cached response" opacity to 0.7 or 0.8 instead of 0.6, to keep enough contrast.
2. The selected row should keep 1.0 opacity.
Assignee

Comment 1

10 months ago
Posted image network-cached-opacity.png (obsolete) —
Illustration of the proposed fixed (using the dark theme here):

1. Opacity at 0.7 instead of 0.6
2. Keep 1.0 opacity for the selected row

It's a CSS-only change in:

devtools/client/netmonitor/src/assets/styles/RequestList.css#588

.request-list-item.fromCache > .requests-list-column:not(.requests-list-waterfall) {
  opacity: 0.6;
}

to:

.request-list-item.fromCache:not(.selected) > .requests-list-column:not(.requests-list-waterfall) {
  opacity: 0.7;
}

Note that this is probably enough to have WCAG-compliant contrasts in the light mode, but not in the dark mode because the normal contrast is not compliant to begin with, event at 100% opacity and on the darker rows (#909090 on #2A2A2E = 4.48:1 contrast ratio). Still, it helps a bit.
Assignee

Comment 2

10 months ago
Just tested contrast ratios for the light theme.

At 0.6 opacity:

- 3.58:1 for white rows
- 3.20:1 for light grey rows

At 0.7 opacity:

- 4.73:1 for white rows
- 4.22:1 for light grey rows

Both are close enough to the 4.50:1 ratio recommended by WCAG2.
I also tried higher opacity (0.9 and 0.8) but then it's hard to notice the difference with normal rows.
Assignee

Comment 3

10 months ago
Changes:

- Keep full opacity for selected rows
- Bump opacity for cached rows from 0.6 to 0.7
Assignee

Comment 4

10 months ago
Comment on attachment 8993792 [details]
network-cached-opacity.png

Hi Victoria, two questions about this proposal:

1. Do you think raising the opacity slightly here (0.7 instead of 0.6) still works for differentiating "cached" rows from normal rows?
2. My intuition was that if a cached rows is selected, it should be full opacity because the user is more likely to want to check out the detailed information in the row and in that case it’s even more important to have accessible contrast. But then we’re losing the visual hint for "cached" (but the word is still visible). So it’s a trade-off. What do you think?
Attachment #8993792 - Flags: feedback?(victoria)
Comment on attachment 8993792 [details]
network-cached-opacity.png

Hi Florens, 

Wow, all of the dark mode Network table seems hard to read -- I'm glad you're working on this! Yes, your idea of full opacity for selected rows sounds great. 

For light mode, I usually use --gray-50 for secondary info, but the idea of bumping up the opacity on cached rows seems fine too.

For dark mode I would suggest changing the base unselected table text from --theme-body-color to --gray-40, to lighten all of that up.
Attachment #8993792 - Flags: feedback?(victoria) → feedback+
Assignee

Comment 6

10 months ago
Expanded the patch a bit per Victoria's feedback.

- Use grey-40 (#b1b1b3) for the dark theme table text, instead of the inherited #909090.
- Use 0.7 opacity instead of 0.6 for cached rows, to keep accessible contrasts in the light theme and not go too far from those in the dark theme.
- Keep full opacity for selected cached rows.
Attachment #8994168 - Attachment is obsolete: true
Attachment #8996136 - Flags: review?(gl)
Assignee

Comment 7

10 months ago
Results of the proposed patch (before/after composite screenshot).
Attachment #8993792 - Attachment is obsolete: true

Updated

10 months ago
Assignee: nobody → florens
Severity: normal → enhancement
Priority: -- → P3
Summary: Insufficient color contrast for cached resource rows → Improve color contrast for cached resource rows

Updated

10 months ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8996136 [details] [diff] [review]
bug-1476590-netmonitor-table-text-contrast.patch

Review of attachment 8996136 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/netmonitor/src/assets/styles/variables.css
@@ +22,4 @@
>  :root.theme-light {
>    --theme-body-color: var(--grey-70);
>  
> +  --table-text-color: var(--grey-70);

I am wondering if this should be grey-50 since this is what Victoria said. Can you check if this is ok with Victoria?

Updated

10 months ago
Flags: needinfo?(florens)

Updated

10 months ago
Flags: needinfo?(victoria)
Assignee

Comment 9

9 months ago
I think Victoria suggested grey-50 for the cached rows.

I did not use that exact value because lightening the cached rows is implemented with partial opacity, and impacts things like the [200] status badges (which use different colors). Changing the text color instead of using opacity would be an option if we want to keep full opacity for status badges.

What we have currently for the light theme:

- grey-70 for table rows (obtained by overriding --theme-body-color, which is normally grey-60)
- grey-70 at 0.6 opacity for cached rows

In my patch I bumped the "cached" opacity to 0.7 so that we can still pass WCAG2 guidelines (or come close enough).

I also assigned the grey-70 color for the light theme, and another contrasting color for the dark theme, to a new --table-text-color variable. The rationale is that it allows setting the table text color in the same ways for both themes, and that it would ultimately allows us to remove the --theme-body-color override. (IMO variables defined in devtools/client/themes/variables.css should not be overridden in a specific tool's views, because it makes it harder to reason about this shared design language.)
Flags: needinfo?(florens)
Attachment #8996136 - Flags: review?(gl) → review+
r+ assuming Victoria is also okay with the rationale from comment 9
Yep, sounds good! Thanks Florens!
Flags: needinfo?(victoria)

Comment 12

9 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/add7c894216a
Increase netmonitor table text contrast in dark theme and for cached rows. r=gl
Keywords: checkin-needed

Comment 13

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/add7c894216a
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.