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

VERIFIED FIXED in Firefox 57

Status

()

P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: rickychien, Assigned: evanxd)

Tracking

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

55 Branch
Firefox 57
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox57 verified)

Details

(Whiteboard: [photon-preference])

Attachments

(1 attachment)

Comment hidden (empty)
Flags: qe-verify+
QA Contact: jwilliams
QA Contact: jwilliams → hani.yacoub
Duplicate of this bug: 1324172
(Assignee)

Updated

2 years ago
Assignee: nobody → evan
Status: NEW → ASSIGNED
Priority: P2 → P1
Duplicate of this bug: 1382515
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8890240 - Flags: review?(mconley)
(Assignee)

Comment 8

2 years ago
Hi Mike,

Could you help review the patch?

Thank you.
(Reporter)

Comment 9

2 years ago
mozreview-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/#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)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 12

2 years ago
Thanks, Ricky. Updated.
Flags: needinfo?(evan)

Comment 13

2 years ago
mozreview-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/#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+
(Assignee)

Comment 14

2 years ago
mozreview-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.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 18

2 years ago
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
(Assignee)

Comment 19

2 years ago
The try is good. Let's land it.
Keywords: checkin-needed

Comment 20

2 years ago
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

Comment 21

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c3910ce15bbb
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
What about the other in-content pages? Shouldn't they fit too?
(Assignee)

Comment 23

2 years ago
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

Updated

2 years ago
Depends on: 1391325

Comment 25

2 years ago
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.