862286 - GCLI panel too narrow for meaningful output

Status: RESOLVED FIXED

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect)

Description

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

Attachment: appcache show command

There are three problems:
1. Wide output is cropped when it could be far wider.
2. Text without a container displayed in a narrow popup so text must be scrolled e.g. bulleted lists.
3. Output is transient so users have to run commands multiple times in order to work through the results.

Items 1 and 2 are illustrated in the attached images.

Comment 1

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

Attachment: appcache validate command

Comment 2

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

For item 3 imagine working through the attachment in the previous comment and having to run the command again every time the issue has been addressed.

Comment 3

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

I don't see any way of solving this without moving GCLI into the toolbox.

Comment 4

[Victor Porof [:vporof][:vp]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #3)
> I don't see any way of solving this without moving GCLI into the toolbox.

Would that mean the output being shown inside the toolbox?

Comment 5

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

(In reply to Victor Porof [:vp] from comment #4)
> (In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #3)
> > I don't see any way of solving this without moving GCLI into the toolbox.
> 
> Would that mean the output being shown inside the toolbox?

It is something we have been planning for a while. Something like http://mozilla.github.io/gcli/ but the output would be in the toolbox.

Apart from it fixing the panels issue it is way more discoverable than it currently is.

Comment 6

[Joe Walker [:jwalker]]

We should have the max-width of the output panel to be the same as the width of the command line. Wouldn't that solve this problem?

Comment 7

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

I can see a few issues with that approach:
- It would allow the contents to be wider but would still clip the contents if they were wide enough.
- The tall, narrow output in comment 1 is nowhere near the current max-width so it would make no difference there.
- The transient nature of the output would still be an issue.

Maybe it is a better solution to what we currently have though.

Comment 8

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

Attachment: Patch

We now size the panel to the width of the longest line or the width of the browser, tweaking for each OS due to differences in panel implementations (no real choices there).

I am unable to get a build working on Windows so it would be great if somebody could give this a go and tweak the WINNT panel adjustment if necessary.

Comment 9

[Joe Walker [:jwalker]]

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

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

Nice one.

::: browser/devtools/shared/DeveloperToolbar.jsm
@@ +843,2 @@
>  
> +  // Adjust max width according to OS.

Could you add:
// We'd like to put this in CSS but we can't do:
//   body { width: calc(min(-5px, max-content)); }
//   #_panel { max-width: -5px; }

(And can you check that I'm understanding this properly? :)

@@ +855,5 @@
> +  }
> +
> +  this.document.body.style.width = "-moz-max-content";
> +  let frameWidth = parseInt(this._frame.contentWindow
> +                       .getComputedStyle(this.document.body).width, 10);

Readability nit, would this be easier to follow?:

  let style = this._frame.contentWindow.getComputedStyle(this.document.body)
  let frameWidth = parseInt(style.width, 10);

Comment 10

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

Attachment: Patch v2

(In reply to Joe Walker [:jwalker] from comment #9)
> Comment on attachment 747874 [details] [diff] [review]
> Patch
> 
> Review of attachment 747874 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nice one.
> 
> ::: browser/devtools/shared/DeveloperToolbar.jsm
> @@ +843,2 @@
> >  
> > +  // Adjust max width according to OS.
> 
> Could you add:
> // We'd like to put this in CSS but we can't do:
> //   body { width: calc(min(-5px, max-content)); }
> //   #_panel { max-width: -5px; }
> 
> (And can you check that I'm understanding this properly? :)
> 

Seems like you understand just fine.

> @@ +855,5 @@
> > +  }
> > +
> > +  this.document.body.style.width = "-moz-max-content";
> > +  let frameWidth = parseInt(this._frame.contentWindow
> > +                       .getComputedStyle(this.document.body).width, 10);
> 
> Readability nit, would this be easier to follow?:
> 
>   let style = this._frame.contentWindow.getComputedStyle(this.document.body)
>   let frameWidth = parseInt(style.width, 10);

Done.

Comment 11

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

https://hg.mozilla.org/integration/fx-team/rev/5d81a35534ba

Comment 12

[Joe Walker [:jwalker]]

https://hg.mozilla.org/mozilla-central/rev/5d81a35534ba