Skip to content
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

Fix attempt - Issue #2895 #3045

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Conversation

keikari
Copy link
Contributor

@keikari keikari commented Jan 24, 2024

Fixes

Issue Number: #2895

New behavior:
-queue pauses playing on end of the last video
-the second item from the queue doesn't start playing in middle of other playlists

Seems that a second onVideoEnded function gets attached to the player, when a second queue item is added. (Doesn't happen with following video adds)
And one of those sticked around, and got called when not wanted. (I didn't quite understood how this exactly worked.)

Clearing onEndeds before adding a new one, seems to fix the behavior.
Also clearing them didn't seem to break anything. Looping and shuffle also kept working the same than before.

@keikari
Copy link
Contributor Author

keikari commented Jan 24, 2024

I'm not quite sure what the lint wants. The error doesn't seem to be related to my changes?

@tzarebczan tzarebczan merged commit 624ac23 into OdyseeTeam:master Jan 25, 2024
1 of 2 checks passed
@infinite-persistence
Copy link
Collaborator

infinite-persistence commented Feb 20, 2024

I think adding the off definition to the Player type should fix all the lint issues

Edit: Update the second parameter to optional. https://docs.videojs.com/player#off
It's somewhat a valid complaint -- not providing the second parameter removes all registered handlers. That may or may not be what we want at times, but seems fine for this case.

@keikari keikari mentioned this pull request Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants