-
Notifications
You must be signed in to change notification settings - Fork 3k
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
render 5 initial items in search screen on mobile #55027
render 5 initial items in search screen on mobile #55027
Conversation
Reviewer Checklist
Screenshots/VideosAndroid: Nativeandroid.movAndroid: mWeb Chromemobile-chrome.moviOS: Nativeios.moviOS: mWeb Safarimobile-safari.movMacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.mov |
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.
LGTM!
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.
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/marcochavezf in version: 9.0.84-0 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.84-7 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.84-7 🚀
|
Explanation of Change
It was reported that the search screen was taking a long time to render on mobile.
After the investigation, it was found that the search screen by default initially renders the same amount of items as in the web/desktop version.
This approach is not optimal for mobile devices, because the visible part of the screen can fit around 5 items only, therefore rendering more puts extra pressure on the performance.
This PR changes this web/desktop based logic and renders only 5 items initially by passing a
initialNumToRender
prop to from theSearch
component toBaseSelectionList
.Performance gains are significant during the mount phase of
SearchPageBottomTab
screen:iOS: 160ms -> 85ms
Android: 350ms -> 120ms
Fixed Issues
$ #53818
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Screen.Recording.2025-01-09.at.21.08.36.mp4
Android: mWeb Chrome
iOS: Native
Screen.Recording.2025-01-09.at.21.24.33.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop