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

feat: add property to disable related videos #28

Closed

Conversation

franciscorode
Copy link
Contributor

First, congrats by the repo!
I tried to disable related videos by adding rel option in playerOptions with playerVars but for some reason, it didn't work, the only way to make it work was to add it as a GET parameter in the src prop. To not set options in several sites I think is better to add it as a prop and I have added the changes to do it

@andrewvasilchuk
Copy link
Owner

@franciscorode, Thanks for the PR. You can pass src prop as 'https://www.youtube.com/embed/$ID?rel=0'. I've just checked it out and it works.

Note: that you can't disable related videos referring to the docs.

After the change, you will not be able to disable related videos. Instead, if the rel parameter is set to 0, related videos will come from the same channel as the video that was just played.

@andrewvasilchuk
Copy link
Owner

I don't think it's a good idea to introduce a separate prop for every parameter, since it will make component really large. Moreover you can pass any of them via src prop.

autoplay and enablejsapi props were introduce since they change the way how component behaves.

@franciscorode
Copy link
Contributor Author

I understand your position, it's fine although I don't like the idea of ​​passing options in the URL. I tried to pass the option in prop playerOptions but I could not know why it did not work for me, other options work well such as events.
If you find of good way to pass options outside of the URL and without overloading the properties component let me know I'll be happy to help

@andrewvasilchuk
Copy link
Owner

@franciscorode, We can add parameters prop, which can include all the parameters. All we need to do is correctly construct the URL.

@franciscorode
Copy link
Contributor Author

Yes seem a good idea, I understand these points, tell me if you don't agree with some of them

  • The prop will be type Object with a default empty object
  • The description of the API documentation for the prop will show a link to the official parameters doc
  • The possible name and values ​​of options will base on youtube API
  • Will be the responsibility of the developer set correctly the option names and values in a similar way to playerOptions prop
  • The computed property srcAttribute will add each option to the URL
  • The enablejsapi and autoplay props should be passed only in this prop instead have own props, to don't break retrocompatibility will maintain them but we should add a warning about your deprecation with the objective of remove them in a future

@andrewvasilchuk
Copy link
Owner

@franciscorode, awesome!

@franciscorode
Copy link
Contributor Author

Good! I can work on this, give me a few days and create another pull request, close this

@andrewvasilchuk
Copy link
Owner

@franciscorode, Thank you so much. I will be glad to review your PR.

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