Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Indented descriptions should use a lighter font color

RESOLVED FIXED in Firefox 56

Status

()

Firefox
Preferences
P1
normal
RESOLVED FIXED
7 months ago
4 days ago

People

(Reporter: jaws, Assigned: evanxd, Mentored)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 56
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox53 affected, firefox56 fixed)

Details

(Whiteboard: [photon-preference])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

From slide 12 and 13 of https://bugzilla.mozilla.org/attachment.cgi?id=8819509
Thought most of the feedback in that guideline was fair, but found little to justify this change - Font is smaller & already indented, however robbing it of the contrast to the background makes it harder to read.
Hopefully if this change does happen it wont be as light as their example.
Using the DevTools I checked the font-sizes and in what we are shipping the font is not smaller.

For the gray text we will use 'color: GrayText', which on my machine results in rgb(109,109,109). The background-color of the page is rgb(251,251,251). Using WCAG Contrast formula (simple web form available at http://webaim.org/resources/contrastchecker/), there is a 5:1 contrast ratio between the two colors. This contrast passes the Normal Text WCAG AA test and fails the WCAG AAA test. I haven't seen us require WCAG AAA in other parts of our UI.

I measured the font-size difference in the spec. The indented text is one pixel shorter in height from the screenshots, so it would be a subtle difference but shouldn't affect readability.
The spec says:
font-size: 13px
font-color: #858585
line-height: 21px
Assignee: nobody → manotejmeka
Mentor: mconley@mozilla.com, jaws@mozilla.com
Status: NEW → ASSIGNED
Assignee: manotejmeka → nobody
Status: ASSIGNED → NEW
Hey Jared, are we okay to proceed with the spec's colours, or should we go back to UX (or a11y) and have them evaluate?
Flags: needinfo?(jaws)
I think the colors here will be okay as far as contrast goes, after the examination I did in comment #2, but I am a bit worried that users may think this is disabled text and not supplemental text. Let's bring that up with UX.

Tina, what are your thoughts about people confusing the text for being disabled or not being applicable because they think it is disabled?
Flags: needinfo?(jaws) → needinfo?(thsieh)
Created attachment 8829304 [details]
Proposed font

Hey Jared,
I won't see it as a possibility to confuse people since the description has no checkbox in the front. The font color #858585 is accessible and can easily be distinguished from the option text.

I had a discussion with our visual designer Helen. She made an example that shows the comparison of the normal state, current disabled state, and the disabled state with lower opacity (30%). Making the opacity lower to show the differentiation might be a choice, but it's not accessible and different from the opacity defined in the current Firefox styleguide. 

Since I don't think the proposed font will confuse people and we need the option text to stand out from lines, I'll suggest us to use the proposed description font :)
Flags: needinfo?(thsieh)
Thank you, the lower opacity (30%) looks to have far too low contrast to be usable. We can use the "proposed font" color, #858585 and leave the font-size and line-height unchanged.

Comment 8

3 months ago
Hi Helen,
Kindly provide the final specs for this after your call tonight. 
Thanks!
Flags: needinfo?(hhuang)
Whiteboard: [photon-preference]

Updated

3 months ago
Flags: qe-verify+
QA Contact: hani.yacoub

Comment 9

3 months ago
Sorry for the late reply. 
Base on the latest color palette, I will change the font color to #737373, please help to fix this, thanks!
Flags: needinfo?(hhuang)

Updated

3 months ago
Priority: -- → P2
Target Milestone: --- → Firefox 57
This is a stlying follow up for re-org, not visual update for 57.
Target Milestone: Firefox 57 → Firefox 55

Updated

23 days ago
Target Milestone: Firefox 55 → Firefox 57
Status: NEW → RESOLVED
Last Resolved: 19 days ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1377167
remove whiteboard tag due to its duplicate
Whiteboard: [photon-preference]
Are you sure comment 10 is incorrect?
Flags: needinfo?(rchien)
Sorry I didn't look through this issue carefully! Let's ask Tina for confirmation.

Reorg v2 (bug 1365133) is ready to land in 56. This change mentioned in description was targeting on Reorg v1 in 55. 

Tina, can you confirm that this change will need to be shipped in Reorg v1 (55) or Reorg v2 (56) or both? Otherwise, does it cover by visual refresh spec in 57 so that we can do it with a sweep in 57 implementation?
Flags: needinfo?(rchien) → needinfo?(thsieh)
Hey Ricky,
The description font style is very important for visual hierarchy. The style we use in 57 is the same us what we proposed before, so there won't be a repetitive work. Let's implement in 56 or even earlier :)
Flags: needinfo?(thsieh)
Target Milestone: Firefox 57 → Firefox 56
Depends on: 1365133
Status: RESOLVED → REOPENED
No longer depends on: 1365133
Resolution: DUPLICATE → ---
Status: REOPENED → NEW
Hi Helen,

We'd like to get double confirmation here. Can you tell us that the proposed font for descriptions is same with the style we use in 57 and even it won't be a repetitive work as well, is that still recommended to land in 56?

Thanks
Flags: needinfo?(hhuang)
Whiteboard: [photon-preference]
Yes, the font style for descriptions in 57 is same as what we proposed before, and that will be great if it could land in 56.

font-size: 13px
font-color: #737373
Flags: needinfo?(hhuang)
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Priority: P2 → P1
(Assignee)

Updated

14 days ago
Duplicate of this bug: 1379549
(Assignee)

Updated

14 days ago
Assignee: rchien → evan
Comment hidden (mozreview-request)
(Assignee)

Updated

14 days ago
Attachment #8884756 - Flags: review?(jaws)
(Assignee)

Comment 20

14 days ago
Hi Jared,

Could you help review the patch?

Thank you.
(Reporter)

Comment 21

12 days ago
mozreview-review
Comment on attachment 8884756 [details]
Bug 1324172 - Add css styles for indented descriptions.

https://reviewboard.mozilla.org/r/155642/#review161434

::: browser/components/preferences/in-content-new/main.xul:908
(Diff revision 1)
> -    <description id="contentProcessCountEnabledDescription">&limitContentProcessOption.description;</description>
> -    <description id="contentProcessCountDisabledDescription">&limitContentProcessOption.disabledDescription;<label class="text-link" href="https://wiki.mozilla.org/Electrolysis">&limitContentProcessOption.disabledDescriptionLink;</label></description>
> +    <description id="contentProcessCountEnabledDescription" class="light">&limitContentProcessOption.description;</description>
> +    <description id="contentProcessCountDisabledDescription" class="light">&limitContentProcessOption.disabledDescription;<label class="text-link" href="https://wiki.mozilla.org/Electrolysis">&limitContentProcessOption.disabledDescriptionLink;</label></description>

Please undo these changes, see the other review comment for why.

::: browser/themes/shared/incontentprefs/preferences.inc.css:15
(Diff revision 1)
> +description.indent,
> +description.light {
> +  font-size: 1.18rem;
> +  color: #737373;

Please just use:

description.indent,
.indent > description {
  font-size: 1.18rem;
  color: #737373;
}
Attachment #8884756 - Flags: review?(jaws) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 24

5 days ago
Updated the patch for the review comments. Thank you for review, Jared.

Let's land the patch after the try[1] is good.

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a049fe4d7e1e
(Assignee)

Comment 25

5 days ago
Looks like the try is good. Land it.
Keywords: checkin-needed

Comment 26

4 days ago
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b940bc3aab48
Add css styles for indented descriptions. r=jaws
Keywords: checkin-needed

Comment 27

4 days ago
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.

Comment 28

4 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b940bc3aab48
Status: ASSIGNED → RESOLVED
Last Resolved: 19 days ago4 days ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
(Assignee)

Updated

4 days ago
Depends on: 1365133
You need to log in before you can comment on or make changes to this bug.