馃殌鈾伙笍Delegate autoplay using signal #15952
Conversation
| @@ -35,6 +35,7 @@ import {LoadingSpinner} from './loading-spinner'; | |||
| import {MediaPool} from './media-pool'; | |||
| import {PageScalingService} from './page-scaling'; | |||
| import {Services} from '../../../src/services'; | |||
| import {VideoServiceSync} from '../../../src/service/video-service-sync-impl'; | |||
There was a problem hiding this comment.
This is okay if it somehow makes your life easier, but generally speaking only P0 issues need to be resolved in 0.1. Any new features and fixes can happen in 1.0.
There was a problem hiding this comment.
I can revert 0.1 but that means supporting two delegation APIs at once.
If stability of 0.1 is a concern I don't mind doing the change (noting that 0.1 will still be bloated by the video service).
| if (this.unlistener_) { | ||
| this.unlistener_(); | ||
| } | ||
| this.unlistener_ = | ||
| playbackObservable.add(isPlaying => this.trigger_(isPlaying)); | ||
| this.video.pause(); |
There was a problem hiding this comment.
Is this right? Isn't it possible that amp-story-page has already called play() and this will pause it?
There was a problem hiding this comment.
Yup! This will not be called if the signal was already received.
(Note that video/autoplay.js is under experimental use by flag video-service).
amp-storyloads beforeamp-video, soamp-storyneeded to install theVideoManagerservice.VideoManagerservice no longer has to be bundled in theamp-storybinary, its size reduces from340134to303611bytes before gzip.