Closed Bug 780826 Opened 9 years ago Closed 8 years ago

Hover and active state for the video controls

Categories

(Toolkit :: Video/Audio Controls, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: jaws, Assigned: jaws)

References

Details

Attachments

(2 files, 5 obsolete files)

Attached patch Patch (obsolete) — Splinter Review
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.
Attachment #649549 - Flags: review?(shorlander)
Attached video Video of patch (obsolete) —
Attached patch Patch v2 (obsolete) — Splinter Review
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).
Attachment #649549 - Attachment is obsolete: true
Attachment #649553 - Attachment is obsolete: true
Attachment #649549 - Flags: review?(shorlander)
Attachment #651215 - Flags: review?(shorlander)
Attached video Video of patch v2
Attachment #651216 - Flags: ui-review?(shorlander)
Attached patch Patch v2.1 (obsolete) — Splinter Review
This is the same as v2 except it also adds active styling for OS X (no hover on OS X).
Attachment #651215 - Attachment is obsolete: true
Attachment #651215 - Flags: review?(shorlander)
Attachment #651230 - Flags: review?(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.
Attachment #651230 - Flags: review?(dao) → review-
Attachment #651216 - Flags: ui-review?(shorlander)
The image would be displayed at full opacity on hover.
Oh, I see you were commenting about pinstripe. I'm going to rethink this approach and I think Stephen has some ideas too.
Depends on: 809278
Attached patch Patch v2.2 (obsolete) — Splinter Review
Attachment #651230 - Attachment is obsolete: true
Attachment #699292 - Flags: review?(dao)
Summary: Add a hover and active state for the video controls buttons in Windows and Linux → Add a hover and active state for the video controls buttons in Windows and Linux, and an active state on OS X
Attachment #699292 - Flags: review?(dolske)
Attached patch Patch v2.3Splinter Review
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
Attachment #699292 - Attachment is obsolete: true
Attachment #699292 - Flags: review?(dolske)
Attachment #699292 - Flags: review?(dao)
Attachment #702473 - Flags: review?(dolske)
Attachment #702473 - Flags: review?(dao)
Attachment #702473 - Flags: review?(shorlander)
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?
Attachment #702473 - Flags: review?(dolske)
Attachment #702473 - Flags: review?(dao)
Attachment #702473 - Flags: review+
(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
Blocks: 832010
Summary: Add a hover and active state for the video controls buttons in Windows and Linux, and an active state on OS X → Hover and active state for the video controls
https://hg.mozilla.org/mozilla-central/rev/a50549c5caca
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.