-
Notifications
You must be signed in to change notification settings - Fork 19
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: ensure transcript button appears in video player #1135
Conversation
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.
Looks good to me!
I have tested the changes on both the video detail page and course previews and everything is working fine. I think we're ready for a merge.
Thanks for contributing to the fix! 👍
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1135 +/- ##
==========================================
- Coverage 87.12% 87.08% -0.05%
==========================================
Files 391 392 +1
Lines 8123 8152 +29
Branches 2005 2011 +6
==========================================
+ Hits 7077 7099 +22
- Misses 997 1003 +6
- Partials 49 50 +1 ☔ View full report in Codecov by Sentry. |
06a1aa8
to
c490c0b
Compare
On stage/production (and seemingly locally on some machines), the "AD" transcripts button, added via
vjstranscribe
plugin forvideo.js
, does not consistently appear, preventing users from using the transcript feature as expected.This PR ensures the "AD" transcripts button always appears, using the out-of-the-box plugin by fetching the asynchronous transcripts data prior to initializing the player with
videojs(...)
:Video demo of fix
video-transcripts-locale-change.mov
For all changes
Only if submitting a visual change