-
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
[ENT-9375] Fix: "AD" button appears but doesn't work for some videos #1166
Conversation
c784403
to
d9392ac
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1166 +/- ##
==========================================
+ Coverage 88.09% 88.14% +0.04%
==========================================
Files 394 394
Lines 8360 8374 +14
Branches 2016 2018 +2
==========================================
+ Hits 7365 7381 +16
+ Misses 953 951 -2
Partials 42 42 ☔ View full report in Codecov by Sentry. |
d9392ac
to
0a84044
Compare
0a84044
to
74ee358
Compare
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.
Resolved an issue where the "AD" button appeared but didn't work for some videos due to text tracks being lost after @tanstack/react-query performed background re-fetches. When the player is re-initialized or its sources are updated due to re-fetching, it loses the text tracks that were previously added, so made changes to re-add text tracks after background re-fetches.
Seems reasonable at face value; unfortunately I wasn't able to test/verify the changes locally given there are no stage videos given the use of localhost.stage.edx.org
for local dev, though...
@adamstankiewicz I'll discuss with the team to come up with a solution for having video data on stage for testing, considering that the feature flag will be removed soon. |
Ticket
ENT-9375
Description
@tanstack/react-query
performed background re-fetches. When the player is re-initialized or its sources are updated due to re-fetching, it loses the text tracks that were previously added, so made changes to re-add text tracks after background re-fetches.getPrimaryLanguageSubtag
method fromedx-platform
to strip full language locales and fallback to English when tracks are unavailable in the site language.For all changes
Only if submitting a visual change