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

Implement videojs-abloop in the video player #656

Closed
wants to merge 2 commits into from
Closed

Implement videojs-abloop in the video player #656

wants to merge 2 commits into from

Conversation

mateusKoppe
Copy link


Implements a loop button in the Player

Pull Request Type

  • Bugfix
  • Feature Implementation

Related issue
#549

Description
It adds a loop button by installing the videojs-abloop plugin in the player.

Screenshots
image

Testing (for code that is not small enough to be easily understandable)
Click in the numbers to configure the loop and click in loop on.

Desktop (please complete the following information):

  • OS: Linux
  • OS Version: Manjaro Gnome 20
  • FreeTube version: 0.9

Additional context
I'm not really sure if the videojs-abloop is the best plugin for this, probably most part of the users just want a simple loop button and abloop instead adds two more button, one to set the start of the loop and other to set the end of that, I read the documentation and couldn't find anything to hide just those two button, I can take another read if you guys prefer, just wanted to make sure that this is or is not the way that this is supposed to work before do any more work.

I had to add this line to make the plugin work

abLoopPlugin(window, videojs)

I saw that in the others plugins there is no declaration like this one, if there is a better way to do it just tell me =]

@PrestonN
Copy link
Member

Judging just by your screenshot and not looking at your code, I definitely agree with you. While I appreciated the added functionality of being able to define how a video is looped, it'd probably be preferred if it was one loop button that toggled it on and off like you mentioned. If there's another plugin out there that accomplishes this, or if videojs-abloop has a toggle for this, then that direction is probably preferred.

I don't have a solution right away, but I'll likely be looking for one in the next day or so. If you end up looking ahead of me, then I'm open to suggestions.

@GilgusMaximus
Copy link
Contributor

There is also the possibility to write a mini plugin that enables this option in VideoJs: https://docs.videojs.com/docs/guides/options.html#loop

@mateusKoppe
Copy link
Author

There is also the possibility to write a mini plugin that enables this option in VideoJs

Yeah, that was my first attempt, the loop parameter works totally fine, I was trying to just add a loop button in the control bar but I could'nt find a way to add and control that, I'll take a better look at that later, to see a way to implement with or without abloop, if you have any material/article/documentation that could be helpful you can send me, I would appreciate

@mateusKoppe
Copy link
Author

Just for you information:
I'm discussing about the possibility of using ab-loop with just the loop button.

You can follow the discussion here: phhu/videojs-abloop#2
I hope to get an answer from the maintainers of the project.

@PrestonN
Copy link
Member

Apologies for not responding to this in so long.

Thanks to a PR for different functionality, we had a base for creating simple button add-ons within the code now. I was able to use that to create a new loop button that behaves the way we'd expect it to. You can look up the code for it in the latest commit.

I appreciate you taking the time to work on this and reaching out to the original maintainer. It appears we have a solution that works now so I'll go ahead and close this.

@PrestonN PrestonN closed this Jan 13, 2021
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