feat(youtube-player): Add width and height as inputs #16734
Conversation
googlebot
added
the
cla: yes
YourDeveloperFriend
force-pushed
the
inputs
branch
from
74a3bdb to
995d47e
Compare
| heightObs: Observable<number> | ||
| ) { | ||
| return combineLatest(playerObs, widthObs, heightObs) | ||
| .subscribe(([player, width, height]) => player && player.setSize(width, height)); |
There was a problem hiding this comment.
Any reason not to just call setSize in ngOnChanges without the streams?
There was a problem hiding this comment.
Width and height are needed for the constructor of the player as well, and I think a stream fits better when managing the player initialization. I could use ngOnChanges for the setSize call, but that won't eliminate the streams entirely, so I think it'll just add complexity if we have two different means of monitoring changes on the same value.
YourDeveloperFriend
force-pushed
the
inputs
branch
from
995d47e to
33a6e16
Compare
|
This PR is ready for review. |
|
Looks like there'a a build error on CI |
YourDeveloperFriend
force-pushed
the
inputs
branch
from
33a6e16 to
22d6fcc
Compare
|
PR is ready for review (I fixed the build errors). |
jelbourn
added
pr: lgtm
action: merge
|
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. |
3 participants
No description provided.