-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(youtube-player): support passing in the playerVars parameter #19746
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
Conversation
We tried doing this before in angular#17672, but we never managed to wrap it up. These changes add support for passing in the `playerVars` parameter to the YouTube API which has some settings like autoplay and hiding the video controls. Fixes angular#19267.
*/ | ||
@Input() | ||
get playerVars(): YT.PlayerVars | undefined { return this._playerVars.value; } | ||
set playerVars(playerVars: YT.PlayerVars | undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jelbourn In #17672 (comment) I was advocating against passing in the playerVars
object to the input. I changed my mind, because the object has many properties that we'd have to proxy and which will increase the API surface a lot. I'm open to changing it if you think otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair. It's a little less Angular-y, but adding all those APIs will be bad for payload size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the internals of the player are due for a rework anyway since this reliance on an observable + getter + setter for every single input isn't scalable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
*/ | ||
@Input() | ||
get playerVars(): YT.PlayerVars | undefined { return this._playerVars.value; } | ||
set playerVars(playerVars: YT.PlayerVars | undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair. It's a little less Angular-y, but adding all those APIs will be bad for payload size.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
We tried doing this before in #17672, but we never managed to wrap it up. These changes add support for passing in the
playerVars
parameter to the YouTube API which has some settings like autoplay and hiding the video controls.Note: I'm setting it to P2, because it's something that we should've supported from the beginning and it comes up in issues once in a while.
Fixes #19267.