Expo-av to expo-video migration after revert#75270
Conversation
|
|
|
Hi @mountiny this is a draft PR for the expo-video main issue after the revert and also linked subissues. I already started with a revert of the revert, fixing the Also cc-ing @blazejkustra @war-in and @Skalakid who were part of the original PR, feel free to unsubscribe 🔙 |
|
I think I've found a correct conditional to display loading spinner and hide controls a little bit longer when video doesn't start playing yet, but also don't trigger it on pausing or ending (occurs when using plain 'idle' status). This way the issue with 'NaN' duration shouldn't be possible to occur since the whole controls panel wouldn't be visible, but I safeguarded it with swapping bullish coalescing for OR operator to also filter NaN and swap it for 0. |
…-fork into Guccio163/expo-video-after-revert
…-fork into Guccio163/expo-video-after-revert
|
Next 2 issues look like handled, during the development I think I've stumbled upon a bug originating in sharedElement logic, I'll focus on it next, hopefully with some help from @Skalakid 🙇 |
…-fork into Guccio163/expo-video-after-revert
…-fork into Guccio163/expo-video-after-revert
…ideo-after-revert
|
Asked for an update |
|
@mountiny I guess that makes you a real wizard then 🪄 Can't wait to see it merged, let's hope for no reverts this time 👀 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
Hi @Guccio163 and @mountiny. Which QA steps do we need to follow here? |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.10-0 🚀
|
|
@IuliiaHerets there are no specific test steps here as this pr changes everything related to video and audio in the app so all the regression tests you might have related to videos |
|
There is also a checklist created some time ago to test video player's behaviour; It's pasted in the 'Tests' section in PR's description, maybe it'll help 👀 |
|
@mountiny @Guccio163, accordingly, I will check off this PR from the list, as these steps are already covered by regression tests. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.10-6 🚀
|
|
@Guccio163 @mountiny @ZhenjaHorbach My IOS app crashed when I shared a video. It seems this problem was caused by this PR. Could you please check again? error.mov |
Did you update the main to the latest version, nodes, pods, and rebuild the iOS app? |
|
I can reproduce, and I have a fix. I'll post a PR asap |
| return isLoading && (!isPlaying || currentTime <= 0) && !isOffline && !hasError; | ||
| }, [currentTime, hasError, isLoading, isOffline, isPlaying]); | ||
| const shouldShowOfflineIndicator = useMemo(() => { | ||
| return isOffline && currentTime + bufferedPosition <= 0; |
There was a problem hiding this comment.
Relying on isOffline would end up showing offline indicator even for intermittent/slow network conditions. We fixed this in #82779

Explanation of Change
This PR introduces expo-video replacing expo-av Video usages. It is a second PR after this one because of the revert. It also aims to fix issues reported during the staging deployment.
Fixed Issues
$ #64846
$ #75203
$ #75206
$ #75207
$ #75212
$ #75214
$ #75215
$ #75216
$ #75234
PROPOSAL:
Tests
Video checklist introduced here below:
Checklist
/onboarding/welcome-videourl)Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-01-09.at.15.52.16.mov
Android: mWeb Chrome
mWeb_android.mov
iOS: Native
iOS.mov
iOS: mWeb Safari
mWeb-iOS.mov
MacOS: Chrome / Safari
web480.mov
MacOS: Desktop
Not included, desktop no longer supported