Improve mute button usability

src/js/control-bar/mute-toggle.js

@@ -53,7 +53,15 @@ class MuteToggle extends Button {
    * @listens click
    */
   handleClick(event) {
-    this.player_.muted(this.player_.muted() ? false : true);
+    const vol = this.player_.volume();
+    const lastVolume = this.player_.lastVolume_();
+
+    if (vol === 0) {
+      this.player_.volume(lastVolume);
+      this.player_.muted(false);
+    } else {
+      this.player_.muted(this.player_.muted() ? false : true);
+    }
   }
 
   /**

src/js/control-bar/volume-control/volume-bar.js

@@ -25,7 +25,7 @@ class VolumeBar extends Slider {
    */
   constructor(player, options) {
     super(player, options);
-
+    this.on('slideractive', this.updateLastVolume_);
     this.on(player, 'volumechange', this.updateARIAAttributes);
     player.ready(() => this.updateARIAAttributes);
   }
@@ -112,6 +112,24 @@ class VolumeBar extends Slider {
     this.el_.setAttribute('aria-valuetext', volume + '%');
   }
 
+  /**
+   * When user starts dragging the VolumeBar, store the volume and listen for
+   * the end of the drag. When the drag ends, if the volume was set to zero,
+   * set lastVolume to the stored volume.
+   *
+   * @listens slideractive
+   * @private
+   */
+  updateLastVolume_() {
+    const volumeBeforeDrag = this.player_.volume();
+
+    this.one('sliderinactive', () => {
+      if (this.player_.volume() === 0) {
+        this.player_.lastVolume_(volumeBeforeDrag);
+      }
+    });
+  }
+
 }
 
 /**

src/js/player.js

@@ -335,6 +335,9 @@ class Player extends Component {
     // Set controls
     this.controls_ = !!options.controls;
 
+    // Set default values for lastVolume
+    this.cache_.lastVolume = 1;
+
     // Original tag settings stored in options
     // now remove immediately so native controls don't flash.
     // May be turned back on by HTML5 tech if nativeControlsForTouch is true
@@ -1812,6 +1815,10 @@ class Player extends Component {
       this.cache_.volume = vol;
       this.techCall_('setVolume', vol);
 
+      if (vol > 0) {
+        this.lastVolume_(vol);
+      }
+
       return;
     }
 
@@ -1872,6 +1879,28 @@ class Player extends Component {
     return this.techGet_('defaultMuted') || false;
   }
 
+  /**
+   * Get the last volume, or set it
+   *
+   * @param  {number} [percentAsDecimal]
+   *         The new last volume as a decimal percent:
+   *         - 0 is muted/0%/off
+   *         - 1.0 is 100%/full
+   *         - 0.5 is half volume or 50%
+   *
+   * @return {number}
+   *         the current value of lastVolume as a percent when getting
+   *
+   * @private
+   */
+  lastVolume_(percentAsDecimal) {
+    if (percentAsDecimal !== undefined && percentAsDecimal !== 0) {
+      this.cache_.lastVolume = percentAsDecimal;
+      return;
+    }
+    return this.cache_.lastVolume;
+  }
+
   /**
    * Check if current tech can support native fullscreen
    * (e.g. with built in controls like iOS, so not our flash swf)

test/unit/controls.test.js

@@ -92,3 +92,44 @@ QUnit.test('Fullscreen control text should be correct when fullscreenchange is t
   QUnit.equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode');
   player.dispose();
 });
+
+QUnit.test('Clicking MuteToggle when volume is above 0 should toggle muted property and not change volume', function(assert) {
+  const player = TestHelpers.makePlayer({ techOrder: ['html5'] });
+  const muteToggle = new MuteToggle(player);
+
+  assert.equal(player.volume(), 1, 'volume is above 0');
+  assert.equal(player.muted(), false, 'player is not muted');
+
+  muteToggle.handleClick();
+
+  assert.equal(player.volume(), 1, 'volume is same');
+  assert.equal(player.muted(), true, 'player is muted');
+});
+
+QUnit.test('Clicking MuteToggle when volume is 0 and muted is false should set volume to lastVolume and keep muted false', function(assert) {
+  const player = TestHelpers.makePlayer({ techOrder: ['html5'] });
+  const muteToggle = new MuteToggle(player);
+
+  player.volume(0);
+  assert.equal(player.lastVolume_(), 1, 'lastVolume is set');
+  assert.equal(player.muted(), false, 'player is muted');
+
+  muteToggle.handleClick();
+
+  assert.equal(player.volume(), 1, 'volume is set to lastVolume');
+  assert.equal(player.muted(), false, 'muted remains false');
+});
+
+QUnit.test('Clicking MuteToggle when volume is 0 and muted is true should set volume to lastVolume and sets muted to false', function(assert) {
+  const player = TestHelpers.makePlayer({ techOrder: ['html5'] });
+  const muteToggle = new MuteToggle(player);
+
+  player.volume(0);
+  player.muted(true);
+  player.lastVolume_(0.5);
+
+  muteToggle.handleClick();
+
+  assert.equal(player.volume(), 0.5, 'volume is set to lastVolume');
+  assert.equal(player.muted(), false, 'muted is set to false');
+});