683906 - Make the Highlighter Toolbar look like shorlander's design

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Paul Rouget [:paul]]

We may want to use OSX design (bug 676253) or Windows design (Bug 676255).

Comment 1

[Paul Rouget [:paul]]

Attachment: patch v1 WIP

Comment 2

[Mihai Sucan [:msucan]]

Attachment: patch v1 - screenshot - ubuntu 10.04

This is Ubuntu 10.04 LTS, clearlooks theme, gnome 2.30, gtk2.

The various Inspect button states:

http://img.i7m.de/show/nxwuk.png

Third state, mouse over, does not look too good for now.

Comment 3

[Paul Rouget [:paul]]

Thank you Mihai.

Comment 4

[Paul Rouget [:paul]]

Attachment: patch v1

Based on Mac design (bug 676253).

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 559097 [details] [diff] [review]
patch v1

>+#inspector-toolbar {
>+  -moz-appearance: none;
>+  height: 32px;

Why are you setting a height here rather than letting the toolbar be sized automatically based on its contents?

>+#inspector-inspect-toolbutton,
>+#inspector-tools > toolbarbutton {
>+  -moz-appearance: none;
>+  width: 78px;

I think you want min-width here.

>+  margin: 3px 5px !important;

Why !important?

>+#inspector-inspect-toolbutton[checked],
>+#inspector-tools > toolbarbutton[checked] {
>+  color: hsl(208,100%,60%) !important;
>+  border-color: hsla(210,8%,5%,.6) !important;
>+  background: -moz-linear-gradient(hsla(220,6%,10%,.6), hsla(210,11%,18%,.45) 75%, hsla(210,11%,30%,.4));
>+  box-shadow: 0 1px 3px hsla(210,8%,5%,.25) inset, 0 1px 3px hsla(210,8%,5%,.25) inset, 0 1px 0 hsla(210,16%,76%,.15);
>+}
>+
>+#inspector-inspect-toolbutton[checked]:hover,
>+#inspector-tools > toolbarbutton[checked]:hover {
>+  background-color: transparent!important;
>+}
>+
>+#inspector-inspect-toolbutton[checked]:hover:active,
>+#inspector-tools > toolbarbutton[checked]:hover:active {
>+  background-color: hsla(210,8%,5%,.2)!important;
>+}

Always add a space before !important.
Which of the above !important flags are really needed?

Comment 6

[Paul Rouget [:paul]]

(In reply to Dão Gottwald [:dao] from comment #5)
> Comment on attachment 559097 [details] [diff] [review]
> patch v1
> 
> >+#inspector-toolbar {
> >+  -moz-appearance: none;
> >+  height: 32px;
> 
> Why are you setting a height here rather than letting the toolbar be sized
> automatically based on its contents?

I'll fix that (for Mac and Windows too).

> 
> >+#inspector-inspect-toolbutton,
> >+#inspector-tools > toolbarbutton {
> >+  -moz-appearance: none;
> >+  width: 78px;
> 
> I think you want min-width here.

Right.

> >+  margin: 3px 5px !important;
> 
> Why !important?

Because we find this in toolbarbutton.css:
.toolbarbutton-text {
  margin: 0 !important;
  text-align: center;
}

> >+#inspector-inspect-toolbutton[checked],
> >+#inspector-tools > toolbarbutton[checked] {
> >+  color: hsl(208,100%,60%) !important;
> >+  border-color: hsla(210,8%,5%,.6) !important;
> >+  background: -moz-linear-gradient(hsla(220,6%,10%,.6), hsla(210,11%,18%,.45) 75%, hsla(210,11%,30%,.4));
> >+  box-shadow: 0 1px 3px hsla(210,8%,5%,.25) inset, 0 1px 3px hsla(210,8%,5%,.25) inset, 0 1px 0 hsla(210,16%,76%,.15);
> >+}
> >+
> >+#inspector-inspect-toolbutton[checked]:hover,
> >+#inspector-tools > toolbarbutton[checked]:hover {
> >+  background-color: transparent!important;
> >+}
> >+
> >+#inspector-inspect-toolbutton[checked]:hover:active,
> >+#inspector-tools > toolbarbutton[checked]:hover:active {
> >+  background-color: hsla(210,8%,5%,.2)!important;
> >+}
> 
> Always add a space before !important.

Ok.

> Which of the above !important flags are really needed?

All of them.

Comment 7

[Paul Rouget [:paul]]

Merging bug 676255 with this bug. Coming patch will include CSS for Linux, Windows and Mac (pinstripe code updated for consistency with Linux and Windows).

Comment 8

[Paul Rouget [:paul]]

Attachment: patch v2

In this new patch, I include code for Linux, Windows and Mac. See comment 7.

Comment 10

[Paul Rouget [:paul]]

Dão, do you have some time to look at this patch?

Comment 12

[Rob Campbell [:rc] (:robcee)]

review ping!

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 563097 [details] [diff] [review]
patch v2

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

>+#inspector-inspect-toolbutton > .toolbarbutton-icon,
>+#inspector-tools > toolbarbutton  > .toolbarbutton-icon {
>+  margin: 0px;

nit: remove "px"

Comment 14

[Paul Rouget [:paul]]

Attachment: patch v2.1

Thanks for the r+ Dao

Comment 15

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/integration/fx-team/rev/359389696002

Comment 16

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/359389696002