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

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?

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?

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 :)

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.

Hi Helen, Kindly provide the final specs for this after your call tonight. Thanks!

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!

This is a stlying follow up for re-org, not visual update for 57.

remove whiteboard tag due to its duplicate

Are you sure comment 10 is incorrect?

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?

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 :)

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

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

Hi Jared, Could you help review the patch? Thank you.

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; }

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

Looks like the try is good. Land it.

Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b940bc3aab48 Add css styles for indented descriptions. r=jaws

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.

Build ID: 20170817100132 User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 Verified as fixed on Firefox Nightly 57.0a1 on Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.