Skip to content

Commit

Permalink
feat: unmute goes back to previously selected volume (#3942)
Browse files Browse the repository at this point in the history
If a user changed the volume to zero either via the mouse or keyboard, clicking unmute will now restore the volume back to this last position. Previously, the mute and volume values were completely not linked.

Fixes #3909.
  • Loading branch information
kevinlitchfield authored and gkatsev committed Jan 31, 2017
1 parent 5b8b41e commit cb42fcf
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 2 deletions.
10 changes: 9 additions & 1 deletion src/js/control-bar/mute-toggle.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

/**
Expand Down
20 changes: 19 additions & 1 deletion src/js/control-bar/volume-control/volume-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
});
}

}

/**
Expand Down
29 changes: 29 additions & 0 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1812,6 +1815,10 @@ class Player extends Component {
this.cache_.volume = vol;
this.techCall_('setVolume', vol);

if (vol > 0) {
this.lastVolume_(vol);
}

return;
}

Expand Down Expand Up @@ -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)
Expand Down
41 changes: 41 additions & 0 deletions test/unit/controls.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

0 comments on commit cb42fcf

Please sign in to comment.