fix: can't play video on a narrow viewport size on IOU report#81962
fix: can't play video on a narrow viewport size on IOU report#81962srikarparsi merged 7 commits intoExpensify:mainfrom
Conversation
|
@srikarparsi Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
An attachment in the comments of an expense thread or iou/expense report should behave consistently, I agree. 👍
|
Hi @eh2077 can you please review this PR when you have a chance |
|
Sure, will look into it today |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-03-10.at.9.21.31.PM.movAndroid: mWeb ChromeScreen.Recording.2026-03-10.at.9.20.32.PM.moviOS: HybridAppScreen.Recording.2026-03-10.at.9.17.37.PM.moviOS: mWeb SafariScreen.Recording.2026-03-10.at.9.20.06.PM.movMacOS: Chrome / SafariScreen.Recording.2026-03-10.at.9.14.44.PM.mov |
|
Bug: can't play video after switch from a narrow screen Screen.Recording.2026-02-11.at.5.58.38.PM.mov |
|
@daledah friendly bump! |
|
I'll investigate and give an update soon |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
web-resize.mp4@eh2077 i fixed, please check again |
|
Cool! Can you also add unit tests please? |
We don't create any a new function. So I think we won't create unit tests here. |
Hmm, it might make sense in the past, but I think we should add unit test for these changes, see the below bullet point from PR author checklist.
See also a note from internal C+ channel
I agree some use cases might be not easy to add unit test, so you can share your thoughts in that case and I'll help look into it as well, thanks! |
In this PR, we’re not changing anything in Utils, Actions, or hooks. I also don’t see any existing tests that are related to this video flow. |
|
@daledah It still doesn't work well after change to small window and then back to large window Screen.Recording.2026-02-19.at.10.33.48.PM.mov |
web-resize.mp4it works well after i merge main in my side. Plesae check again |
|
I'll retest tmr, thanks! |
|
I still can reproduce the issue. @daledah let me know if you can reproduce it with following steps, thanks! Screen.Recording.2026-02-25.at.10.59.10.PM.movPrecondition: add a video to expense details of a report
|
|
@eh2077 It seems the reproduce steps aren't consistent. Could you add Melvin here to help us? |
|
@daledah I'm not sure if Melvin could help in this case. Can you share a video following the steps? hmm, I can reproduce constantly. @MelvinBot please help troubleshoot the issue as mentioned #81962 (comment) and #81962 (comment) |
🔍 Troubleshooting Analysis: Video Controls Break After Viewport Resize (narrow → wide)Reproduction Steps ConfirmedFollowing
Root CauseThe shared video element system was designed for two separate Here is the precise failure chain: Step 1 — Mode flip during resize: shouldUseSharedVideoElement={shouldUseSharedVideoElement && !shouldUseNarrowLayout}When the viewport crosses the 800px breakpoint ( Step 2 — Non-shared → shared transition breaks ref registration: if (shouldUseSharedVideoElement || url !== currentlyPlayingURL || reportID !== currentRouteReportID) {
return; // ← Returns early! video.updateRefs() is NEVER called
}This guard is intentional for the two-instance architecture (shared instances should receive, not donate). But after a resize, the single instance is now in shared mode with no non-shared donor to register the refs. Step 3 — Controls silently fail:
After the resize transition, Step 4 — VideoView unmount during branch switch: What the PR's Changes Do (and Don't Do)The PR correctly addresses half of the problem:
Recommended Fix DirectionThe fundamental issue is that a single Option A — Keep the VideoView alive during transitions: Option B — Re-register refs when entering shared mode: Option C — Prevent mode toggling entirely: Evidence Summary
|
|
thanks @eh2077, i'll check the Melvin solution today |
|
@eh2077 i updated. This is the result: web-resize.mp4 |
|
🚧 @srikarparsi has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/srikarparsi in version: 9.3.36-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.3.36-10 🚀
|

Explanation of Change
Fixed Issues
$ #80421
PROPOSAL: #80421 (comment)
Tests
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-02-10.at.17.49.40.mov