780826 - Hover and active state for the video controls

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, enhancement)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

To match with platform expectations, we should have hover and active states for the buttons in the video + audio controls.

The attached patch makes the icons for the buttons slightly darker (by using a lower opacity) in the default state, then brighter in the hover state (by increasing the icons to full opacity), then holding on to the brighter icon but darkening the button region on activation of the button (by setting a translucent black background-color).

I will attach a short screencast demo to this bug for review.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Video of patch

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2

The previous patch changed the background color and showed some lines that I didn't think belonged. The background color would go right up against other UI elements which I thought felt awkward.

This new patch only modifies the opacity of the icons, making them go from neutral (opacity:.7), to bright in hover (opacity:1), then to dark in hover:active (opacity:.4).

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Video of patch v2

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2.1

This is the same as v2 except it also adds active styling for OS X (no hover on OS X).

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 651230 [details] [diff] [review]
Patch v2.1

>--- a/toolkit/themes/pinstripe/global/media/videocontrols.css
>+++ b/toolkit/themes/pinstripe/global/media/videocontrols.css
>@@ -18,16 +18,24 @@
>   background-repeat: no-repeat;
>   background-position: center;
>   -moz-appearance: none; /* Remove the native button appearance and styling */
>   margin: 0;
>   padding: 0;
>   min-height: 28px;
>   min-width: 28px;
>   border: none;
>+  opacity: 0.7;
>+}
>+
>+.playButton:hover:active,
>+.replayButton:hover:active,
>+.muteButton:hover:active,
>+.fullscreenButton:hover:active {
>+  opacity: 0.4;
> }

It seems strange that the image would never be displayed at full opacity.

Also, this patch doesn't seem to apply at all.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

The image would be displayed at full opacity on hover.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Oh, I see you were commenting about pinstripe. I'm going to rethink this approach and I think Stephen has some ideas too.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2.2

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2.3

This patch adds the hover state to the buttons on OS X based on the following conversation:

3:21 PM <dolske> jaws: I'd be curious what shorlander / UX thinks... For this and other "browser UI in content", I'm sorta thinking we shouldn't have differing platform conventions... instead, a unified "web convention"
3:21 PM <jaws> dolske: i was going based off of buttons on web pages. on os x, i don't remember seeing <input type='button'> having a hover state
3:22 PM <dolske> (which in this case I'd probably lean towards having hover states for videocontrols on all platforms)
3:22 PM <dolske> jaws: youtube's player buttons change on hover. :)
3:22 PM <dolske> (and I'm on Mac)
3:23 PM <jaws> i can't complain with that viewpoint, i like the hover affordance :)
3:23 PM <jaws> i can update the patch
3:34 PM <shorlander> jaws, dolske: Yeah I don't have a problem with a hover state there on OSX
3:34 PM <jaws> shorlander: ok cool

Comment 10

[Justin Dolske [:Dolske]]

Comment on attachment 702473 [details] [diff] [review]
Patch v2.3

r+ assuming shorlander's happy with the visual appearance. One thought is that we might actually want brighter images, so that the unhovered state matches the current appearance. (Unfortunately one can't use "opacity: 1.2" to make an image brighter. ;)

Also, should the big click-to-play button also get a hover+active state?

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Justin Dolske [:Dolske] from comment #10)
> Comment on attachment 702473 [details] [diff] [review]
> Patch v2.3
> 
> r+ assuming shorlander's happy with the visual appearance. One thought is
> that we might actually want brighter images, so that the unhovered state
> matches the current appearance. (Unfortunately one can't use "opacity: 1.2"
> to make an image brighter. ;)

I filed bug 832010 to get some brighter images.

> Also, should the big click-to-play button also get a hover+active state?

The click-to-play button has a hover state, but adding a :hover:active state isn't so easy because of the way that we set those images (background-image + opacity) which also affects the textured background.

Landed on inbound, https://hg.mozilla.org/integration/mozilla-inbound/rev/a50549c5caca

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/a50549c5caca