Last Comment Bug 683906 - Make the Highlighter Toolbar look like shorlander's design
: Make the Highlighter Toolbar look like shorlander's design
Status: RESOLVED FIXED
[minotaur]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 10
Assigned To: Paul Rouget [:paul]
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
: 676255 684188 (view as bug list)
Depends on:
Blocks: 663830 672006 691712
  Show dependency treegraph
 
Reported: 2011-09-01 08:04 PDT by Paul Rouget [:paul]
Modified: 2011-10-22 12:16 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 WIP (2.17 KB, patch)
2011-09-01 11:30 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
patch v1 - screenshot - ubuntu 10.04 (367.26 KB, image/png)
2011-09-01 13:00 PDT, Mihai Sucan [:msucan]
no flags Details
patch v1 (2.38 KB, patch)
2011-09-08 04:14 PDT, Paul Rouget [:paul]
dao+bmo: review-
Details | Diff | Splinter Review
patch v2 (7.59 KB, patch)
2011-09-28 09:56 PDT, Paul Rouget [:paul]
dao+bmo: review+
Details | Diff | Splinter Review
patch v2.1 (7.84 KB, patch)
2011-10-21 09:08 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review

Description Paul Rouget [:paul] 2011-09-01 08:04:55 PDT
We may want to use OSX design (bug 676253) or Windows design (Bug 676255).
Comment 1 Paul Rouget [:paul] 2011-09-01 11:30:21 PDT
Created attachment 557581 [details] [diff] [review]
patch v1 WIP
Comment 2 Mihai Sucan [:msucan] 2011-09-01 13:00:48 PDT
Created attachment 557619 [details]
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] 2011-09-02 02:43:35 PDT
Thank you Mihai.
Comment 4 Paul Rouget [:paul] 2011-09-08 04:14:39 PDT
Created attachment 559097 [details] [diff] [review]
patch v1

Based on Mac design (bug 676253).
Comment 5 Dão Gottwald [:dao] 2011-09-09 07:35:19 PDT
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] 2011-09-22 09:43:10 PDT
(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] 2011-09-28 09:49:52 PDT
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] 2011-09-28 09:56:04 PDT
Created attachment 563097 [details] [diff] [review]
patch v2

In this new patch, I include code for Linux, Windows and Mac. See comment 7.
Comment 9 Dão Gottwald [:dao] 2011-09-28 10:35:38 PDT
*** Bug 676255 has been marked as a duplicate of this bug. ***
Comment 10 Paul Rouget [:paul] 2011-10-06 09:29:30 PDT
Dão, do you have some time to look at this patch?
Comment 11 Paul Rouget [:paul] 2011-10-10 06:19:28 PDT
*** Bug 684188 has been marked as a duplicate of this bug. ***
Comment 12 Rob Campbell [:rc] (:robcee) 2011-10-19 06:27:26 PDT
review ping!
Comment 13 Dão Gottwald [:dao] 2011-10-21 08:26:14 PDT
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] 2011-10-21 09:08:58 PDT
Created attachment 568665 [details] [diff] [review]
patch v2.1

Thanks for the r+ Dao
Comment 15 Rob Campbell [:rc] (:robcee) 2011-10-21 10:56:49 PDT
https://hg.mozilla.org/integration/fx-team/rev/359389696002
Comment 16 Tim Taubert [:ttaubert] 2011-10-22 12:16:39 PDT
https://hg.mozilla.org/mozilla-central/rev/359389696002

Note You need to log in before you can comment on or make changes to this bug.