🏗✨ Basic implementation of VideoInterface for <amp-vimeo> #15326
Conversation
alanorozco
changed the title
| iframe.src = 'https://player.vimeo.com/video/' + encodeURIComponent( | ||
| videoid); | ||
| const iframe = | ||
| htmlFor(element)`<iframe frameborder=0 alllowfullscreen></iframe>`; |
There was a problem hiding this comment.
allowfullscreen is misspelled.
| const {element} = this; | ||
|
|
||
| const eventToDispatch = VIMEO_EVENTS[data['event']]; | ||
| if (eventToDispatch) { |
There was a problem hiding this comment.
Is it worth rechecking that the event is in VIMEO_EVENTS or is it guaranteed they wont sent events that addEventListener message hasn't been sent for?
There was a problem hiding this comment.
The frame won't send events that you don't listen for.
| /** @override */ | ||
| preimplementsMediaSessionAPI() { | ||
| // TODO(alanorozco): dis tru? | ||
| return false; |
There was a problem hiding this comment.
it is. just checked, they hook into media session API themselves.
There was a problem hiding this comment.
Fixed. Forgot I left that comment 😛
|
|
||
| /** @override */ | ||
| mute() { | ||
| this.sendCommand_(this.setVolumeMethod_(), '0'); |
There was a problem hiding this comment.
huh, can you test autoplay on iOS? I hope behind the scene they set muted attribute if volume = 0, otherwise I expect autoplay to be broken on iOS since they don't allow changing volume and only autoplay if muted attribute is there.
There was a problem hiding this comment.
It didn't, but now it works! We can just muted=1 on the query params.
|
|
||
| let src = `https://player.vimeo.com/video/${encodeURIComponent(vidId)}`; | ||
|
|
||
| if (this.hasAutoplay_()) { |
There was a problem hiding this comment.
as discussed, this would to use the isFullscreenSupported && hasFullscreen which unfortunately is async putting all of this a frame out but that should be fine.
There was a problem hiding this comment.
I'm guessing you meant isAutoplaySupported? :) if so, then fixed.
There was a problem hiding this comment.
Lol, context switch overflow
|
@aghassemi PTAL |
Autoplay support tested manually with mute/unmute + icon.