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

Use new Vimeo player.js API #142

Closed
wants to merge 1 commit into from
Closed

Conversation

nikolas
Copy link
Contributor

@nikolas nikolas commented Jan 17, 2017

These changes turn out to solve my big problem I'm having with issue #140.

Also addresses issue #90.

I'm sure there are things that are missing from here. I realize you have your own vimeo branch going - I'm putting this up here for reference, to share thoughts, etc., and because it solves my problem.

@nikolas
Copy link
Contributor Author

nikolas commented Jan 17, 2017

Hmmm.. tests are passing for me locally (they're run in Firefox and Chromium). Looking into why travis is failing.

nikolas added a commit to ccnmtl/juxtapose that referenced this pull request Jan 17, 2017
Updating from Froogaloop to vimeo's new [player.js API](https://github.com/vimeo/player.js#vimeo-player-api----)
solved this issue.

The changes are here:
ccnmtl/react-player@9f1eaff

Pull request to react-player is here:
cookpete/react-player#142

In order to actually use vimeo in the sequence tool, you need to remove
'vimeo' from the filters in collectionwidget.js in Mediathread.
nikolas added a commit to ccnmtl/juxtapose that referenced this pull request Jan 17, 2017
Updating from Froogaloop to vimeo's new [player.js API](https://github.com/vimeo/player.js#vimeo-player-api----)
solved this issue.

The changes are here:
ccnmtl/react-player@9f1eaff

Pull request to react-player is here:
cookpete/react-player#142

In order to actually use vimeo in the sequence tool, you need to remove
'vimeo' from the filters in collectionwidget.js in Mediathread.
@cookpete
Copy link
Owner

Nice work @nikolas. My only issue is that I don't like the idea of directly importing the vimeo package into the ReactPlayer library, which needlessly increases the package size even if the vimeo player is not used. This is why I load the vimeo SDK in using load-script in my branch (same with the youtube SDK).

Is there anything different you have done compared to my attempt that I should keep in mind? What I'll probably end up doing is combining the wisdom of both before merging it in.

@nikolas
Copy link
Contributor Author

nikolas commented Jan 19, 2017

Ah okay - good point with importing the vimeo package. I was wondering why you didn't do that with your branch and have the loadSDK stuff, but now I get it.

@cookpete
Copy link
Owner

Yeah I wanted to avoid a massive ReactPlayer bundle with every single SDK embedded into it, considering the majority of users will only use it for one or two types of URL (possibly just file paths, which uses no SDK at all).

@cookpete cookpete closed this in f262c1e Apr 27, 2017
nikolas added a commit to ccnmtl/juxtapose that referenced this pull request May 9, 2017
react-player now supports the new Vimeo API, so we no longer need to use
our custom branch of this library.

cookpete/react-player@f262c1e

cookpete/react-player#142
david-hub024 pushed a commit to david-hub024/React_VideoPlayer that referenced this pull request Dec 23, 2018
david-hub024 pushed a commit to david-hub024/React_VideoPlayer that referenced this pull request May 23, 2020
albanqoku added a commit to albanqoku/react-player that referenced this pull request Feb 24, 2021
Webmaster1116 added a commit to Webmaster1116/video-player that referenced this pull request May 20, 2021
webmiraclepro added a commit to webmiraclepro/video-player that referenced this pull request Sep 9, 2022
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.

None yet

2 participants