search

muxinc / media-chrome

Custom elements (web components) for making audio and video player controls that look great in your website or app.
https://media-chrome.org
MIT License
1.21k stars 62 forks source link

fix: default subs async complexities #789

Closed cjpillsbury closed 8 months ago

cjpillsbury commented 8 months ago

While defaultsubtitles and user preference subtitles worked for simple cases, there were various async considerations that were causing issues, including e.g. autoplay. This PR updates the logic to be potentially less efficient but more reflective of dev + user preferences and avoids some odd state mismatch possibilities that could result from the prior implementation.

Example: Consider cases where the developer has enabled defaultsubtitles. Let's say my navigator languages includes English, and my local storage preferred language is Japanese. If the first track asynchronously added is Swedish, none of my subtitle language prefs will match, so I'll pick the Swedish track to show. Then, an English track is added asynchronously. I'll end up here again, and, although the "most preferred" language is Japanese, I'll update the showing subtitle track to the English one, since it's more reflective of the user's preferences/environment than Swedish. Finally, a Japanese track is added, so I'll end up selecting that one.

NOTE: This code could be further optimized to still bail if the current selected showing track is the same as the one that will be selected, but I'd rather hold off unless/until that becomes a real concern (unless folks feel strongly).

To smoke test, use https://media-chrome-9gxm13m21-mux.vercel.app/examples/vanilla/media-elements/mux-video.html

vercel[bot] commented 8 months ago

@cjpillsbury is attempting to deploy a commit to the Mux Team on Vercel.

A member of the Team first needs to authorize it.

codecov[bot] commented 8 months ago

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (c9a6980) 78.63% compared to head (96ae025) 78.63%.

Files Patch % Lines
src/js/controller.js 70.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #789 +/- ## ========================================== - Coverage 78.63% 78.63% -0.01% ========================================== Files 58 58 Lines 10781 10782 +1 ========================================== Hits 8478 8478 - Misses 2303 2304 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

vercel[bot] commented 8 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
media-chrome ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 18, 2024 8:39pm
media-chrome-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 18, 2024 8:39pm