amp-bind: Support amp-youtube #8067
Conversation
| mutatedAttributesCallback(mutations) { | ||
| if (mutations['data-videoid'] !== undefined) { | ||
| this.videoid_ = this.getVideoId_(); | ||
| if (this.iframe_) { // `null` if element hasn't been laid out yet. |
There was a problem hiding this comment.
no need to refresh the iframe, YouTube supports dynamically loading a video which should be much faster than a full reload, the following should work (not tested though)
this.playerReadyPromise_.then(() => {
this.sendCommand_('loadVideoById', this.videoid_);
});
There was a problem hiding this comment.
Nice, thanks for pointing this out.
There was a problem hiding this comment.
There is short amount of time (actually a good 2 seconds, so not that short :) ) when this.iframe_ is not null but player is not loaded either (meaning commands will be ignored) wrapping the sendCommand_ in the this.playerReadyPromise_.then will queue it up in those cases.
There was a problem hiding this comment.
I actually moved the this.playerReadyPromise_.then to within sendCommand_ to reduce duplication. I assume that we never want to call sendCommand_ before the player is ready as you mentioned.
dreamofabear
force-pushed
the
bind-youtube
branch
from
8fd9509
to
538b9ce
Compare
| mutatedAttributesCallback(mutations) { | ||
| if (mutations['data-videoid'] !== undefined) { | ||
| this.videoid_ = this.getVideoId_(); | ||
| if (this.iframe_) { // `null` if element hasn't been laid out yet. |
There was a problem hiding this comment.
There is short amount of time (actually a good 2 seconds, so not that short :) ) when this.iframe_ is not null but player is not loaded either (meaning commands will be ignored) wrapping the sendCommand_ in the this.playerReadyPromise_.then will queue it up in those cases.
Done, though the added value isn't great since it doesn't test any logic within |
dreamofabear
force-pushed
the
bind-youtube
branch
from
bd5d8fc
to
b3f6922
Compare
dreamofabear
force-pushed
the
bind-youtube
branch
from
b3f6922
to
6599424
Compare
|
@kmh287 Gentle ping. |
dreamofabear
force-pushed
the
bind-youtube
branch
from
6599424
to
a3865e8
Compare
I don't think so since |
* support binding to amp-youtube's data-videoid * unit test * use loadVideoById instead of changing iframe src * fix lint error * add integration test
Partial for #7825.
amp-youtube'sdata-videoidattribute./to @aghassemi