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

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Brian Grinstead [:bgrins]]

Attachment: 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).

Comment 1

[Brian Grinstead [:bgrins]]

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

Comment 2

[Brian Grinstead [:bgrins]]

#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)

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: tabbar-splitter-color.png

Comparison of light/dark contrast with the splitter

Comment 4

[Brian Grinstead [:bgrins]]

Attachment: theme-splitter-color.patch

What do you think?

Comment 5

[Victor Porof [:vporof][:vp]]

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?

Comment 6

[Brian Grinstead [:bgrins]]

(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...

Comment 7

[Victor Porof [:vporof][:vp]]

(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.

Comment 8

[Brian Grinstead [:bgrins]]

(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.

Comment 9

[Victor Porof [:vporof][:vp]]

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.

Comment 10

[Brian Grinstead [:bgrins]]

Attachment: 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

Comment 11

[Brian Grinstead [:bgrins]]

Attachment: splitter-comparison.png

Here is an updated screenshot with the rgba(118, 121, 125, .5) splitter

Comment 12

[Victor Porof [:vporof][:vp]]

I like this.

Comment 13

[Brian Grinstead [:bgrins]]

Attachment: theme-splitter-color.patch

Updated the color for the devtools splitter

Comment 14

[Victor Porof [:vporof][:vp]]

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?

Comment 15

[Brian Grinstead [:bgrins]]

(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.

Comment 16

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/7e577607c404
https://tbpl.mozilla.org/?tree=Fx-Team&rev=7e577607c404

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7e577607c404