Closed Bug 862286 Opened 7 years ago Closed 6 years ago

GCLI panel too narrow for meaningful output

Categories

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

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: miker, Assigned: miker)

References

Details

Attachments

(3 files, 1 obsolete file)

Attached image 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.
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.
I don't see any way of solving this without moving GCLI into the toolbox.
(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?
(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.
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?
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.
Assignee: nobody → mratcliffe
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
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.
Attachment #747874 - Flags: review?(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);
Attachment #747874 - Flags: review?(jwalker) → review+
Attached patch Patch v2Splinter Review
(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.
Attachment #747874 - Attachment is obsolete: true
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/5d81a35534ba
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 23
Duplicate of this bug: 829586
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.