DevTools Themes - Change color of main devtools splitter to better match both themes

RESOLVED FIXED in Firefox 29

Status

()

Firefox
Developer Tools
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

Trunk
Firefox 29
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first verify])

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Description

4 years ago
Created attachment 8368566 [details]
splitter-on-dark-theme.png

It used to be black, now it is #aaa after Bug 957117.  Anything that has too much of a contrast looks weird on either theme, but we can't use different colors based on the theme, so we need to come up with a compromise.

#aaa looks great on all backgrounds for the light theme, but it looks quite bright on a dark background with the dark theme (see screenshot).
(Assignee)

Comment 1

4 years ago
The light theme tab toolbar is #ebeced, and the dark theme is #252c33, so we want something that works against both of these colors.
(Assignee)

Comment 2

4 years ago
#42484f is ideal on dark theme (it matches the border colors of the tabs), while #aaa is ideal on light theme (matches border colors of tabs).  The midpoint of these two colors is #76797D - it seems to look alright (screenshot and patch incoming)
(Assignee)

Comment 3

4 years ago
Created attachment 8368569 [details]
tabbar-splitter-color.png

Comparison of light/dark contrast with the splitter
(Assignee)

Comment 4

4 years ago
Created attachment 8368573 [details] [diff] [review]
theme-splitter-color.patch

What do you think?
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Attachment #8368573 - Flags: review?(vporof)
I think this is still much too light on a dark background. Why not just make the splitter invisible and add a colored border on the top of the tabbar?
Attachment #8368573 - Flags: review?(vporof)
(Assignee)

Comment 6

4 years ago
(In reply to Victor Porof [:vp] from comment #5)
> I think this is still much too light on a dark background. Why not just make
> the splitter invisible and add a colored border on the top of the tabbar?

We could do this for the BottomHost, but it wouldn't work for the SidebarHost, and right now there is no way to target CSS specifically for the host type.  Maybe there is a way in toolbox-hosts.js to do this?  I can check...
(In reply to Brian Grinstead [:bgrins] from comment #6)

You could use the 700 media query. It'll work 95% of the time, and this is a pretty small UI nit for users to be really nitpicky.
(Assignee)

Comment 8

4 years ago
(In reply to Victor Porof [:vp] from comment #7)
> (In reply to Brian Grinstead [:bgrins] from comment #6)
> 
> You could use the 700 media query. It'll work 95% of the time, and this is a
> pretty small UI nit for users to be really nitpicky.

I'm not a fan of this.  If it is wrong, then there will be no splitter visible and the content will be shifted in a weird way (there will be a border moved to the left of a bottom host, for instance).  Plus, I don't think it would be that uncommon to have it set to bottom host and < 700px if the browser was not taking up the full screen.

I'd say we should either pick a neutral color that works for both themes, or spend the time to properly notify the toolbox UI of which host it is in.  Honestly, I don't think the current color in the patch is that bad (especially when you see it next to the command icons, which appear just as light).  I'd be open to try out some different colors though.
What do you say about simply having a transparent splitter? :) (just a thought) Naturally users think they can resize the toolbox. No need for a necessarily colored line for this, if the cursor changes accordingly all is good.
(Assignee)

Comment 10

4 years ago
Created attachment 8368758 [details]
neutral-with-alpha.png

(In reply to Victor Porof [:vp] from comment #9)
> What do you say about simply having a transparent splitter? :) (just a
> thought) Naturally users think they can resize the toolbox. No need for a
> necessarily colored line for this, if the cursor changes accordingly all is
> good.

I tried transparent.. it looks a little weird then for light them / light bg and dark theme / dark bg.  What about if we do the midpoint color with alpha?  This screenshot shows rgba(118, 121, 125, .5)
Or we could
(Assignee)

Comment 11

4 years ago
Created attachment 8368791 [details]
splitter-comparison.png

Here is an updated screenshot with the rgba(118, 121, 125, .5) splitter
Attachment #8368569 - Attachment is obsolete: true
Attachment #8368758 - Attachment is obsolete: true
I like this.
(Assignee)

Comment 13

4 years ago
Created attachment 8368808 [details] [diff] [review]
theme-splitter-color.patch

Updated the color for the devtools splitter
Attachment #8368573 - Attachment is obsolete: true
Attachment #8368808 - Flags: review?(vporof)
Comment on attachment 8368808 [details] [diff] [review]
theme-splitter-color.patch

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

::: browser/themes/shared/devtools/light-theme.css
@@ +307,5 @@
>    border-bottom: 0;
>  }
>  
> +.devtools-horizontal-splitter {
> +  border-bottom: 1px solid #aaa;

Shouldn't there be corresponding rules in the dark theme?
Attachment #8368808 - Flags: review?(vporof) → review+
(Assignee)

Comment 15

4 years ago
(In reply to Victor Porof [:vp] from comment #14)
> Comment on attachment 8368808 [details] [diff] [review]
> theme-splitter-color.patch
> 
> Review of attachment 8368808 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/shared/devtools/light-theme.css
> @@ +307,5 @@
> >    border-bottom: 0;
> >  }
> >  
> > +.devtools-horizontal-splitter {
> > +  border-bottom: 1px solid #aaa;
> 
> Shouldn't there be corresponding rules in the dark theme?

There already is for dark theme: https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/dark-theme.css#311.  The light theme didn't yet have them since it was the same color for both.
(Assignee)

Comment 16

4 years ago
https://hg.mozilla.org/integration/fx-team/rev/7e577607c404
https://tbpl.mozilla.org/?tree=Fx-Team&rev=7e577607c404
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/7e577607c404
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29

Updated

4 years ago
Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.