-
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] Add loading state to player #5149
Conversation
Your Render PR Server URL is https://social-app-pr-5149.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-crcbe0rtq21c73fjafp0. |
|
The Pull Request introduced fingerprint changes against the base commit: Fingerprint diff[{"type":"dir","filePath":"node_modules/expo-video/ios","reasons":["expoAutolinkingIos"],"hash":"83f39cea9de7cb277422ffc9ded938d167748016"},{"type":"dir","filePath":"patches","reasons":["patchPackage"],"hash":"efbf60dd34e99fd1232fe618391bf23dce5bbfa9"}] Generated by PR labeler 🤖 |
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 amazing, one nit you can ignore if you want
</ErrorBoundary> | ||
</View> | ||
) | ||
} | ||
|
||
function Inner({embed}: Props) { |
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.
Could we possibly have a name other than Inner
to disambiguate from VideoEmbedInner
?
0604783
to
062b6d0
Compare
Why
Adding loading state to the player, similar to what we do with JIFs.
Test Plan
TBD