1025057 - DevTools themes: thin toolbar button follow up

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Brian Grinstead [:bgrins]]

A few UI things I've noticed since Bug 942292:

1) The "connect" screen button doesn't look like a button anymore (it should be standalone and make sure the color is inherited)

2) The webconsole labels for 'logging' and 'security' bump directly up the icon

3) The debugger breadcrumbs scroll arrows are always visible when paused with a call stack even when it does not need to scroll.

Comment 1

[Brian Grinstead [:bgrins]]

Apparently (2) is not a regression from Bug 942292, but we may as well fix it here anyway

Comment 2

[Brian Grinstead [:bgrins]]

I'm fairly certain that (3) is just a rendering glitch, as the arrows go away once resized to be narrow enough for them to show up, then enlarged again.  To be more clear:

* Open browser to wide resolution
* Open debugger
* Go to http://htmlpad.org/debugger
* Press the button

Notice that the scroll arrows are visible but disabled.  Now, shrink the browser window until the scroll arrows are needed.  Then maximize the window again and they go away.

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: thin-toolbar-followup.patch

This addresses the button on the connect screen and the squished together icon and text on the web console filter buttons.

Comment 4

[Tim Nguyen :ntim]

There are a few extra stuff you might want to address as well :
- The breadcrumbs text is misaligned (http://images.devs-on.net/Image/gdQhu52TmsuL5Tmg-Region.png )
- Same for the computed view checkbox label (http://images.devs-on.net/Image/J4QjmIbKcPVDmUpA-Region.png )
- Also, the style editor options button doesn't have the same hover/active/open state as the debugger's

Comment 5

[Girish Sharma [:Optimizer]]

Also, the super short button height is looking really weird. specially for button which are supposed to be square are not squished! (the icon buttons, debugger, etc)

Not only icon buttons, but text buttons also look weird.

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Girish Sharma [:Optimizer] from comment #5)
> Also, the super short button height is looking really weird. specially for
> button which are supposed to be square are not squished! (the icon buttons,
> debugger, etc)
> 
> Not only icon buttons, but text buttons also look weird.

You are going to have to be more specific about that if you want any changes to be made.  Can you provide screenshots pointing out what looks weird?

Comment 7

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

Comment on attachment 8440909 [details] [diff] [review]
thin-toolbar-followup.patch

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

::: browser/devtools/framework/connect/connect.xhtml
@@ +28,5 @@
>            <span>&port;</span>
>            <input required="required" class="devtools-textinput" id="port" type="number" pattern="\d+"></input>
>          </label>
>          <label>
> +          <input class="devtools-toolbarbutton" standalone="true" id="submit" type="submit" value="&connect;"></input>

All nodes in this file seem to have class, then id, then other attributes. Might want to reorder these here to match.

Comment 8

[Brian Grinstead [:bgrins]]

Attachment: thin-toolbar-followup-part1.patch

Reordered attributes.  Marking this as part 1, will handle other things in another patch

Comment 9

[Girish Sharma [:Optimizer]]

(In reply to Brian Grinstead [:bgrins] from comment #6)
> (In reply to Girish Sharma [:Optimizer] from comment #5)
> > Also, the super short button height is looking really weird. specially for
> > button which are supposed to be square are not squished! (the icon buttons,
> > debugger, etc)
> > 
> > Not only icon buttons, but text buttons also look weird.
> 
> You are going to have to be more specific about that if you want any changes
> to be made.  Can you provide screenshots pointing out what looks weird?

For instance, debugger icon based buttons : http://i.snag.gy/QuHTk.jpg
Style editor's text based buttons : http://i.snag.gy/x9l5j.jpg

They all seem too much shorter , or maybe its just my eyes.

Also, I would expect a uniform normal/hover/active/focused state css (background, etc) across all buttons in the toolbox. Right now, text only buttons have a background color even when they are not hovered, while icons only buttons do not.

Comment 10

[Brian Grinstead [:bgrins]]

(In reply to Girish Sharma [:Optimizer] from comment #9)
> (In reply to Brian Grinstead [:bgrins] from comment #6)
> > (In reply to Girish Sharma [:Optimizer] from comment #5)
> > > Also, the super short button height is looking really weird. specially for
> > > button which are supposed to be square are not squished! (the icon buttons,
> > > debugger, etc)
> > > 
> > > Not only icon buttons, but text buttons also look weird.
> > 
> > You are going to have to be more specific about that if you want any changes
> > to be made.  Can you provide screenshots pointing out what looks weird?
> 
> For instance, debugger icon based buttons : http://i.snag.gy/QuHTk.jpg
> Style editor's text based buttons : http://i.snag.gy/x9l5j.jpg
> 
> They all seem too much shorter , or maybe its just my eyes.

They are quite shorter than they were before, but I think it's subjective whether that is good or not (we were trying to shorten the height of the toolbar, so the buttons had to shrink).  Might help to make them a few px more narrow.

> Also, I would expect a uniform normal/hover/active/focused state css
> (background, etc) across all buttons in the toolbox. Right now, text only
> buttons have a background color even when they are not hovered, while icons
> only buttons do not.

At first we tried to not have a background for any of the buttons, but the text buttons *need* to have a background (otherwise they just look like labels and it is very bad UX).  That does lead to the question of whether icon buttons should also have the background to be consistent.  That's something to talk with Darrin about, I think.  I've based the implementation quite a bit off of https://bugzilla.mozilla.org/attachment.cgi?id=8385335, with some adjustments that we had to make while working through Bug 942292.

Comment 11

[Brian Grinstead [:bgrins]]

(In reply to Tim Nguyen [:ntim] from comment #4)
> There are a few extra stuff you might want to address as well :
> - The breadcrumbs text is misaligned
> (http://images.devs-on.net/Image/gdQhu52TmsuL5Tmg-Region.png )
> - Same for the computed view checkbox label
> (http://images.devs-on.net/Image/J4QjmIbKcPVDmUpA-Region.png )
> - Also, the style editor options button doesn't have the same
> hover/active/open state as the debugger's

Thanks, I've got points 2 and 3 done locally, and it looks like the extreme misalignment on breadcrumbs may be a Windows issue - I'll see if I can reproduce.

Comment 12

[Brian Grinstead [:bgrins]]

Attachment: thin-toolbar-followup-part2.patch

Addresses issues from Comment 4 (I have a try push in which I will confirm on Windows, but I tested with inspector on a Nightly build of windows): https://tbpl.mozilla.org/?tree=Try&rev=6ebf07218f3f).

I made some alignment changes and also changed some things about the options cog in the style editor to make it have an active state when opened (like the debugger cog does).

Comment 13

[Brian Grinstead [:bgrins]]

Attachment: thin-toolbar-followup-part2.patch

Sorry, I uploaded the part1 patch earlier.  This should be right

Comment 14

[Brian Grinstead [:bgrins]]

Attachment: thin-toolbar-followup-part2.patch

rebased

Comment 15

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/136afe37f6ff
https://hg.mozilla.org/integration/fx-team/rev/86fbf924bf86

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/136afe37f6ff
https://hg.mozilla.org/mozilla-central/rev/86fbf924bf86