Open report from switcher perf improvement#64077
Conversation
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
| navigateToAndOpenReport(item.login ? [item.login] : [], false); | ||
| } | ||
| }); | ||
| InteractionManager.runAfterInteractions(() => { |
There was a problem hiding this comment.
This delay is making things feel significantly less snappy for me on web in general.
Screen.Recording.2025-06-17.at.13.22.12.mov
There was a problem hiding this comment.
Thanks a lot for the review and for catching that!
Just to clarify, what user account were you using during testing? And if I'm not mistaken, you were running this on Chrome?
I agree - the transition feels a bit cranky on the recording - and although it doesn't happen consistently (I personally haven't been able to reproduce it), even occasional hiccups in UX could outweigh the performance gain. The profiling tools shows improvement in performance (in loading the report), but we might be trading off too much in terms of perceived feeling.
I’ll revisit this approach and try to find some other way to optimize report opening
There was a problem hiding this comment.
I was just using my high traffic account, just a gmail account.
It does seem a fair bit smoother in the ad-hoc build, so it may just be an issue on my dev setup.
Maybe some others can give it a test and chime in.
There was a problem hiding this comment.
@Ollyws I removed runAfterInteraction method, which might have been causing the issue, and also removed some logic in SearchRouterContext.
The changes are now minimal, so the performance gain won’t be as significant as initially expected, but it's still worth adding.
Let me know if the visual result is satisfactory - if not, I’ll revisit it and explore other ideas for improvement
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
@Ollyws Can you please prioritize the review here? |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp01_Android_Native.mp4Android: mWeb Chrome02_Android_Chrome.mp4iOS: HybridApp03_iOS_Native.mp4iOS: mWeb Safari04_iOS_Safari.mp4MacOS: Chrome / Safari05_MacOS_Chrome.mp4MacOS: Desktop06_MacOS_Desktop.mp4 |
Ollyws
left a comment
There was a problem hiding this comment.
Alot smoother now, thanks!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.75-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.1.76-5 🚀
|
Explanation of Change
Improve the time to display the report from Chat Switcher
Background
Navigating to a report from the Chat Switcher is slower than expected. There is a noticeable lag (~500ms) between the time the user clicks on a report and the OpenReport action is triggered.
Problem
It is cause because of the call onRouterClose() inside SearchRouter, which is executed immediately upon report click. This function closes a modal that’s mounted at the top level in AuthScreen, leading to re-renders and delays before the report is actually opened.
Solution
Deferring onRouterClose() Execution
Instead of calling onRouterClose() immediately, we delay its execution until after we start navigating to new report. This reduces contention in UI updates and avoids blocking navigation.
Refactoring SearchRouterContext
We can wrap the function in useCallback to avoid unnecesary rerenders when day are called. They are called when onRouterClose function is invoked.
Fixed Issues
$ #64174
PROPOSAL: https://callstack-hq.slack.com/archives/C05LX9D6E07/p1749814149209879
Tests
Offline tests
QA Steps
🎥 Recording from before/after improvements:
Web before:
https://github.com/user-attachments/assets/d85cfc70-3ac6-4efb-8a2e-469cbbe1eaf9
Web after:
https://github.com/user-attachments/assets/f3679743-392a-48c3-adf8-bfbd4c9194c1
Android before:
https://github.com/user-attachments/assets/27e92b51-c6e2-4438-98d7-46570266a8d1
Android after:
https://github.com/user-attachments/assets/d120cd63-2267-479b-b484-44b98cf0875b
IOS before:
https://github.com/user-attachments/assets/0a693ca6-4d91-47cc-ad4c-3feae7ef4073
IOS after:
https://github.com/user-attachments/assets/d7595a01-decc-4dbf-adc6-55b64ab65288
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))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
MacOS: Desktop