Closed Bug 938616 Opened 6 years ago Closed 6 years ago

webconsole UI updates while in split mode with other panels

Categories

(DevTools :: Console, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 28

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

(Whiteboard: [good first verify])

Attachments

(3 files, 4 obsolete files)

Here is (kind of) what the split console will look like: https://bugzilla.mozilla.org/attachment.cgi?id=828842.  There are some UI cleanup items we should land in this release with it.

Some thoughts from msucan about the the split console UI: https://bugzilla.mozilla.org/show_bug.cgi?id=862558#c27.

We should have:

- a button for new split console.
- a smaller console toolbar when the console is displayed at the bottom.

Note that the new comps in Bug  (like https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/DarkTheme-Console-AllToggled@2x.png) already have a smaller console.  We will need to decide if this is enough, or if we need an even smaller top toolbar.

Regardless, this bug should land with Firefox 28, so depending on the progress on Bug 916766 we may want to just do something simple with the existing toolbar when it is in split mode.
Darrin, do you have any thoughts on adding a button to toggle the split console?  Should we add one, where should it go, and what should it look like?
Flags: needinfo?(dhenein)
> We will need to decide if this is enough, or if we need an even smaller top toolbar.

I'd prefer if we can avoid duplicating code. Let's not have a different design for the split-console. And at some point, we will use JSTerm instead of the console, so it's a temporary thing.

I see 3 options:
1) we just land this as it is (big buttons)
2) we wait until we redesigned the web console toolbar
3) we remove the toolbar in split-mode (in split mode, people want just to evaluate JS I guess)
fwiw, i like option 3.
(In reply to Paul Rouget [:paul] from comment #2)
> > We will need to decide if this is enough, or if we need an even smaller top toolbar.
> 
> I'd prefer if we can avoid duplicating code. Let's not have a different
> design for the split-console. And at some point, we will use JSTerm instead
> of the console, so it's a temporary thing.
> 
> I see 3 options:
> 1) we just land this as it is (big buttons)
> 2) we wait until we redesigned the web console toolbar
> 3) we remove the toolbar in split-mode (in split mode, people want just to
> evaluate JS I guess)

I'm thinking we land it as-is and do whatever changes here as a follow up.

I've never been a big fan of (3), since I think that actually complicates the UX - if your logs weren't showing up b/c a filter wasn't selected, then we need to somehow tell them to go over to the 'real' console panel and enable them.

I'm kind of leaning towards (2).  In the meantime we could probably update the button height in the toolbar to match screens in Bug 916766, without fully implementing the whole design (which seems to have some behavior changes with it wrt filters).
Status: NEW → ASSIGNED
Priority: -- → P2
Darrin, can you make an icon for the split console UI for us to add as a command button?  The other command-* buttons can be seen here: https://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/devtools/.
Attached image command-console.png (obsolete) —
I made an icon (attached), can you show me a screenshot of it in the toolbar so I can ui-review it in place?
Flags: needinfo?(dhenein) → needinfo?(bgrinstead)
Attached image splitconsole-button.png (obsolete) —
> I made an icon (attached), can you show me a screenshot of it in the toolbar
> so I can ui-review it in place?

Here is a screenshot of the button in both on and off states
Attachment #8338701 - Flags: ui-review?(dhenein)
Flags: needinfo?(bgrinstead)
Attached image command-console.png r2
Going to be picky here... this icon in this attachment is 1px taller and 1px shifted down (within the same 16px area). Can you also toggle the paintbrush icon in the screenshot so I can make sure the active state is consistent?
Attachment #8338563 - Attachment is obsolete: true
Flags: needinfo?(bgrinstead)
OK, updated to the new icon, and have a screenshot next to paint flashing enabled.
Attachment #8338701 - Attachment is obsolete: true
Attachment #8338701 - Flags: ui-review?(dhenein)
Attachment #8338729 - Flags: ui-review?(dhenein)
Flags: needinfo?(bgrinstead)
Comment on attachment 8338729 [details]
splitconsole-button1.png

Looks good to me :) Thanks!
Attachment #8338729 - Flags: ui-review?(dhenein) → ui-review+
Attached patch webconsole-ui-updates-WIP.patch (obsolete) — Splinter Review
Work in progress patch that adds a command button for the split console.  

* Added gcli command for "splitconsole on", "splitconsole off", and "splitconsole toggle".
* Modified toggleSplitConsole to take a bool to force on or off
* The "splitconsole toggle" command button listens to toolbox events for split console and tool selected to show current state.  I've tested this with in different host states and across multiple tabs, but need to add in a test for this, and double check that it isn't going to cause any problems.
Attached patch webconsole-ui-updates.patch (obsolete) — Splinter Review
Patch updated with tests and hidden gcli command.  Will need one string localized for the tooltip text on the button.  Ongoing try push: https://tbpl.mozilla.org/?tree=Try&rev=eb25a9426dc0.
Attachment #8338754 - Attachment is obsolete: true
Comment on attachment 8341857 [details] [diff] [review]
webconsole-ui-updates.patch

Mihai,
Can you take a look at this patch? Darrin has already done a UI review on the button itself.
Attachment #8341857 - Flags: review?(mihai.sucan)
Comment on attachment 8341857 [details] [diff] [review]
webconsole-ui-updates.patch

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

Brian, thanks for the patch - glad to see this close to being done.

Patch looks good, but the icon is not rendered on Linux. You only updated toolbox.css for osx. The same problem might be valid for Windows builds as well.
Attachment #8341857 - Flags: review?(mihai.sucan)
(In reply to Mihai Sucan [:msucan] from comment #14)
> Comment on attachment 8341857 [details] [diff] [review]
> webconsole-ui-updates.patch
> 
> Review of attachment 8341857 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Brian, thanks for the patch - glad to see this close to being done.
> 
> Patch looks good, but the icon is not rendered on Linux. You only updated
> toolbox.css for osx. The same problem might be valid for Windows builds as
> well.

My mistake - marking as dependent on Bug 941673 so that the CSS only needs to be added in one place.
Depends on: 941673
The button should be showing up now.  The patch is actually simpler now as well - I made the gcli command hidden, and only added the single splitconsole command.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=9a26c078b1a9.
Attachment #8341857 - Attachment is obsolete: true
Attachment #8343769 - Flags: review?(mihai.sucan)
Comment on attachment 8343769 [details] [diff] [review]
webconsole-ui-updates.patch

Looks good to me. Glad to see this is landing!
Attachment #8343769 - Flags: review?(mihai.sucan) → review+
https://hg.mozilla.org/mozilla-central/rev/fc277a7f8d60
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Duplicate of this bug: 948062
Whiteboard: [good first verify]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.