Last Comment Bug 718259 - Remove the resizer button in the Inspector Toolbar
: Remove the resizer button in the Inspector Toolbar
Status: RESOLVED FIXED
[fixed-in-fx-team]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Firefox 12
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: 717924
  Show dependency treegraph
 
Reported: 2012-01-14 22:12 PST by Paul Rouget [:paul]
Modified: 2012-01-21 07:39 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 (5.95 KB, patch)
2012-01-14 22:28 PST, Paul Rouget [:paul]
rcampbell: review+
shorlander: ui‑review+
Details | Diff | Review
patch v1 - rebased (5.82 KB, patch)
2012-01-17 04:37 PST, Paul Rouget [:paul]
no flags Details | Diff | Review
patch v1.1 (7.44 KB, patch)
2012-01-18 18:16 PST, Paul Rouget [:paul]
dao+bmo: review+
Details | Diff | Review

Description Paul Rouget [:paul] 2012-01-14 22:12:33 PST
1) I am not sure this button is that useful. We don't need such a resizer anywhere else in Firefox.
2) Introducing this resizer force us to get it in the other tools (webconsole, debugger, ...) for consistency. That makes the UI quite heavy (especially when we will get the dock buttons)
3) The resizer-bar is certainly enough
Comment 1 Paul Rouget [:paul] 2012-01-14 22:28:53 PST
Created attachment 588714 [details] [diff] [review]
patch v1
Comment 2 Paul Rouget [:paul] 2012-01-14 22:36:08 PST
Also, another problem with this resizer: opening the HTML panel move (a little) all the right buttons to the left.
Comment 3 Paul Rouget [:paul] 2012-01-14 22:38:16 PST
Comment on attachment 588714 [details] [diff] [review]
patch v1

Rob and Stephen, let me know what you think.
Comment 4 Rob Campbell [:rc] (:robcee) 2012-01-16 10:49:09 PST
Comment on attachment 588714 [details] [diff] [review]
patch v1

yeah, I'm fine with this change. Gets rid of a bunch of extra CSS and a control that is only of limited value. Plus, when we move the HTML panel, we'll be doing away with all of these anyway.
Comment 5 Paul Rouget [:paul] 2012-01-17 04:37:01 PST
Created attachment 589156 [details] [diff] [review]
patch v1 - rebased
Comment 6 Stephen Horlander [:shorlander] 2012-01-18 09:59:23 PST
Comment on attachment 588714 [details] [diff] [review]
patch v1

Review of attachment 588714 [details] [diff] [review]:
-----------------------------------------------------------------

I think removing it is the right way to go.

I did notice that it removes the padding when the HTML panel is open: http://cl.ly/2A3l2s220r1s0y1S2O3S
Comment 7 Paul Rouget [:paul] 2012-01-18 10:05:39 PST
(In reply to Stephen Horlander from comment #6)
> Comment on attachment 588714 [details] [diff] [review]
> patch v1
> 
> Review of attachment 588714 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think removing it is the right way to go.
> 
> I did notice that it removes the padding when the HTML panel is open:
> http://cl.ly/2A3l2s220r1s0y1S2O3S

Good catch! I'll update the patch.
Comment 8 Paul Rouget [:paul] 2012-01-18 18:16:51 PST
Created attachment 589746 [details] [diff] [review]
patch v1.1
Comment 9 Paul Rouget [:paul] 2012-01-18 18:32:44 PST
Comment on attachment 589746 [details] [diff] [review]
patch v1.1

A little explanation about this part of the patch (pinstripe only):
 
diff --git a/browser/themes/pinstripe/browser.css b/browser/themes/pinstripe/browser.css 
 #inspector-toolbar {
   border-top: 1px solid hsla(210, 8%, 5%, .65);
-  padding: 4px 16px 4px 0; /* use -moz-padding-end: 16px when/if bug 631729 gets fixed */

That was wrong in the first place. 0px of padding on the left was wrong on RTL.

+  padding-top: 4px;
+  padding-bottom: 4px;
+}
+
+#inspector-toolbar:-moz-locale-dir(ltr) {
+  padding-left: 2px;
+  padding-right: 16px; /* use -moz-padding-end when/if bug 631729 gets fixed */
+}
+
+#inspector-toolbar:-moz-locale-dir(rtl) {
+  padding-left: 4px;
+  padding-right: 18px; /* use -moz-padding-end when/if bug 631729 gets fixed */
 }

These values are needed to make sure that the close button is aligned with the close button of the status bar.
 
 #inspector-toolbar[treepanel-open] {
-  padding: 0 0 4px;
+  padding-top: 0;
+  padding-right: 0;
+  -moz-padding-end: 4px;
 }

Once the treepanel is open, we don't need the 16px padding anymore.
In LTR, we need 2px on the left (to align with the status bar close-button), and 4px on the right (like any devtools toolbar).
In RTL, we need 4px (like any devtools toobar) on the left, and 0px on the left (to align with the status bar close-button).
Comment 10 Rob Campbell [:rc] (:robcee) 2012-01-19 11:21:52 PST
setting to ASSIGNED.
Comment 11 Rob Campbell [:rc] (:robcee) 2012-01-20 10:47:56 PST
https://hg.mozilla.org/integration/fx-team/rev/96036564910e
Comment 12 Rob Campbell [:rc] (:robcee) 2012-01-21 07:39:07 PST
https://hg.mozilla.org/mozilla-central/rev/96036564910e

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