Closed Bug 1370929 Opened 3 years ago Closed 3 years ago

Update Dark theme colors

Categories

(Firefox :: Theme, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 56
Iteration:
56.4 - Aug 1
Tracking Status
firefox56 --- fixed
firefox57 --- verified

People

(Reporter: dao, Assigned: daleharvey)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

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

Attachments

(1 file)

Flags: qe-verify+
Priority: -- → P2
QA Contact: brindusa.tot
Whiteboard: [photon-visual][p2][triage] → [photon-visual][p2]
Assignee: nobody → dharvey
Status: NEW → ASSIGNED
Iteration: --- → 56.2 - Jul 10
Priority: P2 → P1
Iteration: 56.2 - Jul 10 → 56.3 - Jul 24
I'd love to see this new dark theme implemented using the theming API!
(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 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-
Iteration: 56.3 - Jul 24 → 56.4 - Aug 1
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+
https://hg.mozilla.org/mozilla-central/rev/73981f510cd6
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Depends on: 1385277
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)
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)
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
Flags: qe-verify+
Regressions: 1567408
You need to log in before you can comment on or make changes to this bug.