-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(preview): remove auto-preview experience #1069
Conversation
Verified that @mxiao6 has signed the CLA. Thanks for the pull request! |
@@ -44,7 +44,6 @@ class MediaBaseViewer extends BaseViewer { | |||
this.toggleMute = this.toggleMute.bind(this); | |||
this.handleRate = this.handleRate.bind(this); | |||
this.handleAutoplay = this.handleAutoplay.bind(this); | |||
this.mediaendHandler = this.mediaendHandler.bind(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think leaving in this event is still fine, we just don't need to react to it internally in the preview sdk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see
e445b8c
to
184bade
Compare
184bade
to
b35b80d
Compare
src/lib/Preview.js
Outdated
@@ -1239,7 +1239,6 @@ class Preview extends EventEmitter { | |||
this.ui.finishProgressBar(); | |||
break; | |||
case VIEWER_EVENT.mediaEndAutoplay: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire condition can be removed, as it's now a no-op.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it looks like it's preventing the event from being forwarded by the default
condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then the test for it should be removed entirely too, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or change it to test the default condition
No description provided.