-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Video] use dynamic import for hls.js #5429
Conversation
|
src/view/com/util/post-embeds/VideoEmbedInner/VideoEmbedInnerWeb.tsx
Outdated
Show resolved
Hide resolved
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.
- there's a race that needs to be fixed
- this is also a bit stale compared to main, and more refactoring would be needed on main before the same approach can be made to work here. i've tried doing it here so you can try to use this but i think we also need to adjust the
handleFragChange
there (and onmain
), i'll make another ticket for that
src/view/com/util/post-embeds/VideoEmbedInner/VideoEmbedInnerWeb.tsx
Outdated
Show resolved
Hide resolved
@@ -413,7 +416,7 @@ export function Controls({ | |||
<View | |||
pointerEvents="none" | |||
style={[a.absolute, a.inset_0, a.justify_center, a.align_center]}> | |||
{buffering && <Loader fill={t.palette.white} size="lg" />} | |||
{showSpinner && <Loader fill={t.palette.white} size="lg" />} |
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.
should the above condition use showSpinner
too instead of buffering
? i have not tested visually closely
Tested on the video feed w/ deployment, seems to be working good now. Appreciate you taking a look @gaearon |
No description provided.