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

Add ability to pass sources prop to support multiple video qualities (fixes #67) #116

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

zbuttram
Copy link

@zbuttram zbuttram commented Dec 6, 2018

Allows the official way of adding multiple HD/SD qualities as seen here: https://support.jwplayer.com/articles/how-to-add-an-hd-quality-toggle

Also a resolution for #67

I have confirmed it works with a test build.

@zbuttram zbuttram changed the title Add ability to pass player sources to support multiple video qualities (fixes #67) Add ability to pass sources prop to support multiple video qualities (fixes #67) Dec 6, 2018
Copy link
Contributor

@ellell ellell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@danmakenoise want to take a peek at this?

@danmakenoise
Copy link
Contributor

@zbuttram I'm good with it! Would you be willing to add a test for it before we merge?

@zbuttram
Copy link
Author

zbuttram commented Dec 7, 2018

@danmakenoise Yeah, no problem. Would something in the get-player-opts tests be good? Or is there somewhere we should test that its actually applying to the player?

Edit: Looks like I should add a check in shouldComponentUpdate for the sources prop as well since file and playlist are checked currently so I'll add a commit for that as well when I do the test(s).

@danmakenoise
Copy link
Contributor

@zbuttram Sounds good to me, thank you!

@harsharora04
Copy link

Guys, will you be merging this anytime soon? Would really like this feature.

@zbuttram
Copy link
Author

zbuttram commented May 7, 2019

@harsharora04 Sorry, I imagine that's kind of on me for putting the requested changes off for so long. Just finished it up so it should be ready to merge now.

@harsharora04
Copy link

Thanks @zbuttram.

@ade1256
Copy link

ade1256 commented May 3, 2020

Thanks, this is what i need. it worked well to me

@Cilveti
Copy link

Cilveti commented Jun 15, 2021

Sources can be set now, but are a bit buggy, I'm guessing it's because this wasn't merged?
Right now if you have a playlist with multiple sources, the video resets every time you change the browser tab, which is really not ideal

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.

6 participants