Closed Bug 1377167 Opened 7 years ago Closed 7 years ago

Update font & size & color & background color to match the Photon preferences spec

Categories

(Firefox :: Settings UI, defect, P1)

Product:
Firefox
Component:
Settings UI
Version:
55 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 57
Tracking Flags:
Tracking Status
firefox57 --- verified

People

(Reporter: rickychien, Assigned: evanxd)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [photon-preference])

Attachments

(1 file)

Bug 1377167 - Update font size, font color, and background color to match the Photon preferences visual refresh spec.
59 bytes, text/x-review-board-request
mconley
: review+
Details 
      No description provided.
Flags: qe-verify+
QA Contact: jwilliams
QA Contact: jwilliams → hani.yacoub
Assignee: nobody → evan
Status: NEW → ASSIGNED
Priority: P2 → P1
Attachment #8890240 - Flags: review?(mconley)
Hi Mike,

Could you help review the patch?

Thank you.
Comment on attachment 8890240 [details]
Bug 1377167 - Update font size, font color, and background color to match the Photon preferences visual refresh spec.

https://reviewboard.mozilla.org/r/161356/#review169554

Evan, please add MOZ_PHOTON_PREFERENCES flag when introducing new Photon style changes.

#ifdef MOZ_PHOTON_PREFERENCES in XUL

%ifdef MOZ_PHOTON_PREFERENCES in CSS

You can reference my patch https://reviewboard.mozilla.org/r/160914/diff/23/ as well. Thanks
Flags: needinfo?(evan)
Thanks, Ricky. Updated.
Flags: needinfo?(evan)
Comment on attachment 8890240 [details]
Bug 1377167 - Update font size, font color, and background color to match the Photon preferences visual refresh spec.

https://reviewboard.mozilla.org/r/161356/#review169926

I didn't do a pixel-by-pixel comparison with the spec, but it certainly looks right from a quick pass. Code looks good too - just one nit below. Thanks!

::: browser/themes/shared/incontentprefs/preferences.inc.css:768
(Diff revision 7)
> +.category-name {
> +  font-size: 1.45rem;
> +  color: #0c0c0d;
> +}
> +
> +.category, .category:hover, .category[selected] {

Nit: I think in general, we like to break up separate selectors over separate lines. Can you break these up over 3 lines, please?
Attachment #8890240 - Flags: review?(mconley) → review+
Comment on attachment 8890240 [details]
Bug 1377167 - Update font size, font color, and background color to match the Photon preferences visual refresh spec.

https://reviewboard.mozilla.org/r/161356/#review170208

::: browser/themes/shared/incontentprefs/preferences.inc.css:768
(Diff revision 7)
> +.category-name {
> +  font-size: 1.45rem;
> +  color: #0c0c0d;
> +}
> +
> +.category, .category:hover, .category[selected] {

Sure, let's do it.
Updated patch to fix font-weight issues. Let's land it after the try[1] is good.

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2eeec89f035f
The try is good. Let's land it.
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c3910ce15bbb
Update font size, font color, and background color to match the Photon preferences visual refresh spec. r=mconley
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c3910ce15bbb
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
What about the other in-content pages? Shouldn't they fit too?
According to my understanding, other in-content pages will be updated except about:addons page.
Depends on: 1388414
(In reply to Evan Tseng [:evanxd] from comment #23)
> According to my understanding, other in-content pages will be updated except
> about:addons page.

Black text 'Inactive tab' and 'Blue text - Active tab' is hard to read on the dark gray background.  Those people with vision impairments are likely to have issues.  This IMO needs to be re-thought about not changing the about:addons page
Depends on: 1388761
Depends on: 1391325
Build ID: 20170829100404
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.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: