Closed Bug 1451947 Opened 2 years ago Closed 2 years ago

Plain black is too dark for the arrow panels

Categories

(Firefox :: Theme, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: ntim, Assigned: ntim)

References

Details

Attachments

(1 file)

This is common criticism about the themed panels: Plain black is not pleasant to the eye.
Stephen, can we opt for a lighter black/gray? If so, which color should we use?

Note that we can also color the border if needed.
Flags: needinfo?(shorlander)
Yes! Besides being a little harsh, using a darker color for overlays breaks the visual z-ordering we created by going from lighter to darker  (e.g. URL Bar --> tabs / toolbars --> window)

Here is a spec that goes from light to dark:
https://mozilla.invisionapp.com/share/J6GQCZR2PZY#/289803032_Desktop_Common

Colors pulled from the Design System color palette:
https://design.firefox.com/photon/visuals/color.html#dark-theme
Flags: needinfo?(shorlander)
Assignee: nobody → ntim.bugs
Comment on attachment 8965883 [details]
Bug 1451947 - Tweak dark theme popup styles.

https://reviewboard.mozilla.org/r/234706/#review240376


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/parent/ext-theme.js:99
(Diff revision 1)
>          defaultTheme = this;
>        }
>        onUpdatedEmitter.emit("theme-updated", this.details, this.windowId);
>  
>        LightweightThemeManager.fallbackThemeData = this.lwtStyles;
> +      //LightweightThemeManager.currentTheme = null;

Error: Expected space or tab after '//' in comment. [eslint: spaced-comment]
Comment on attachment 8965883 [details]
Bug 1451947 - Tweak dark theme popup styles.

https://reviewboard.mozilla.org/r/234706/#review240392
Attachment #8965883 - Flags: review?(jaws) → review+
Comment on attachment 8965883 [details]
Bug 1451947 - Tweak dark theme popup styles.

https://reviewboard.mozilla.org/r/234706/#review240394

The shortcuts aren't readable with these colors. -arrowpanel-dimmed-even-further looks best if we use it at https://searchfox.org/mozilla-central/rev/2ce99e8054b0ff6ed1adf484aeaacacf2fea084c/browser/themes/shared/customizableui/panelUI.inc.css#1015 instead of --arrowpanel-dimmed
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> The shortcuts aren't readable with these colors _when hovered_*
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> Comment on attachment 8965883 [details]
> Bug 1451947 - Tweak dark theme popup styles.
> 
> https://reviewboard.mozilla.org/r/234706/#review240394
> 
> The shortcuts aren't readable with these colors.
> -arrowpanel-dimmed-even-further looks best if we use it at
> https://searchfox.org/mozilla-central/rev/
> 2ce99e8054b0ff6ed1adf484aeaacacf2fea084c/browser/themes/shared/
> customizableui/panelUI.inc.css#1015 instead of --arrowpanel-dimmed

That's bug 1451944.
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/93ad00eae574
Tweak dark theme popup styles. r=jaws
https://hg.mozilla.org/mozilla-central/rev/93ad00eae574
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Depends on: 1454331
Depends on: 1463720
No longer depends on: 1463720
Depends on: 1530632
Depends on: 1540529
No longer depends on: 1540529
Regressions: 1540529
No longer regressions: 1540529
You need to log in before you can comment on or make changes to this bug.