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

Wistia support #133

Merged
merged 15 commits into from
Dec 26, 2016
Merged

Wistia support #133

merged 15 commits into from
Dec 26, 2016

Conversation

edelgado
Copy link
Contributor

@edelgado edelgado commented Dec 7, 2016

Hi Pete,

Thanks for a great React-based player. This PR adds support for Wistia videos. I followed the structure from the YouTube component and I'll leave some specific notes in the diff below.

I'd love to work with you to get Wistia support into a react-player 🙂

return this.player.percentWatched()
}
getFractionLoaded () {
return null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK, the Wistia JS player does not expose this info.

it('fires onPlay', done => testPlay(TEST_WISTIA_URL, done))
it('fires onDuration', done => testDuration(TEST_WISTIA_URL, done))
it('fires onDuration with delayed load', done => testDurationDelayed(TEST_WISTIA_URL, done))
it.skip('fires onPause', done => testPause(TEST_WISTIA_URL, done))
Copy link
Contributor Author

@edelgado edelgado Dec 7, 2016

Choose a reason for hiding this comment

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

The //fast.wistia.com/assets/external/E-v1.js Wistia JS player is outputting some errors in Firefox. The player works as expected, but it makes the tests have some output. I'll reach out to the Wistia folks to see if they know what the issue may be. I'm skipping this test as it makes the whole test suite hang.

Enrique Delgado added 3 commits December 14, 2016 13:50
When the react-player component is mounted in another app, each component would load the Wistia script, but we only need it once.
@edelgado
Copy link
Contributor Author

Hi @cookpete. I just merged master and added support for playback rate as well. Let me know what you think. I'd love to get Wistia support into the player.

@cookpete cookpete merged commit bede619 into cookpete:master Dec 26, 2016
@cookpete
Copy link
Owner

Thanks for the PR @edelgado. I've tidied things up slightly but it was mainly just me being anal about code style and consistency of stuff. Published in 0.14.0. Merry christmas! 🎄

@edelgado
Copy link
Contributor Author

Thanks for the merge and cleanup @cookpete, Merry Christmas to you too :-)

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.

2 participants