-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Fix null issue in url youtube url when running a playlist with no vid… #31403
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
…eoId When playing a playlist via playerVars.list = [playlistId] we dont have a videoId available yet. Plus we don't need one, Youtube will start at the first video of the playlist by default. By injecting an undefined videoId in the YT Player constructur, we end up with null value in the iframe url (`ps://www.youtube.com/embed/null?cc_load_policy=1&enablejsapi=1&controls=1&showinfo=0&rel=0&listType=playlist&list=PLxf07yK_HAvdwhW-L0X3uP5aMe6eEt50v`) which is causing an Invalid Video Id error for consumers.
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
I have just signed the CLA |
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.
The change looks good to me, but there are a few issues:
- The CLA doesn't show up as signed.
- The linter is failing.
- Some unit tests are failing.
If it's easier, I can also land these changes separately.
src/youtube-player/youtube-player.ts
Outdated
@@ -587,17 +587,21 @@ export class YouTubePlayer implements AfterViewInit, OnChanges, OnDestroy { | |||
|
|||
// Important! We need to create the Player object outside of the `NgZone`, because it kicks | |||
// off a 250ms setInterval which will continually trigger change detection if we don't. | |||
const params: any = { |
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.
This can be typed as YT.PlayerOptions
, rather than any
.
apply YT.PlayerOptions to params
fix formatting
fix unit test
@crisbeto let me know if it is alright now. |
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.
It's almost there. It looks like the formatting in the youtube-player.ts
is off.
Yeah, I am doing it inline so I can't format it. I haven't downloaded the full repo to my mac, I was a bit lazy. For the linter I ma have to do that. |
Fix formatting
@crisbeto Formatting should now be resolved. My comments went out of the boundary and some other comments also just crossed over so I corrected that. |
The changes were merged into the following branches: main, 20.0.x |
…ideoId (#31403) * Fix null issue in url youtube url when running a playlist with no videoId When playing a playlist via playerVars.list = [playlistId] we dont have a videoId available yet. Plus we don't need one, Youtube will start at the first video of the playlist by default. By injecting an undefined videoId in the YT Player constructur, we end up with null value in the iframe url (`ps://www.youtube.com/embed/null?cc_load_policy=1&enablejsapi=1&controls=1&showinfo=0&rel=0&listType=playlist&list=PLxf07yK_HAvdwhW-L0X3uP5aMe6eEt50v`) which is causing an Invalid Video Id error for consumers. * Update youtube-player.ts apply YT.PlayerOptions to params * Update youtube-player.ts fix formatting * Update youtube-player.spec.ts fix unit test * Update youtube-player.ts Fix formatting (cherry picked from commit 2c87ec3)
…eoId
When playing a playlist via playerVars.list = [playlistId] we dont have a videoId available yet. Plus we don't need one, Youtube will start at the first video of the playlist by default.
By injecting an undefined videoId in the YT Player constructur, we end up with null value in the iframe url (
ps://www.youtube.com/embed/null?cc_load_policy=1&enablejsapi=1&controls=1&showinfo=0&rel=0&listType=playlist&list=PLxf07yK_HAvdwhW-L0X3uP5aMe6eEt50v
) which is causing an Invalid Video Id error for consumers.