676253 - Make the Highlighter Toolbar look like faaborg's design on OS X

Status: VERIFIED FIXED

(DevTools :: General, defect, P2)

Description

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

The highlighter toolbar should have the white on-black styling designed by Alex Faaborg on OS X.

see: http://people.mozilla.com/~faaborg/files/projects/webDeveloper/web%20developer%20toolbar%20i1.png

Comment 1

[Stephen Horlander [:shorlander]]

Attachment: Toolbar Design Specs - i01

Comment 2

[Paul Rouget [:paul]]

Questions:
- the border-top of the toolbars is transparent (.65), what is supposed to be behind this border? The content page? The toolbar? 
- What if the breadcrumbs bar is hidden? Should we keep the dark border-top?
- Is it ok to use an image for the ">" shape betweens the buttons? Or it is better to use a 45deg-rotated block element?

Comment 3

[Paul Rouget [:paul]]

Replying to my own questions,

(In reply to Paul Rouget [:paul] from comment #2)
> Questions:
> - the border-top of the toolbars is transparent (.65), what is supposed to
> be behind this border? The content page? The toolbar?

The toolbar of course :)

> - What if the breadcrumbs bar is hidden? Should we keep the dark border-top?

Yes. But this will only happen when we will have the developer toolbar, so not a problem here.

> - Is it ok to use an image for the ">" shape betweens the buttons? Or it is
> better to use a 45deg-rotated block element?

Using a 45deg-rotated block element works, but the CSS code is very ugly.
Stephen, do you think you can provide an image where the right part is transparent?
I would need 2 version of it, when the element is selected, and when it's not selected.

Comment 4

[Stephen Horlander [:shorlander]]

Attachment: Toolbar Design Specs - i02

Comment 5

[Paul Rouget [:paul]]

Attachment: patch v1

Comment 6

[Paul Rouget [:paul]]

Attachment #557177 [details] [diff] is only for the toolbar and the buttons inside. No breadcrumbs or layout changes included.

Comment 7

[Paul Rouget [:paul]]

Attachment: Screenshot (v1)

Comment 8

[Dão Gottwald [:dao]]

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

>+#inspector-toolbar {

>+  padding-left: 6px;

this doesn't look RTL-ready

>+#inspector-inspect-toolbutton,
>+#inspector-tools > toolbarbutton {

>+  margin: 3px 10px 3px 0;

this doesn't look RTL-ready either

>+  border-radius: 3px;

use @toolbarbuttonCornerRadius@

>+  font-size: 12px;

Where does this number come from? Can you use a relative unit?

>+#inspector-inspect-toolbutton:not([checked="true"]):hover:active,
>+#inspector-tools > toolbarbutton:not([checked="true"]):hover:active {

nit: remove ="true"

>+  border: 1px solid hsla(210,8%,5%,.6);

use border-color

>+#inspector-inspect-toolbutton[checked="true"],
>+#inspector-tools > toolbarbutton[checked="true"] {

nit: remove ="true"

>+  border: 1px solid hsla(210,8%,5%,.6);

border-color again

>+  color: hsl(208,100%,60%)!important;

Why !important?

>+#inspector-inspect-toolbutton[checked="true"]:hover:active,
>+#inspector-tools > toolbarbutton[checked="true"]:hover:active {

nit: remove ="true"

Comment 9

[Paul Rouget [:paul]]

(In reply to Dão Gottwald [:dao] from comment #8)
> >+  color: hsl(208,100%,60%)!important;
> 
> Why !important?

Because of this:
http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/pinstripe/global/toolbarbutton.css#79

Comment 10

[Paul Rouget [:paul]]

Attachment: patch v1.1

Addressed Dao's comments. Removed some useless properties.

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 557482 [details] [diff] [review]
patch v1.1

>+#inspector-inspect-toolbutton[checked],
>+#inspector-tools > toolbarbutton[checked] {
>+  color: hsl(208,100%,60%)!important;

nit: please add a space before !important

Comment 12

[Paul Rouget [:paul]]

Attachment: [checked-in] patch v1.2

Thank you Dao.

Comment 13

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

Comment on attachment 557484 [details] [diff] [review]
[checked-in] patch v1.2

http://hg.mozilla.org/integration/fx-team/rev/cefb66e24059

Comment 14

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

Comment on attachment 557484 [details] [diff] [review]
[checked-in] patch v1.2

http://hg.mozilla.org/mozilla-central/rev/cefb66e24059

Comment 15

[Teodosia Pop]

Verified Fixed using Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0a1) Gecko/20111020 Firefox/10.0a1.