Last Comment Bug 691712 - Add some pretty resizers to the Highlighter toolbar when the HTML tree panel is open
: Add some pretty resizers to the Highlighter toolbar when the HTML tree panel ...
Status: VERIFIED FIXED
[testday-20111125]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: Firefox 10
Assigned To: Paul Rouget [:paul]
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
: 689944 (view as bug list)
Depends on: 683906
Blocks: 663830 672006
  Show dependency treegraph
 
Reported: 2011-10-04 03:47 PDT by Paul Rouget [:paul]
Modified: 2011-11-25 06:56 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Shorlander's proposal (203.97 KB, image/png)
2011-10-04 03:48 PDT, Paul Rouget [:paul]
no flags Details
patch v1 (10.31 KB, patch)
2011-10-04 03:54 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
screenshot patch v1 (76.33 KB, image/png)
2011-10-04 03:55 PDT, Paul Rouget [:paul]
no flags Details
screenshot patch v1 - with active regions (86.99 KB, image/png)
2011-10-04 03:56 PDT, Paul Rouget [:paul]
no flags Details
patch v1.1 (10.29 KB, patch)
2011-10-04 09:01 PDT, Paul Rouget [:paul]
rcampbell: review+
Details | Diff | Splinter Review
Resize Grippy (116 bytes, image/png)
2011-10-05 15:01 PDT, Stephen Horlander [:shorlander]
no flags Details
patch v1.2 (10.70 KB, patch)
2011-10-06 04:01 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
screenshot patch v1.2 (228.37 KB, image/png)
2011-10-06 04:02 PDT, Paul Rouget [:paul]
no flags Details
patch v1.3 (10.70 KB, patch)
2011-10-06 04:25 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
screenshot patch v1.3 (247.26 KB, image/png)
2011-10-06 04:26 PDT, Paul Rouget [:paul]
shorlander: ui‑review+
Details
patch v1.4 (10.70 KB, patch)
2011-10-06 07:20 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
patch v1.5 (10.99 KB, patch)
2011-10-21 03:34 PDT, Paul Rouget [:paul]
dao+bmo: review-
Details | Diff | Splinter Review
patch v1.6 (10.67 KB, patch)
2011-10-26 10:33 PDT, Paul Rouget [:paul]
dao+bmo: review+
Details | Diff | Splinter Review
patch v1.7 (10.65 KB, patch)
2011-10-26 15:51 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review

Description Paul Rouget [:paul] 2011-10-04 03:47:24 PDT
We want to be able to resize the tree panel. The current situation (using the free space in the toolbar as a resizer) is blocking further developments in the toolbar (like the breadcrumbs).
Comment 1 Paul Rouget [:paul] 2011-10-04 03:48:55 PDT
Created attachment 564506 [details]
Shorlander's proposal
Comment 2 Paul Rouget [:paul] 2011-10-04 03:54:21 PDT
Created attachment 564507 [details] [diff] [review]
patch v1

Rob, would that work for you?
Comment 3 Paul Rouget [:paul] 2011-10-04 03:55:56 PDT
Created attachment 564508 [details]
screenshot patch v1
Comment 4 Paul Rouget [:paul] 2011-10-04 03:56:26 PDT
Created attachment 564509 [details]
screenshot patch v1 - with active regions
Comment 5 Paul Rouget [:paul] 2011-10-04 05:53:42 PDT
*** Bug 689944 has been marked as a duplicate of this bug. ***
Comment 6 Paul Rouget [:paul] 2011-10-04 09:01:13 PDT
Created attachment 564568 [details] [diff] [review]
patch v1.1

CSS tweaks.
Comment 7 Rob Campbell [:rc] (:robcee) 2011-10-04 13:29:22 PDT
Comment on attachment 564568 [details] [diff] [review]
patch v1.1

fantastique!
Comment 8 Stephen Horlander [:shorlander] 2011-10-05 15:01:07 PDT
Created attachment 565030 [details]
Resize Grippy

Can we use this image instead? Or alter the gradient to match it?

Mockup: http://cl.ly/1D3A0D0y2a1z3r0U1P11
Comment 9 Paul Rouget [:paul] 2011-10-06 03:19:19 PDT
Ok, I'll update the gradient
Comment 10 Paul Rouget [:paul] 2011-10-06 04:01:23 PDT
Created attachment 565176 [details] [diff] [review]
patch v1.2

prettier.
Comment 11 Paul Rouget [:paul] 2011-10-06 04:02:41 PDT
Created attachment 565177 [details]
screenshot patch v1.2
Comment 12 Paul Rouget [:paul] 2011-10-06 04:07:19 PDT
Oops, I think this is 2 pixels too small.
Comment 13 Paul Rouget [:paul] 2011-10-06 04:25:03 PDT
Created attachment 565180 [details] [diff] [review]
patch v1.3
Comment 14 Paul Rouget [:paul] 2011-10-06 04:26:08 PDT
Created attachment 565181 [details]
screenshot patch v1.3
Comment 15 Paul Rouget [:paul] 2011-10-06 07:20:11 PDT
Created attachment 565214 [details] [diff] [review]
patch v1.4

One pixel row was missing
Comment 16 Rob Campbell [:rc] (:robcee) 2011-10-19 06:26:29 PDT
Review ping! We have some patches reliant on this.
Comment 17 Paul Rouget [:paul] 2011-10-21 03:34:29 PDT
Created attachment 568630 [details] [diff] [review]
patch v1.5

rebased
Comment 18 Dão Gottwald [:dao] 2011-10-25 13:43:17 PDT
Comment on attachment 568630 [details] [diff] [review]
patch v1.5

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

> #inspector-toolbar {
>   -moz-appearance: none;
>-  padding: 4px 3px;
>+  padding: 0px 3px 4px 3px;

nit: 0 instead of 0px

>+#inspector-end-resizer {
>+  width: 12px; height: 8px;

new line after ;

>+  background-image: -moz-linear-gradient(top,black 1px,rgba(255, 255, 255, 0.2) 1px) !important;

Why do you need !important here?

nit: add a space after each comma, *except* within color functions:
-moz-linear-gradient(top, black 1px, rgba(255,255,255,0.2) 1px)

>@@ -2670,8 +2688,9 @@ panel[dimmed="true"] {
> #highlighter-nodeinfobar-container[position="top"] > #highlighter-nodeinfobar,
> #highlighter-nodeinfobar-container[position="overlap"] > #highlighter-nodeinfobar {
>   box-shadow: 0 1px 0 hsla(0, 0%, 100%, .1) inset;
> }
> 
> #highlighter-nodeinfobar-container[hide-arrow] > #highlighter-nodeinfobar {
>   margin: 7px 0;
> }
>+

bogus change
Comment 19 Rob Campbell [:rc] (:robcee) 2011-10-26 06:52:41 PDT
so, r+ with those changes, Dao?
Comment 20 Dão Gottwald [:dao] 2011-10-26 07:12:08 PDT
Comment on attachment 568630 [details] [diff] [review]
patch v1.5

I'd prefer seeing a new patch, as I've seen people mess up trivial changes, forget some of our three themes, etc.

Also waiting for a response regarding !important.

By the way, write "padding: 0 3px 4px;" instead of "padding: 0 3px 4px 3px;". This makes it more obvious that the padding is symmetrical (so RTL won't be an issue).
Comment 21 Paul Rouget [:paul] 2011-10-26 10:33:42 PDT
Created attachment 569713 [details] [diff] [review]
patch v1.6

Addressed Dao's comments. !important wasn't useful. I was too aggressive with the background:none!important.
Comment 22 Dão Gottwald [:dao] 2011-10-26 14:15:25 PDT
Comment on attachment 569713 [details] [diff] [review]
patch v1.6

>+          <toolbarseparator />

nit: remove the space before />

>+    let resizerTop = 

trailing space

>+    let resizerEnd = 

ditto

>+#inspector-end-resizer {

>+  border-width: 1px 1px 0 1px;

border-width: 1px 1px 0;
(affects all themes)

>+  margin: 7px 7px 8px 7px;

margin: 7px 7px 8px;
(affects all themes)
Comment 23 Paul Rouget [:paul] 2011-10-26 15:51:23 PDT
Created attachment 569827 [details] [diff] [review]
patch v1.7

Thanks for the r+ Dao.
Comment 25 :Margaret Leibovic 2011-10-27 11:44:04 PDT
https://hg.mozilla.org/mozilla-central/rev/ca0e85e9fe89
Comment 26 Ioan C. 2011-11-25 06:56:37 PST
Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111123 Firefox/10.0a2
Verified fixed. http://screencast.com/t/0IGuh6gZpy1s

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