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

RESOLVED FIXED in Firefox 22

Status

()

Firefox
Developer Tools: Framework
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: paul, Assigned: miker)

Tracking

Trunk
Firefox 22
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox22 verified)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Depends on: 818288
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.
Component: Developer Tools → Developer Tools: Framework
Assignee: nobody → mratcliffe
Status: NEW → ASSIGNED
No longer depends on: 818288
Duplicate of this bug: 842515
Created attachment 715981 [details] [diff] [review]
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.
Attachment #715981 - Flags: review?(jwalker)
(Reporter)

Comment 4

5 years ago
Comment on attachment 715981 [details] [diff] [review]
Patch

I'll take a look too.

Don't turn on the pref in the bug though.
Attachment #715981 - Flags: review?(paul)
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
Attachment #715981 - Flags: review?(jwalker) → review+
Created attachment 716478 [details] [diff] [review]
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.
Attachment #715981 - Attachment is obsolete: true
Attachment #715981 - Flags: review?(paul)
Attachment #716478 - Flags: review?(paul)
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?
Whiteboard: [ux-feedback-needed]
(Reporter)

Comment 10

5 years ago
Comment on attachment 716478 [details] [diff] [review]
Patch v2

(removing review until ux comes up with a design)
Attachment #716478 - Flags: review?(paul)
(Reporter)

Comment 11

5 years ago
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)
(Reporter)

Comment 12

5 years ago
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."
(Reporter)

Updated

5 years ago
Whiteboard: [ux-feedback-needed]
(Reporter)

Updated

5 years ago
Blocks: 846371
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.
Created attachment 722399 [details] [diff] [review]
Patch v3

Very simple.
Attachment #716478 - Attachment is obsolete: true
Attachment #716630 - Attachment is obsolete: true
Attachment #716631 - Attachment is obsolete: true
Attachment #722399 - Flags: review?(paul)
Created attachment 722711 [details] [diff] [review]
Patch v4

Removed pointless assignment.
Attachment #722399 - Attachment is obsolete: true
Attachment #722399 - Flags: review?(paul)
Attachment #722711 - Flags: review?(paul)
Whiteboard: [has-patch]
(Reporter)

Comment 16

5 years ago
Comment on attachment 722711 [details] [diff] [review]
Patch v4

On OSX:

> label is undefined
> Toolbox.jsm:431
Attachment #722711 - Flags: review?(paul)
Created attachment 724605 [details] [diff] [review]
patch v5

This works well.

Mihai and Benvie need to test this on Windows and Linux before you review it.
Attachment #722711 - Attachment is obsolete: true
Attachment #724605 - Flags: review?(paul)
Attachment #724605 - Flags: feedback?(mihai.sucan)
Attachment #724605 - Flags: feedback?(bbenvie)
(Reporter)

Comment 18

5 years ago
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 ?
Attachment #724605 - Flags: feedback?(bbenvie) → feedback+
Attachment #724605 - Flags: feedback?(mihai.sucan)
Created attachment 724660 [details] [diff] [review]
patch v47

Added min-width to side panel
Attachment #724605 - Attachment is obsolete: true
Attachment #724605 - Flags: review?(paul)
Attachment #724660 - Flags: review?(paul)
(Reporter)

Comment 20

5 years ago
Comment on attachment 724660 [details] [diff] [review]
patch v47

Let's land that!
Attachment #724660 - Flags: review?(paul) → review+
Whiteboard: [has-patch] → [land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/10e454c16406
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
(Reporter)

Comment 22

5 years ago
https://hg.mozilla.org/mozilla-central/rev/10e454c16406
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox22: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Blocks: 856814
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
status-firefox22: fixed → verified
You need to log in before you can comment on or make changes to this bug.