DevTools Themes: in the light theme, the sidebar tab separators are too dark

VERIFIED FIXED in Firefox 30

Status

DevTools
General
VERIFIED FIXED
5 years ago
a month ago

People

(Reporter: u443210, Assigned: bgrins)

Tracking

Trunk
Firefox 30
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 8372326 [details]
Bug.png

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140207053801



Actual results:

See the screenshot.
(Reporter)

Updated

5 years ago
Component: Untriaged → Developer Tools
Status: UNCONFIRMED → NEW
status-firefox30: --- → affected
tracking-firefox30: --- → ?
Ever confirmed: true

Updated

5 years ago
Flags: needinfo?(bgrinstead)

Updated

5 years ago
Blocks: 966661
(Assignee)

Comment 1

5 years ago
Yes they are.  They should match: https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/LightTheme-Inspector-Locked@2x.png
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Depends on: 957117
Flags: needinfo?(bgrinstead)
(Assignee)

Updated

5 years ago
OS: Windows XP → All
Hardware: x86 → All
Summary: [Devtools] In the light-theme, the panel's separators are to dark. → DevTools Themes: in the light theme, the sidebar tab separators are too dark
(Assignee)

Comment 2

5 years ago
Created attachment 8373423 [details] [diff] [review]
light-theme-sidebar-separators.patch

A patch that updates the light theme sidebar tab separators to match the separators on the top tab bar (using splitter color, #aaa)
Attachment #8373423 - Flags: review?(pbrosset)
(Assignee)

Comment 3

5 years ago
Created attachment 8373424 [details]
Screenshot 2014-02-10 10.55.09.png

Screenshot showing current UI (left) with the patch applied (right)

Comment 4

5 years ago
(In reply to Brian Grinstead [:bgrins] from comment #3)
> Created attachment 8373424 [details]
> Screenshot 2014-02-10 10.55.09.png
> 
> Screenshot showing current UI (left) with the patch applied (right)

Nice ! I'd remove the seperators from the selected tab since they are invisible in the mockup though.
(Assignee)

Comment 5

5 years ago
(In reply to Tim Nguyen [:ntim] from comment #4)
> (In reply to Brian Grinstead [:bgrins] from comment #3)
> > Created attachment 8373424 [details]
> > Screenshot 2014-02-10 10.55.09.png
> > 
> > Screenshot showing current UI (left) with the patch applied (right)
> 
> Nice ! I'd remove the seperators from the selected tab since they are
> invisible in the mockup though.

Actually, there is a (light) splitter next to the selected tab if you magnify it.  The colors are a bit different, but this patch just sticks with the conventions that we used for the light theme during implementation (#aaa).
(Assignee)

Updated

5 years ago
status-firefox30: affected → ---
tracking-firefox30: ? → ---
Version: 30 Branch → Trunk
Comment on attachment 8373423 [details] [diff] [review]
light-theme-sidebar-separators.patch

Review of attachment 8373423 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

::: browser/themes/shared/devtools/toolbars.inc.css
@@ +6,5 @@
>  %filter substitution
> +%define smallSeparatorDark linear-gradient(transparent 15%, #5a6169 15%, #5a6169 85%, transparent 85%)
> +%define smallSeparatorLight linear-gradient(transparent 15%, #aaa 15%, #aaa 85%, transparent 85%)
> +%define solidSeparatorDark linear-gradient(#2d5b7d, #2d5b7d)
> +%define solidSeparatorLight linear-gradient(#aaa, #aaa)

Just out of curiosity, do we use these %define statements just as a local stylesheet var mechanism?
Or can they be accessed from other included *.inc.css files?
Attachment #8373423 - Flags: review?(pbrosset) → review+
(Assignee)

Comment 7

5 years ago
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #6)
> Comment on attachment 8373423 [details] [diff] [review]
> light-theme-sidebar-separators.patch
> 
> Review of attachment 8373423 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> LGTM
> 
> ::: browser/themes/shared/devtools/toolbars.inc.css
> @@ +6,5 @@
> >  %filter substitution
> > +%define smallSeparatorDark linear-gradient(transparent 15%, #5a6169 15%, #5a6169 85%, transparent 85%)
> > +%define smallSeparatorLight linear-gradient(transparent 15%, #aaa 15%, #aaa 85%, transparent 85%)
> > +%define solidSeparatorDark linear-gradient(#2d5b7d, #2d5b7d)
> > +%define solidSeparatorLight linear-gradient(#aaa, #aaa)
> 
> Just out of curiosity, do we use these %define statements just as a local
> stylesheet var mechanism?
> Or can they be accessed from other included *.inc.css files?

They can be accessed by other files that are directly included by the file in which it was declared.  Attempting to switch to global declarations in light and dark theme for color variables, for instance, does not work because most of the panels are not loaded directly from the theme files (though it would work for propagating these vars into toolbars.inc.css, since each of the theme files include it.
https://hg.mozilla.org/mozilla-central/rev/ed39fdfb0fd0
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 30
(Assignee)

Updated

5 years ago
Blocks: 971186
Verified with latest build of Nightly
Status: RESOLVED → VERIFIED

Updated

a month ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.