718259 - Remove the resizer button in the Inspector Toolbar

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Paul Rouget [:paul]]

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]]

Attachment: patch v1

Comment 2

[Paul Rouget [:paul]]

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]]

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

Rob and Stephen, let me know what you think.

Comment 4

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

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]]

Attachment: patch v1 - rebased

Comment 6

[Stephen Horlander [:shorlander]]

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]]

(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]]

Attachment: patch v1.1

Comment 9

[Paul Rouget [:paul]]

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)]

setting to ASSIGNED.

Comment 11

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

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

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/96036564910e