Conversation
src/service/video-manager-impl.js
Outdated
| element.signals().whenSignal(VideoEvents.REGISTERED) | ||
| .then(() => this.onRegister_()); | ||
|
|
||
| // |
src/service/video-manager-impl.js
Outdated
| * Trigger event for first manual play. | ||
| * @private @const {!function()} | ||
| */ | ||
| this.triggerFirstPlayOrNoop_ = once(() => { |
There was a problem hiding this comment.
let's make the naming and location of the method match existing timeUpdateActionEvent_ method.
There was a problem hiding this comment.
The event is fired per-video, so it should be at the entry level. Unless you mean firstPlay should be triggered once per document?
src/service/video-manager-impl.js
Outdated
| * Trigger event for first manual play. | ||
| * @private @const {!function()} | ||
| */ | ||
| this.triggerFirstPlayOrNoop_ = once(() => { |
There was a problem hiding this comment.
🙇 for adding once to our library, so useful.
There was a problem hiding this comment.
It's been handy once or twice before 🤓
src/service/video-manager-impl.js
Outdated
| */ | ||
| this.triggerFirstPlayOrNoop_ = once(() => { | ||
| const trust = ActionTrust.LOW; | ||
| const event = createCustomEvent(this.ampdoc_.win, 'firstPlay', |
There was a problem hiding this comment.
| const trust = ActionTrust.LOW; | ||
| const event = createCustomEvent(this.ampdoc_.win, 'firstPlay', | ||
| /* detail */ {}); | ||
| const actions = Services.actionServiceForDoc(this.ampdoc_); |
There was a problem hiding this comment.
this.actions_ should be available.
There was a problem hiding this comment.
It's available at the manager level, see comment above.
src/service/video-manager-impl.js
Outdated
| */ | ||
| this.triggerFirstPlayOrNoop_ = once(() => { | ||
| const trust = ActionTrust.LOW; | ||
| const event = createCustomEvent(this.ampdoc_.win, 'firstPlay', |
There was a problem hiding this comment.
const for firstPlay (mostly yo make it consistent with timeUpdateActionEvent_)
No description provided.