646713 - Buttons on Win7 don't have on-hover blue glow, are too small

Status: RESOLVED INACTIVE

(Toolkit :: Add-ons Manager, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Regression from bug 623199, which changed the style of the buttons on Win7.

All buttons should have a blue glow hover effect, like the browser navbar buttons do.

Also, the header buttons look too small - they're missing about a pixel worth of padding on each side.

Comment 1

[Dão Gottwald [:dao]]

I don't think you want to follow the nav bar here. The glow seemed like a misfit especially for the labeled buttons.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Feels like it needs some feedback when you hover other them, even if its not the same blue glow that the navbar buttons get. However, given the header buttons are meant to look/feel like the navbar buttons, the blue glow felt right there. 

I agree that the blue glow may not make as much sense for the addon control buttons, but they feel lifeless without a hover effect (especially with the whole list item having a hover state with bug 625465).

Also note that bug 625465 makes the "more" link into a button, and the patch there currently makes that button have a hover state.

Boriss: thoughts?

Comment 4

[Peter Henkel [:Terepin]]

I have a proposal: Can we highlight only icons with their own color?

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #4)
> I have a proposal: Can we highlight only icons with their own color?

Not sure what you mean by this - could you explain?

Comment 6

[Peter Henkel [:Terepin]]

Normally a whole button is highlighted. In AOM, why not highlight just icons? Since they're black we can use a lighter black glow coming from them. Technical: Is it possible to mimic their shape? For example, glow of arrow will look like arrow.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #6)
> Normally a whole button is highlighted. In AOM, why not highlight just
> icons? Since they're black we can use a lighter black glow coming from them.
> Technical: Is it possible to mimic their shape? For example, glow of arrow
> will look like arrow.

Oh! I think that's technically possible - I *think* box-shadow follows images like that.

However, not all buttons have an icon. Currently, all the buttons at the top of the addons manager do have an icon - but I'm in the process of unifying all in-content UI to share a common style (see bug 658431 & bug 658530). And not all pages will use icons there (it's not always possible or wanted). 

Additionally, it would break from the style used in the navigation toolbar, which these buttons essentially replace.

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

This updates the style of the top toolbar buttons to match the slightly updated style of the navbar buttons in Firefox. Found the back/forward icons were outdated also, so updated those.

Also updates the normal buttons, adding visual feedback to mouse hover.

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 555069 [details] [diff] [review]
Patch v1

>--- a/toolkit/themes/winstripe/global/inContentUI.css
>+++ b/toolkit/themes/winstripe/global/inContentUI.css

>+  button:not([disabled="true"]):not(:active):hover,
>+  menulist:not([open="true"]):not([disabled="true"]):not(:active):hover,
>+  colorpicker[type="button"]:not([open="true"]):not([disabled="true"]):not(:active):hover {
>+    background-color: hsla(190,60%,70%,.5);
>+    border-color: hsla(190,50%,65%,.8) hsla(190,50%,50%,.8) hsla(190,50%,40%,.8);
>+    box-shadow: 0 0 0 1px rgba(255,255,255,.3) inset,
>+                0 0 0 1.5px rgba(255,255,255,.1) inset,
>+                0 0 3.5px hsl(190,90%,80%);
>+    -moz-transition: background-color .4s ease-in,
>+                    border-color .3s ease-in,
>+                     box-shadow .3s ease-in;
>+  }

The context for these buttons is quite different from the navbar; there's no point really in copying everything we're doing there. So here's what I think should be changed:

- I don't think we should have the fuzzy outer glow.

- The transition feels too slow to me. For testing I halved and quartered the durations, both of which felt more responsive.

- Just for fun I stopped changing the border-color on hover, which looked good to me as well, although I'm more indifferent about this.

Comment 10

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2

Indeed, that does feel better.

It does look good without the border changing, but I do prefer the feel of having the border change. Something about not changing the border makes it feel unfinished to me.

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 555647 [details] [diff] [review]
Patch v2

>--- a/toolkit/themes/winstripe/global/inContentUI.css
>+++ b/toolkit/themes/winstripe/global/inContentUI.css

>+    box-shadow: 0 0 0 1px rgba(255,255,255,.3) inset,
>+                0 0 0 1.5px rgba(255,255,255,.1) inset;

Why is this different from the non-hover state?

Comment 12

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Dão Gottwald [:dao] from comment #11)
> Why is this different from the non-hover state?

Direct copy from the values in browser.css - normal and hover states use slightly different inset shadows there too. Extremely small perceivable difference though, so I'll remove it.

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 555647 [details] [diff] [review]
Patch v2

>--- a/toolkit/themes/winstripe/mozapps/extensions/extensions.css
>+++ b/toolkit/themes/winstripe/mozapps/extensions/extensions.css

> .header-button {
>   -moz-appearance: none;
>-  padding: 1px 3px;
>-  color: #444;
>-  text-shadow: 0 0 3px white;
>-  background: -moz-linear-gradient(rgba(251, 252, 253, 0.95), rgba(246, 247, 248, 0) 49%, 
>-                                   rgba(211, 212, 213, 0.45) 51%, rgba(225, 226, 229, 0.3));
>+  padding: 2px 3px;
>+  background: rgba(151,152,153,.05)
>+              -moz-linear-gradient(rgba(251,252,253,.95), rgba(246,247,248,.47) 49%, 
>+                                   rgba(231,232,233,.45) 51%, rgba(225,226,229,.3));
>   background-clip: padding-box;
>   border-radius: 3.5px;
>-  border: 1px solid rgba(31, 64, 100, 0.4);
>-  border-top-color: rgba(31, 64, 100, 0.3);
>-  box-shadow: 0 0 0 1px rgba(255, 255, 255, 0.25) inset,
>-              0 0 2px 1px rgba(255, 255, 255, 0.25) inset;
>+  border: 1px solid;
>+  border-color: rgba(0,0,0,.12) rgba(0,0,0,.19) rgba(0,0,0,.38);
>+  box-shadow: 0 0 0 1px rgba(255,255,255,.3) inset,
>+              0 0 0 2px rgba(255,255,255,.1) inset;
>+  color: black;
>+  text-shadow: 0 0 2px white;
>+}

> .header-button:not([disabled="true"]):active:hover,
> .header-button[open="true"] {
>-  background-color: rgba(61, 76, 92, 0.2);
>-  border-color: rgba(39, 53, 68, 0.5);
>-  box-shadow: 0 0 3px 1px rgba(39, 53, 68, 0.2) inset;
>+  background-color: transparent;
>+  border-color: rgba(0,0,0,.65) rgba(0,0,0,.55) rgba(0,0,0,.5);
>+  box-shadow: 0 0 6.5px rgba(0,0,0,.4) inset,
>+              0 0 2px rgba(0,0,0,.4) inset,
>+              0 1px 0 rgba(255,255,255,.4);
>+  text-shadow: none;
> }

Most of what you're undoing here (background, borders, shadows etc.) were intentional changes explicitly requested in bug 623199 (e.g. attachment 501307 [details]).

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Existing button style

Comment 15

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Toolbar buttons - normal

Comment 16

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Toolbar buttons - hover

Comment 17

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Normal button - hover

Comment 18

[Stephen Horlander [:shorlander]]

Comment on attachment 557350 [details]
Normal button - hover

As with bug 676795 I think we should move towards something more neutral and less heavy with these buttons. This will be inline with the new theme work as well.

HTML Mockup: http://people.mozilla.com/~shorlander/incontent-hover-buttons/incontentHoverButtons.html

Comment 19

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v3

Comment 20

[Dão Gottwald [:dao]]

Comment on attachment 565094 [details] [diff] [review]
Patch v3

>+    -moz-transition-duration: 10ms;

Is this actually visible?

> .header-button[disabled="true"] > .toolbarbutton-icon {
>-  opacity: 0.4;
>+  opacity: 1;
> }

Does some other rule reduce the icon's opacity? If not, this is a no-op.

Comment 21

[Dão Gottwald [:dao]]

Comment on attachment 565094 [details] [diff] [review]
Patch v3

> .header-button {
>   -moz-appearance: none;
>-  padding: 1px 3px;
>+  padding: 2px 3px;

This makes the buttons two pixels taller than the search field.

Comment 22

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.