@heff fixed the inline volume control and made it the default. closes #…

…2553
videojs · Sep 10, 2015 · 4dadd01 · 4dadd01
1 parent 0bd7538
commit 4dadd01
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 77 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -124,6 +124,7 @@ CHANGELOG
 * @heff fixed the UI for live streams ([view](https://github.com/videojs/video.js/pull/2557))
 * @gkatsev updated opacity of caption settings background color ([view](https://github.com/videojs/video.js/pull/2573))
 * @gkatsev made all sass variables !default ([view](https://github.com/videojs/video.js/pull/2574))
+* @heff fixed the inline volume control and made it the default ([view](https://github.com/videojs/video.js/pull/2553))
 
 --------------------
 

diff --git a/src/css/components/_volume.scss b/src/css/components/_volume.scss
@@ -28,12 +28,12 @@
   margin: 1.35em;
 }
 
-.video-js .vjs-volume-bar.vjs-slider-horizontal {
+.vjs-volume-bar.vjs-slider-horizontal {
   width: 5em;
   height: 0.3em;
 }
 
-.video-js .vjs-volume-bar.vjs-slider-vertical {
+.vjs-volume-bar.vjs-slider-vertical {
   width: 0.3em;
   height: 5em;
 }
@@ -54,7 +54,7 @@
   }
 }
 
-.video-js .vjs-slider-vertical .vjs-volume-level {
+.vjs-slider-vertical .vjs-volume-level {
   width: 0.3em;
 
   // Volume handle
@@ -63,7 +63,7 @@
     left: -0.3em;
   }
 }
-.video-js .vjs-slider-horizontal .vjs-volume-level {
+.vjs-slider-horizontal .vjs-volume-level {
   height: 0.3em;
 
   // Volume handle
@@ -74,28 +74,34 @@
 }
 
 /* Assumes volume starts at 1.0. */
-.video-js .vjs-volume-bar.vjs-slider-vertical .vjs-volume-level { height: 100%; }
-.video-js .vjs-volume-bar.vjs-slider-horizontal .vjs-volume-level { width: 100%; }
+.vjs-volume-bar.vjs-slider-vertical .vjs-volume-level {
+  height: 100%;
+}
+
+.vjs-volume-bar.vjs-slider-horizontal .vjs-volume-level {
+  width: 100%;
+}
 
 /* The volume menu button is like menu buttons (captions/subtitles) but works
 a little differently. It needs to be possible to tab to the volume slider
 without hitting space bar on the menu button. To do this we're not using
 display:none to hide the slider menu by default, and instead setting the
 width and height to zero. */
-.video-js .vjs-menu-button-popup.vjs-volume-menu-button .vjs-menu {
+.vjs-menu-button-popup.vjs-volume-menu-button .vjs-menu {
   display: block;
   width: 0;
   height: 0;
   border-top-color: transparent;
 }
-.video-js .vjs-volume-menu-button.vjs-volume-menu-button-vertical .vjs-menu {
+
+.vjs-menu-button-popup.vjs-volume-menu-button-vertical .vjs-menu {
   left: 0.5em;
 }
-.video-js .vjs-volume-menu-button-popup.vjs-volume-menu-button-horizontal .vjs-menu {
+.vjs-menu-button-popup.vjs-volume-menu-button-horizontal .vjs-menu {
   left: -2em;
 }
 
-.video-js .vjs-menu-button.vjs-menu-button-popup.vjs-volume-menu-button .vjs-menu .vjs-menu-content {
+.vjs-menu-button-popup.vjs-volume-menu-button .vjs-menu-content {
   height: 0;
   width: 0;
 
@@ -104,27 +110,14 @@ width and height to zero. */
   overflow-y: hidden;
 }
 
-.video-js .vjs-volume-menu-button:hover .vjs-menu,
-.video-js .vjs-volume-menu-button .vjs-menu.vjs-lock-showing {
-  // border-top-color: rgba(7, 40, 50, 0.5); /* Same as ul background */
-}
-
-.video-js .vjs-volume-menu-button.vjs-volume-menu-button-vertical:hover .vjs-menu .vjs-menu-content,
-.video-js .vjs-volume-menu-button.vjs-volume-menu-button-vertical .vjs-menu.vjs-lock-showing .vjs-menu-content {
+.vjs-volume-menu-button-vertical:hover .vjs-menu-content,
+.vjs-volume-menu-button-vertical .vjs-lock-showing .vjs-menu-content {
   height: 8em;
   width: 2.9em;
 }
 
-
-.video-js .vjs-volume-menu-button.vjs-volume-menu-button-horizontal:hover .vjs-menu .vjs-menu-content,
-.video-js .vjs-volume-menu-button.vjs-volume-menu-button-horizontal .vjs-menu.vjs-lock-showing .vjs-menu-content {
+.vjs-volume-menu-button-horizontal:hover .vjs-menu-content,
+.vjs-volume-menu-button-horizontal .vjs-lock-showing .vjs-menu-content {
   height: 2.9em;
   width: 8em;
 }
-
-// By default, all menu items are shown, but we hide .vjs-mute-control and .vjs-volume-control
-// so that the volume menu button is the only visible volume control.
-.video-js .vjs-mute-control,
-.video-js .vjs-volume-control {
-  display: none;
-}
diff --git a/src/css/components/menu/_menu-inline.scss b/src/css/components/menu/_menu-inline.scss
@@ -1,17 +1,30 @@
 .video-js .vjs-menu-button-inline {
   @include transition(all 0.4s);
-
   overflow: hidden;
 }
 
-.video-js .vjs-menu-button.vjs-menu-button-inline:before {
+.video-js .vjs-menu-button-inline:before {
   // Icon pseudoelement has a different base font size (1.8em), so we need to
   // account for that in the width. 4em (standard button width) divided by 1.8
   // to get the same button width as normal.
   width: 2.222222222em;
 }
 
-.video-js .vjs-menu-button-inline .vjs-menu {
+// Hover state
+.video-js .vjs-menu-button-inline:hover,
+.video-js .vjs-menu-button-inline:focus,
+.video-js .vjs-menu-button-inline.vjs-slider-active,
+.video-js.vjs-no-flex .vjs-menu-button-inline {
+  // This width is currently specific to the inline volume bar.
+  width: 12em;
+}
+// Don't transition when tabbing in reverse to the volume menu
+// because it looks weird
+.video-js .vjs-menu-button-inline.vjs-slider-active {
+  @include transition(none);
+}
+
+.vjs-menu-button-inline .vjs-menu {
   opacity: 0;
   height: 100%;
   width: auto;
@@ -23,34 +36,32 @@
   padding: 0;
   margin: 0;
 
-  @include transition(all 0.2s);
+  @include transition(all 0.4s);
+}
+
+.vjs-menu-button-inline:hover .vjs-menu,
+.vjs-menu-button-inline:focus .vjs-menu,
+.vjs-menu-button-inline.vjs-slider-active .vjs-menu {
+  display: block;
+  opacity: 1;
 }
 
-.video-js.vjs-no-flex .vjs-menu-button-inline .vjs-menu {
+.vjs-no-flex .vjs-menu-button-inline .vjs-menu {
+  display: block;
+  opacity: 1;
   position: relative;
-  width: 0;
+  width: auto;
 }
 
-.video-js.vjs-no-flex .vjs-menu-button-inline:hover .vjs-menu {
+.vjs-no-flex .vjs-menu-button-inline:hover .vjs-menu,
+.vjs-no-flex .vjs-menu-button-inline:focus .vjs-menu,
+.vjs-no-flex .vjs-menu-button-inline.vjs-slider-active .vjs-menu {
   width: auto;
 }
 
-.video-js .vjs-menu-button-inline .vjs-menu .vjs-menu-content {
+.vjs-menu-button-inline .vjs-menu-content {
   width: auto;
   height: 100%;
   margin: 0;
-
   overflow: hidden;
 }
-
-// Hover state
-.video-js .vjs-menu-button-inline:hover {
-  // This width is currently specific to the inline volume bar.
-  width: 10em;
-}
-
-.video-js .vjs-menu-button.vjs-menu-button-inline:hover .vjs-menu,
-.video-js .vjs-menu-button-inline .vjs-menu.vjs-lock-showing {
-  display: block;
-  opacity: 1;
-}
diff --git a/src/css/components/menu/_menu-popup.scss b/src/css/components/menu/_menu-popup.scss
@@ -1,4 +1,4 @@
-.video-js .vjs-menu-button-popup .vjs-menu {
+.vjs-menu-button-popup .vjs-menu {
   display: none;
   position: absolute;
   bottom: 0;
@@ -10,18 +10,18 @@
 }
 
 /* Button Pop-up Menu */
-.video-js .vjs-menu-button-popup .vjs-menu-content {
+.vjs-menu-button-popup .vjs-menu-content {
   @include background-color-with-alpha($primary-bg, 0.7);
 }
 
-.video-js .vjs-menu-button-popup .vjs-menu .vjs-menu-content {
+.vjs-menu-button-popup .vjs-menu-content {
   position: absolute;
   width: 10em;
   bottom: 1.5em; /* Same bottom as vjs-menu border-top */
   max-height: 15em;
 }
 
-.video-js .vjs-menu-button.vjs-menu-button-popup:hover .vjs-menu,
-.video-js .vjs-menu-button-popup .vjs-menu.vjs-lock-showing {
+.vjs-menu-button-popup:hover .vjs-menu,
+.vjs-menu-button-popup .vjs-menu.vjs-lock-showing {
   display: block;
 }
diff --git a/src/css/video-js.scss b/src/css/video-js.scss
@@ -7,17 +7,18 @@
 @import "components/big-play";
 @import "components/button";
 
+@import "components/menu/menu";
+@import "components/menu/menu-popup";
+@import "components/menu/menu-inline";
+
 @import "components/control-bar";
 @import "components/control";
 @import "components/control-spacer";
 
 @import "components/progress";
 @import "components/slider";
-@import "components/volume";
 
-@import "components/menu/menu";
-@import "components/menu/menu-popup";
-@import "components/menu/menu-inline";
+@import "components/volume";
 
 @import "components/poster";
 @import "components/live";

diff --git a/src/js/control-bar/control-bar.js b/src/js/control-bar/control-bar.js
@@ -55,8 +55,6 @@ ControlBar.prototype.options_ = {
     'remainingTimeDisplay',
     'customControlSpacer',
     'playbackRateMenuButton',
-    'muteToggle',
-    'volumeControl',
     'chaptersButton',
     'subtitlesButton',
     'captionsButton',

diff --git a/src/js/control-bar/volume-menu-button.js b/src/js/control-bar/volume-menu-button.js
@@ -19,19 +19,24 @@ import VolumeBar from './volume-control/volume-bar.js';
 class VolumeMenuButton extends MenuButton {
 
   constructor(player, options={}){
+    // Default to inline
+    if (options.inline === undefined) {
+      options.inline = true;
+    }
+
     // If the vertical option isn't passed at all, default to true.
     if (options.vertical === undefined) {
-      // If an inline volumeMenuButton is used, we should default to using a horizontal
-      // slider for obvious reasons.
+      // If an inline volumeMenuButton is used, we should default to using
+      // a horizontal slider for obvious reasons.
       if (options.inline) {
         options.vertical = false;
       } else {
         options.vertical = true;
       }
     }
 
-    // The vertical option needs to be set on the volumeBar as well, since that will
-    // need to be passed along to the VolumeBar constructor
+    // The vertical option needs to be set on the volumeBar as well,
+    // since that will need to be passed along to the VolumeBar constructor
     options.volumeBar = options.volumeBar || {};
     options.volumeBar.vertical = !!options.vertical;
 
@@ -42,17 +47,24 @@ class VolumeMenuButton extends MenuButton {
     this.on(player, 'loadstart', this.volumeUpdate);
 
     // hide mute toggle if the current tech doesn't support volume control
-    if (player.tech && player.tech['featuresVolumeControl'] === false) {
-      this.addClass('vjs-hidden');
-    }
-    this.on(player, 'loadstart', function(){
-      if (player.tech['featuresVolumeControl'] === false) {
+    function updateVisibility() {
+      if (player.tech && player.tech['featuresVolumeControl'] === false) {
         this.addClass('vjs-hidden');
       } else {
         this.removeClass('vjs-hidden');
       }
+    }
+
+    updateVisibility.call(this);
+    this.on(player, 'loadstart', updateVisibility);
+
+    this.on(this.volumeBar, ['slideractive', 'focus'], function(){
+      this.addClass('vjs-slider-active');
+    });
+
+    this.on(this.volumeBar, ['sliderinactive', 'blur'], function(){
+      this.removeClass('vjs-slider-active');
     });
-    this.addClass('vjs-menu-button');
   }
 
   /**
@@ -83,15 +95,11 @@ class VolumeMenuButton extends MenuButton {
       contentElType: 'div'
     });
 
-    let vc = new VolumeBar(this.player_, this.options_.volumeBar);
+    let vb = new VolumeBar(this.player_, this.options_.volumeBar);
 
-    vc.on('focus', function() {
-      menu.lockShowing();
-    });
-    vc.on('blur', function() {
-      menu.unlockShowing();
-    });
-    menu.addChild(vc);
+    menu.addChild(vb);
+
+    this.volumeBar = vb;
     return menu;
   }
 

diff --git a/src/js/slider/slider.js b/src/js/slider/slider.js
@@ -66,7 +66,9 @@ class Slider extends Component {
   handleMouseDown(event) {
     event.preventDefault();
     Dom.blockTextSelection();
+
     this.addClass('vjs-sliding');
+    this.trigger('slideractive');
 
     this.on(document, 'mousemove', this.handleMouseMove);
     this.on(document, 'mouseup', this.handleMouseUp);
@@ -90,7 +92,9 @@ class Slider extends Component {
    */
   handleMouseUp() {
     Dom.unblockTextSelection();
+
     this.removeClass('vjs-sliding');
+    this.trigger('sliderinactive');
 
     this.off(document, 'mousemove', this.handleMouseMove);
     this.off(document, 'mouseup', this.handleMouseUp);