Skip to content
This repository has been archived by the owner on Jan 5, 2018. It is now read-only.

Add Vimeo support #22

Open
wants to merge 11 commits into
base: 8.x-1.x
Choose a base branch
from

Conversation

steveoliver
Copy link

This adds support for Vimeo videos.

TODO/Discuss:

  • Configurable width/height? Where do these config options get set?
  • Thumbnail -- maybe we want to get the largest thumbs possible (I'm currently getting medium)

@@ -5,3 +5,5 @@ aol_sid: ''
wochit_progn: ''
youtube:
api_key: ''
vimeo:
access_token: ''
Copy link
Member

Choose a reason for hiding this comment

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

Missing newline.

Copy link
Author

Choose a reason for hiding this comment

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

newline added.

@slashrsm
Copy link
Member

Configurable width/height? Where do these config options get set?

We are missing this piece at the moment I believe. We should think how to introduce that.

Thumbnail -- maybe we want to get the largest thumbs possible (I'm currently getting medium)

Yup, that makes sense.

@steveoliver
Copy link
Author

I believe commit steveoliver@d7bc032 does the injection you're talking about? Also, not sure how defaultConfiguration() from your captured code comment is affected. What should I do with defaultConfiguration()?

@steveoliver
Copy link
Author

@slashrsm, I see that you must not have meant to cite the defaultConfiguration() method ... you selected everything up until that line. So I think we're good on the injection of the token.

However, the configuration of the token, I think we should show an alert and way for administrators to configure the API key/token(s), maybe in the UI? What's the way to do this in D8, would you say?

@slashrsm
Copy link
Member

Yup, injected token now looks OK.

defaultSettings() are respected in VideoProviderBase. Problem at the moment is that there is no way to override that via UI. You can do it when initializing video provider plugin, but that is not what we really want. The best solution would be to expose this configuration on media type configuration page. This is out of the scope of this PR of course.

As far as it goes about access token I'd create a standalone configuration page where we would be able to set this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants