-
Notifications
You must be signed in to change notification settings - Fork 7.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve mute button usability #3942
Changes from 5 commits
a0d5c96
b2212a1
0095835
ad8dd5d
3bb6419
500e692
c530565
ffac358
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
*/ | ||
updateLastVolume() { | ||
const volumeBeforeDrag = this.player_.volume(); | ||
|
||
this.on('sliderinactive', function setLastVolume() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can actually use this.one('sliderinactive', () => {
// current function goes here
}); |
||
if (this.player_.volume() === 0) { | ||
this.player_.lastVolume_(volumeBeforeDrag); | ||
} | ||
this.off('sliderinactive', setLastVolume); | ||
}); | ||
} | ||
|
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,26 @@ 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 | ||
*/ | ||
lastVolume_(percentAsDecimal) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should have an |
||
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) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably want this function to be private (only used internally) just like
Player#lastVolume_()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also,
@private
jsdoc directive