Play middleware: only trigger play event when our play middleware terminates #365
Conversation
…ads play middleware terminates, and not when another middleware terminates.
ldayananda
force-pushed
the
play-middleware-concrete
branch
from
c3be617
to
f9762bf
Compare
ldayananda
changed the title
src/plugin.js
Outdated
| @@ -193,6 +193,8 @@ const contribAdsPlugin = function(options) { | |||
|
|
|||
| // Should we block calls to play on the content player? | |||
| _shouldBlockPlay: false, | |||
| // Was play blocked by the plugin? | |||
There was a problem hiding this comment.
Suggest specifying that this is blocked by middleware
There was a problem hiding this comment.
So that someone reading it knows what feature it's related to
| assert.strictEqual(playSpy.callCount, 0, | ||
| 'play event should not be triggered'); | ||
| done(); | ||
| }, 200); |
There was a problem hiding this comment.
Could this timeout replaced by an event handler?
There was a problem hiding this comment.
I don't believe so, as if the play is blocked by another middleware we neither advance in the state machine nor do we start playback. That said, if you have another idea I'm all ears 😄
There was a problem hiding this comment.
Would it work if it was 0 or 1 ms instead of 200? 200 makes it seem like it's waiting for something
There was a problem hiding this comment.
Looks like 1ms was actually enough!
There was a problem hiding this comment.
Great, that should make it a little less mysterious
|
@incompl updated based on comments |
|
LGTM |
|
QA: Pass! |
This ensures that the code in playMiddleware.play does not run if another middleware terminates.