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: refactor fullscreen-api and state mediator to make fewer assumpt… #946

Closed cjpillsbury closed 2 months ago

cjpillsbury commented 2 months ago

…ions on env-specific divergences. Fixes bug in modern Safari iOS fullscreen.

vercel[bot] commented 2 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.

vercel[bot] commented 2 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 Jul 25, 2024 6:23pm
media-chrome-demo-nextjs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 6:23pm
media-chrome-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 6:23pm
codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 68.98396% with 58 lines in your changes missing coverage. Please review.

Please upload report for BASE (3.x@3301719). Learn more about missing BASE report.

Files Patch % Lines
src/js/utils/fullscreen-api.js 66.85% 58 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## 3.x #946 +/- ## ====================================== Coverage ? 72.35% ====================================== Files ? 79 Lines ? 16213 Branches ? 0 ====================================== Hits ? 11731 Misses ? 4482 Partials ? 0 ```

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

cjpillsbury commented 2 months ago

Did basic enter/exit smoke testing on:

Not sure if we have a way to validate the Safari Mobile (iOS) < 16.4 specific logic. Should be roughly equivalent, but maybe spend a few extra ticks on code review reasoning through that if we can't validate via tests. Also please do your own smoke testing (even if on the PR deploy apps) just for more eyes.