Update Dark theme colors

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Theme
P1
normal
VERIFIED FIXED
3 months ago
6 days ago

People

(Reporter: dao, Assigned: daleharvey)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed, firefox57 verified)

Details

(Whiteboard: [photon-visual][p2])

MozReview Requests

()

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

Attachments

(1 attachment)

(Reporter)

Description

3 months ago
See https://people-mozilla.org/~shorlander/projects/photon/Mockups/windows-10.html (Theme: Dark)

Updated

3 months ago
Flags: qe-verify+
Priority: -- → P2
QA Contact: brindusa.tot
Whiteboard: [photon-visual][p2][triage] → [photon-visual][p2]
Duplicate of this bug: 1094532
(Assignee)

Updated

2 months ago
Assignee: nobody → dharvey

Updated

2 months ago
Status: NEW → ASSIGNED
Iteration: --- → 56.2 - Jul 10
Priority: P2 → P1

Updated

a month ago
Iteration: 56.2 - Jul 10 → 56.3 - Jul 24

Comment 2

a month ago
I'd love to see this new dark theme implemented using the theming API!
(Reporter)

Comment 3

a month ago
(In reply to Tim Nguyen :ntim from comment #2)
> I'd love to see this new dark theme implemented using the theming API!

That's not going to be part of this bug.
Comment hidden (mozreview-request)
(Reporter)

Comment 5

a month ago
mozreview-review
Comment on attachment 8888344 [details]
Bug 1370929 - Update dark theme colours.

https://reviewboard.mozilla.org/r/159286/#review164714

::: browser/themes/shared/compacttheme.inc.css:23
(Diff revision 1)
>    --forwardbutton-width: 29px;
>  }
>  
>  :root:-moz-lwtheme-brighttext {
> +
> +  --chrome-color: #F5F7FA;

Please keep related variables like --chrome-background-color and --chrome-color together even if this means duplicting them in the photon and non-photon section.

::: browser/themes/shared/toolbarbutton-icons.inc.css:4
(Diff revision 1)
>  :root {
>    --toolbarbutton-icon-fill: #4c4c4c;
> +%ifdef MOZ_PHOTON_THEME
> +  --toolbarbutton-icon-fill-inverted: rgba(249, 249, 250, .7);

We need to keep using full white here for dark high contrast themes and dark lightweight themes with noisy backgrounds.

compacttheme.inc.css can set --toolbarbutton-icon-fill-inverted to rgba(249, 249, 250, .7) instead.
Attachment #8888344 - Flags: review?(dao+bmo) → review-

Updated

a month ago
Iteration: 56.3 - Jul 24 → 56.4 - Aug 1
Comment hidden (mozreview-request)
(Reporter)

Comment 7

28 days ago
mozreview-review
Comment on attachment 8888344 [details]
Bug 1370929 - Update dark theme colours.

https://reviewboard.mozilla.org/r/159286/#review167282

::: browser/themes/shared/compacttheme.inc.css:68
(Diff revision 2)
> +  --tab-hover-background-color: hsla(240, 9%, 98%, .1);
> +  --tab-selection-color: #f5f7fa;
> +  --tab-selection-background-color: hsl(240, 1%, 20%);
> +  --tab-selection-box-shadow: none;
> +
> +  --url-and-searchbar-background-color: hsla(0, 0%, 100%, .1);

Please move this back to the /* Url and search bars */ section. You can just add another ifdef there.
Attachment #8888344 - Flags: review?(dao+bmo) → review+
Comment hidden (mozreview-request)

Comment 9

28 days ago
Pushed by dharvey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/73981f510cd6
Update dark theme colours. r=dao

Comment 10

27 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/73981f510cd6
Status: ASSIGNED → RESOLVED
Last Resolved: 27 days ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Updated

27 days ago
Depends on: 1385277

Updated

16 days ago
Depends on: 1388394
QA Contact: brindusa.tot → ovidiu.boca
Hi Dale, the link from the description doesn't work, and also can you please tell us what we need to test and if the tests need to be on all OSes or only on Windows 10? Thanks
Flags: needinfo?(dharvey)

Comment 12

7 days ago
Here's the new URL: http://design.firefox.com/people/shorlander/photon/Mockups/windows-10.html
(Assignee)

Comment 13

7 days ago
Yup Tim provided the new URL, and this is across all OS's cheers
Flags: needinfo?(dharvey)
I compared the dark theme from the spec: https://mozilla.invisionapp.com/share/MRBK1MZF7#/screens/237333118 and the dark theme from the latest Nightly 57.0a1(2017-08-16) and they are not the same:

for example, the color that should be on the title tab or the tab buttons (B from the spec) - actual result #F9F2D4
                                                                           D from spec - actual result #323234
                                                                           F from spec - actual result #0069D9

I also attached a print screen with the build without the fix and the one with the fix and there are some visible changes but the colors don't respect the spec. Please tell me your opinion about this. Thanks
Flags: needinfo?(dharvey)
(Assignee)

Comment 15

7 days ago
So the only color that this changed in that list is the toolbar colour buttons (B from the spec), it is changed to rgba(249, 249, 250, .7) which matches the fill color of the svg used in http://design.firefox.com/people/shorlander/photon/Mockups/macOS.html combined with the opacity: .7 set on the icons. I think we are good here
Flags: needinfo?(dharvey)
Cool, thanks. I will mark this as verified fixed.
Status: RESOLVED → VERIFIED
status-firefox57: --- → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.