Make DebugReportActions use new SelectionList#74112
Conversation
| label: translate('common.search'), | ||
| onChangeText: setSearchValue, | ||
| headerMessage: getHeaderMessageForNonUserList(searchedReportActions.length > 0, debouncedSearchValue), | ||
| }} |
There was a problem hiding this comment.
❌ PERF-4 (docs)
Creating a new arrow function on every render causes unnecessary re-renders of the SelectionList component, as the function reference changes each time.
Memoize this callback using useCallback:
const handleSelectRow = useCallback(
(item) => Navigation.navigate(ROUTES.DEBUG_REPORT_ACTION.getRoute(reportID, item.reportActionID)),
[reportID]
);
// Then use:
onSelectRow={handleSelectRow}There was a problem hiding this comment.
I don't think we should use useCallback in every possible place
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
JmillsExpensify
left a comment
There was a problem hiding this comment.
No review required from a product perspective. Unsubscribing.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-11-11.at.18.20.10.movAndroid: mWeb ChromeScreen.Recording.2025-11-11.at.18.17.05.moviOS: HybridAppScreen.Recording.2025-11-11.at.18.14.41.moviOS: mWeb SafariScreen.Recording.2025-11-11.at.18.15.35.movMacOS: Chrome / SafariScreen.Recording.2025-11-11.at.18.12.52.movMacOS: DesktopScreen.Recording.2025-11-11.at.18.16.15.mov |
| if (text === '') { | ||
| setFocusedIndex(-1); | ||
| } else { | ||
| setFocusedIndex(0); | ||
| } |
There was a problem hiding this comment.
@zfurtak Why do we need to add this logic? It causes the behavior in the report action debug page to differ from what's seen in staging.
This PR:
Screen.Recording.2025-11-11.at.17.58.36.mov
Staging:
Screen.Recording.2025-11-11.at.17.58.49.mov
There was a problem hiding this comment.
You can see what it looks like on main below, however I got the impression that the behaviour of focused index which stays on the first one looks like bug (after deleting everything or No results)
What do you think? If we want to stick with this behaviour I'll revert it
Screen.Recording.2025-11-12.at.11.37.47.mov
There was a problem hiding this comment.
@zfurtak I think it makes sense to revert this for now and follow up with the design team about your suggestion in a separate ticket. Since TextInput is a widely used component across the app, any changes to it should go through proper design review first.
There was a problem hiding this comment.
Btw I found a bug, input didn't focus automatically after entering the page, just pushed a fix
@zfurtak In native, the input didn't auto focus after entering the page: Screen.Recording.2025-11-13.at.11.21.15.mov |
|
@truph01 should be all good now :) |
|
@truph01 could you re-check the PR? |
|
I am checking it now |
|
hi @grgia could you review and then merge this one? It has changes needed for all next PRs 😊 |
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.2.62-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.62-5 🚀
|
|
@grgia could you handle payment for me since the PR is deployed to production? |
Explanation of Change
Fixed Issues
$ #72970
PROPOSAL:
Tests
Settings->TroubleshootInboxActionstabOffline tests
QA Steps
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-20251105-170746.mp4
Android: mWeb Chrome
iOS: Native
ios.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
web.mov
MacOS: Desktop