Update Dark theme colors

VERIFIED FIXED in Firefox 56

Status

()

P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: dao, Assigned: daleharvey)

Tracking

(Blocks: 1 bug)

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

Firefox Tracking Flags

(firefox56 fixed, firefox57 verified)

Details

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

Attachments

(1 attachment)

Flags: qe-verify+
Priority: -- → P2
QA Contact: brindusa.tot
Whiteboard: [photon-visual][p2][triage] → [photon-visual][p2]
(Assignee)

Updated

2 years ago
Assignee: nobody → dharvey
Status: NEW → ASSIGNED
Iteration: --- → 56.2 - Jul 10
Priority: P2 → P1
Iteration: 56.2 - Jul 10 → 56.3 - Jul 24

Comment 2

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

Comment 3

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

2 years 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-
Iteration: 56.3 - Jul 24 → 56.4 - Aug 1
Comment hidden (mozreview-request)
(Reporter)

Comment 7

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

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

Updated

2 years ago
Depends on: 1385277

Updated

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

Comment 13

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

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