818151 - [toolbox] adapt the toolbox UI to make it fit on the side of the browser

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[Paul Rouget [:paul]]

We have disabled the "dock in the sidebar" feature because our tools and the toolbox look "squashed".

We should implement a responsive UI and then re-enable the pref.

Comment 1

[Girish Sharma [:Optimizer]]

One generic thing that can be done , which will help a lot in this bug and bug 818288 will be to make the sidebar responsive. As soon as the width to height ratio drop a threshold, make the sidebar come below the rest of the content (instead of to the right of the rest of the content). Just like style editor's source editor behave.

Similar thing can be done for the two side panels of debugger. When the panels are hidden, no issues. But when you show them, instead of showing to the left and right of the source editor, show them stacked together, below the editor.

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Patch

Ideally we would have used the overflow and underflow events but they just don't work here (they do not get triggered at the right time). Using a resize listener works far better.

Comment 4

[Paul Rouget [:paul]]

Comment on attachment 715981 [details] [diff] [review]
Patch

I'll take a look too.

Don't turn on the pref in the bug though.

Comment 5

[Joe Walker [:jwalker]]

Comment on attachment 715981 [details] [diff] [review]
Patch

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

I think this is a good step forward, some things I think we need to do before we can pref-on by default (I don't mind if we do them in this bug or another)
- The font for the menu items should be the same as the font for the normal buttons
- The menu items should have the icons of the normal buttons
- There should be more left padding, and less white border to the menu
- The tab overflow marker used by the Firefox tabbrowser is a single '>' i.e. (chrome://browser/skin/tabbrowser/tab-arrow-right.png) I think it would be good to have a graphic based on that
- When an item in the menu is selected then the overflow '>' should have the overline highlight, and really it would be good to have the current menuitem having a similar highlight (but probably on the RHS?)

::: browser/devtools/framework/Toolbox.jsm
@@ +257,5 @@
>          iframe.removeEventListener("DOMContentLoaded", domReady, true);
>  
>          this.isReady = true;
>  
> +        this.closeButton = this.doc.getElementById("toolbox-close");

this._closeButton maybe ?

@@ +419,4 @@
>        this.selectTool(id);
> +    }.bind(this, id);
> +
> +    radio.addEventListener("command", func);

Perhaps we can find a better function name than func?

@@ +735,5 @@
>  
>      gDevTools.off("tool-registered", this._toolRegistered);
>      gDevTools.off("tool-unregistered", this._toolUnregistered);
>  
> +    this.frame.removeEventListener("resize", this._tabBarRefresh, false);

I've not looked into the flow, but we're removing in 2 places. Is that right?

::: browser/devtools/framework/toolbox.css
@@ +21,5 @@
> +#toolbox-tab-dropdown,
> +#toolbox-tab-dropdown > .button-box,
> +#toolbox-tab-dropdown > menupopup,
> +#toolbox-tab-dropdown > menupopup menuitem {
> +  -moz-appearance: none;

I think this should be theme CSS?
https://wiki.mozilla.org/DevTools/CSSTips#Content_or_Theme_CSS

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Patch v2

(In reply to Joe Walker [:jwalker] from comment #5)
> Comment on attachment 715981 [details] [diff] [review]
> Patch
> 
> Review of attachment 715981 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think this is a good step forward, some things I think we need to do
> before we can pref-on by default (I don't mind if we do them in this bug or
> another)
> - The font for the menu items should be the same as the font for the normal
> buttons
> - The menu items should have the icons of the normal buttons
> - There should be more left padding, and less white border to the menu
> - The tab overflow marker used by the Firefox tabbrowser is a single '>'
> i.e. (chrome://browser/skin/tabbrowser/tab-arrow-right.png) I think it would
> be good to have a graphic based on that
> - When an item in the menu is selected then the overflow '>' should have the
> overline highlight, and really it would be good to have the current menuitem
> having a similar highlight (but probably on the RHS?)
> 

Most of the tools also need a narrow view via media queries.

> ::: browser/devtools/framework/Toolbox.jsm
> @@ +257,5 @@
> >          iframe.removeEventListener("DOMContentLoaded", domReady, true);
> >  
> >          this.isReady = true;
> >  
> > +        this.closeButton = this.doc.getElementById("toolbox-close");
> 
> this._closeButton maybe ?
> 

Done

> @@ +419,4 @@
> >        this.selectTool(id);
> > +    }.bind(this, id);
> > +
> > +    radio.addEventListener("command", func);
> 
> Perhaps we can find a better function name than func?
> 

Actually, we don't even need a new function there. Removed.

> @@ +735,5 @@
> >  
> >      gDevTools.off("tool-registered", this._toolRegistered);
> >      gDevTools.off("tool-unregistered", this._toolUnregistered);
> >  
> > +    this.frame.removeEventListener("resize", this._tabBarRefresh, false);
> 
> I've not looked into the flow, but we're removing in 2 places. Is that right?
> 

This is because the resize listener is added to this.frame. switchHost creates a new frame so the listener is first removed from the old frame and then added to the new one. I have added a comment to clarify this.

> ::: browser/devtools/framework/toolbox.css
> @@ +21,5 @@
> > +#toolbox-tab-dropdown,
> > +#toolbox-tab-dropdown > .button-box,
> > +#toolbox-tab-dropdown > menupopup,
> > +#toolbox-tab-dropdown > menupopup menuitem {
> > +  -moz-appearance: none;
> 
> I think this should be theme CSS?
> https://wiki.mozilla.org/DevTools/CSSTips#Content_or_Theme_CSS

moz-appearance being theme css surprises me as for any styling to be possible it needs to be set to none. Moved.

So in a nutshell:
- The font for the menu items should be the same as the font for the normal buttons on all OSes.
- The menu items should have the icons of the normal buttons
- There should be more left padding, and less white border to the menu
- The tab overflow marker used by the Firefox tabbrowser is a single '>'
  i.e. (chrome://browser/skin/tabbrowser/tab-arrow-right.png) I think it would be good to have a graphic based on that
- When an item in the menu is selected then the overflow '>' should have the overline highlight, and really it would be good to have the current menuitem having a similar highlight (but probably on the RHS?)
- Most of the tools also need a narrow view via media queries.

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Sidebar Linux

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Sidebar Mac

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

We should refresh the tabs onLoad instead of onDOMLoad as otherwise they are missed.

Shorlander, The current look of a narrower version of the toolbax (for docking on the right of the screen) is attached.

We figure that maybe this needs some UX love ... what do you think?

Comment 10

[Paul Rouget [:paul]]

Comment on attachment 716478 [details] [diff] [review]
Patch v2

(removing review until ux comes up with a design)

Comment 11

[Paul Rouget [:paul]]

I believe we should either fold all the tabs into a single menu, or not fold them at all (but we're still waiting for UX feedback)

Comment 12

[Paul Rouget [:paul]]

Shorlander suggested that we just hide the labels on overflow. About only having icons, he said:

"Labels are good, however I am not too worried about dropping them for space savings in this case.

People learn the meanings of ideograms either through prior experience/convention or by active association with a real world thing or label. In this case we do have labels in the default state where things are docked to the bottom. A user would have to start in that state and actively move the toolbar to the side.

I don't find this any different than all of the other places we use just icons and no label.
                 
I would shrink the tabs down to their smallest size (icon) and if there is still overflow use the chevron and list approach but also include the icon in the list with the tool name."

Comment 13

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

For the moment we have decided to show only icons when docked to the side and show the full tabs when docked to the bottom.

Comment 14

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Patch v3

Very simple.

Comment 15

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Patch v4

Removed pointless assignment.

Comment 16

[Paul Rouget [:paul]]

Comment on attachment 722711 [details] [diff] [review]
Patch v4

On OSX:

> label is undefined
> Toolbox.jsm:431

Comment 17

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: patch v5

This works well.

Mihai and Benvie need to test this on Windows and Linux before you review it.

Comment 18

[Paul Rouget [:paul]]

Comment on attachment 724605 [details] [diff] [review]
patch v5

>--- a/browser/themes/linux/devtools/toolbox.css
> .devtools-tab {
>   -moz-appearance: none;
>   min-width: 88px;
>-  min-height: 32px;
>+  min-height: 47px;
>+  max-width: 137px;
>   color: #b6babf;
>   margin: 0;
>   padding: 0;
>   background-image: linear-gradient(hsla(204,45%,98%,.05), hsla(204,45%,98%,.1)),
>                     linear-gradient(hsla(204,45%,98%,.05), hsla(204,45%,98%,.1));
>   background-size: 1px 100%;
>   background-repeat: no-repeat;
>   background-position: left, right;
>   border-right: 1px solid hsla(206,37%,4%,.45);
>+  width: 1px;
> }

width:1px ?

Comment 19

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: patch v47

Added min-width to side panel

Comment 20

[Paul Rouget [:paul]]

Comment on attachment 724660 [details] [diff] [review]
patch v47

Let's land that!

Comment 21

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://hg.mozilla.org/integration/fx-team/rev/10e454c16406

Comment 22

[Paul Rouget [:paul]]

https://hg.mozilla.org/mozilla-central/rev/10e454c16406

Comment 23

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 22 beta 2: -the toolbox UI fits on the side of the browser.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0
Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20100101 Firefox/22.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20100101 Firefox/22.0
Build ID: 20130521223249