Skip to content

fix(youtube-player): Add playerVars input and tweak readme. #17672

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

fix(youtube-player): Add playerVars input and tweak readme. #17672

wants to merge 1 commit into from

Conversation

YourDeveloperFriend
Copy link
Collaborator

@YourDeveloperFriend YourDeveloperFriend commented Nov 11, 2019 •
edited by devversion
Loading

Fixes #17368. Closes #19267

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 11, 2019
crisbeto
crisbeto reviewed Jan 2, 2020
View reviewed changes
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

It looks like there are some merge conflicts as well.

src/youtube-player/youtube-player.ts
@@ -153,8 +165,7 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
@Output() playbackRateChange = new EventEmitter<YT.OnPlaybackRateChangeEvent>();

/** The element that will be replaced by the iframe. */
@ViewChild('youtubeContainer')
youtubeContainer: ElementRef<HTMLElement>;
@ViewChild('youtubeContainer', {static: true}) youtubeContainer: ElementRef<HTMLElement>;
Copy link
Member

Choose a reason for hiding this comment

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

From what I can tell we don't need the query to be static. Am I missing something?

src/youtube-player/youtube-player.ts
@@ -119,6 +119,18 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
private _width = DEFAULT_PLAYER_WIDTH;
private _widthObs = new EventEmitter<number>();

/** PlayerVars for the YouTube player */
@Input()
get playerVars(): YT.PlayerVars | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

It might be better to have individual inputs for the different playerVars properties, rather than the entire object. We've been avoiding adding inputs where you pass in a configuration object, because change detection won't pick up changes to individual properties. What do you think @jelbourn?

Copy link
Member

Choose a reason for hiding this comment

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

I would also lean in that direction; it's the path we took for the google-maps component, since it makes the settings align more with Angular's framework idioms

Copy link

Choose a reason for hiding this comment

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

How about using the same api and within ngOnChanges => detecting "special" options and updating change detection settings accordingly?

Either way, I am looking forward to making use of this soon.

Also: Is there any workaround right now to customise the player?

Copy link

Choose a reason for hiding this comment

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

Also: Is there any workaround right now to customise the player?

I take the silence as a no. ;)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I've been pretty busy with everything at work, I should be able to begin work on this again next week.

This was referenced Jan 29, 2020
Closed
Closed
@crisbeto crisbeto mentioned this pull request May 6, 2020
Closed
@Eraldo
Copy link

Eraldo commented Jun 23, 2020

What is the status here?
The playerVars still don't show up in the latest version on (at least inn my setup). ?!
Some clarity would be appreciated. :)

@crisbeto
Copy link
Member

@YourDeveloperFriend if you don't have time wrap this one up, I wouldn't mind taking it over.

@YourDeveloperFriend
Copy link
Collaborator Author

YourDeveloperFriend commented Jun 23, 2020 via email

crisbeto added a commit to crisbeto/material2 that referenced this pull request Jun 24, 2020
@crisbeto
feat(youtube-player): support passing in the playerVars parameter
a841c48 
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.
@crisbeto crisbeto mentioned this pull request Jun 24, 2020
Merged
jelbourn pushed a commit that referenced this pull request Jun 26, 2020
@crisbeto
feat(youtube-player): support passing in the playerVars parameter (#1…
cc36af7 
…9746)

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.

Fixes #19267.
@crisbeto
Copy link
Member

playerVars were added in #19746 so this PR can be closed.

@crisbeto crisbeto closed this Jan 18, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jelbourn jelbourn jelbourn left review comments

@crisbeto crisbeto crisbeto left review comments

+1 more reviewer

@Eraldo Eraldo Eraldo left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

youtube-player: Support PlayerVars youtube-player docs use invalid tag syntax
5 participants
@YourDeveloperFriend @Eraldo @crisbeto @jelbourn @googlebot