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

Vimeo private videos: check for hash in src and add as a param #2322

Merged
merged 2 commits into from
Sep 29, 2021

Conversation

Frosch
Copy link
Contributor

@Frosch Frosch commented Sep 17, 2021

Link to related issue (if applicable)

#2321

Summary of proposed changes

As Vimeo now expects a given hash as a url-parameter ?h={hash} for newly uploaded private videos, I added some code for basic checking whether the given src-url contains the private hash, extracting it and adding it as a parameter to the iframe, if needed.
'Basic' as in, you might want to make this more sophisticated: currently I just strip the src-url, but adding a separate option for vimeo-src or something might be preferred. Also this might be put into some checks whether the user has a business-/pro-account, as otherwise he can't have private videos anyway.

I can change this PR accordingly, if you need me to :)
This currently works as a workaround for us, as you have to do nothing different when using plyr (don't have to split the embed-url by yourself etc.)

@sforward
Copy link

Hi @Frosch, I ran into the same problem with the private Vimeo videos, and tested your PR in our project. Some things I noticed: your code breaks the player when no source is set, for example: <div id="player" data-plyr-provider="vimeo" data-plyr-embed-id="12345"></div>. The used regex doesn't cover the Vimeo embed url like https://player.vimeo.com/video/{id}?h={hash}. In addition, we probably need to add an extra attribute to handle the hash, like data-plyr-embed-hash.

} else {
hash = parseHash(source)
}
const hashParam = (!!hash) ? {h: hash} : {}
Copy link
Owner

Choose a reason for hiding this comment

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

I think this can just be const hashParam = hash ? { h: hash } : {}?

Copy link
Owner

@sampotts sampotts left a comment

Choose a reason for hiding this comment

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

Just a minor tweak. I can amend this though. Cheers! 👍🏼

@sampotts sampotts merged commit e4a18a5 into sampotts:develop Sep 29, 2021
sampotts pushed a commit that referenced this pull request Sep 29, 2021
* check for hash in src and add as a param

* Enable different methods of passing hash
@vkwc
Copy link

vkwc commented Oct 6, 2021

Is there timeline when it will go to release?

@sampotts
Copy link
Owner

sampotts commented Oct 6, 2021 via email

@ctsstc ctsstc mentioned this pull request Oct 15, 2021
3 tasks
@vkwc
Copy link

vkwc commented Oct 26, 2021

I’ll try and jump on tonight and get a release out. There’s just a SASS change that needs some testing. Sorry for the delay, Sam

On 6 Oct 2021, at 4:13 pm, vkwc @.***> wrote:  Is there timeline when it will go to release? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

Is it available now on master ?

@erikt9
Copy link

erikt9 commented Oct 26, 2021

Is it available now on master ?

It's in 3.6.9

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.

5 participants