fix: reset to a play/pause button when seeking after ended #4614
Conversation
src/js/control-bar/play-toggle.js
Outdated
| // depending on the current state | ||
| this.removeClass('vjs-ended'); | ||
|
|
||
| if (this.player_.paused()) { |
There was a problem hiding this comment.
if we're ended, we should already be paused, so, I think we can probably simply this. Maybe we can just call the handlePause handler so we don't duplicate work?
There was a problem hiding this comment.
I was thinking about it, but it seemed a bit weird to be calling a function made for being called by events manually. If you don't see a problem with it. I can go ahead and do it though.
There was a problem hiding this comment.
probably ok? Can always pull it out into a separate function that both methods can call.
brandonocasey
force-pushed
the
fix/replay-button-seek
branch
from
c40307e
to
97ac11c
Compare
brandonocasey
force-pushed
the
fix/replay-button-seek
branch
from
97ac11c
to
4aad07d
Compare
|
@gkatsev updated |
src/js/control-bar/play-toggle.js
Outdated
| */ | ||
| handleSeeked(event) { | ||
| // check to see if the video has ended yet | ||
| if (!this.hasClass('vjs-ended')) { |
There was a problem hiding this comment.
oh, we probably should use this.ended() instead of checking the class.
There was a problem hiding this comment.
Yeah I guess that will work, should be this.player_.ended() right?
| // remove the ended class | ||
| this.removeClass('vjs-ended'); | ||
|
|
||
| if (this.player_.paused()) { |
There was a problem hiding this comment.
why would we not be paused when we're ended?
There was a problem hiding this comment.
Without this check clicking the button (which causes a play and then a seek to 0) would cause the button to show up as a play button, even when the player is playing.
src/js/control-bar/play-toggle.js
Outdated
| this.removeClass('vjs-paused'); | ||
| this.addClass('vjs-playing'); | ||
|
|
||
| if (!this.hasClass('vjs-playing')) { |
There was a problem hiding this comment.
why not just keep this as unconditional?
src/js/control-bar/play-toggle.js
Outdated
| */ | ||
| handleEnded(event) { | ||
| this.removeClass('vjs-playing'); | ||
| this.addClass('vjs-ended'); | ||
|
|
||
| if (!this.hasClass('vjs-ended')) { |
There was a problem hiding this comment.
why not just keep this as unconditional?
There was a problem hiding this comment.
moved them back to unconditional as this is already a check in addClass.
src/js/control-bar/play-toggle.js
Outdated
| @@ -83,20 +105,33 @@ class PlayToggle extends Button { | |||
| */ | |||
| handlePause(event) { | |||
| this.removeClass('vjs-playing'); | |||
| this.addClass('vjs-paused'); | |||
|
|
|||
| if (!this.hasClass('vjs-paused')) { | |||
There was a problem hiding this comment.
why not just keep this as unconditional?
brandonocasey
force-pushed
the
fix/replay-button-seek
branch
from
53daff5
to
777157c
Compare
brandonocasey
force-pushed
the
fix/replay-button-seek
branch
from
777157c
to
d5b512f
Compare
Description
When seeking backwards after the video has ended, the
vjs-endedclass will remain on the video until play is pushed. This means the the play button will still show as a replay button, which seems incorrect to me.Specific Changes proposed
This PR removes
vjs-endedon anyseekedevent on the player and resets the button into a play/pause state depending upon the player state.Requirements Checklist