Make NewReportWorkspaceSelectionPage use new SelectionList#74974
Conversation
…ate-NewReportWorkspaceSelectionPage
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…ate-NewReportWorkspaceSelectionPage
|
@dukenv0307 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] |
|
PR doesn’t need product input as a migration PR. Unassigning and unsubscribing myself. |
| const resultsFound = headerMessage !== translate('common.noResultsFound'); | ||
| const noData = dataLength === 0 && !showLoadingPlaceholder; | ||
| const shouldShowHeaderMessage = !!headerMessage && !isLoadingNewOptions && resultsFound && !noData; | ||
| const shouldShowHeaderMessage = !!headerMessage && (!isLoadingNewOptions || resultsFound || !noData); |
There was a problem hiding this comment.
Can you please explain this change?
There was a problem hiding this comment.
It was an oversight of mine, it juts wasn't visible before. The new version has the same logic as the original SelectionList
| return; | ||
| } | ||
|
|
||
| if (typeof textInputRef === 'function') { |
There was a problem hiding this comment.
can you please explain why we can remove handleTextInputRef? I think if we do that, textInputRef won't be assigned
There was a problem hiding this comment.
textInputRef is now in textInputOptions and I just moved the code to TextInput, where I merged those 2 refs and pass them to the component.
IMO the textInputRef is always passed as an object so we don't need those ifs.
const mergedRef = mergeRefs<BaseTextInputRef>(ref ~ innerTextInputRef, optionsRef ~ textInputRef);NewReportWorkspaceSelectionPage use new SelectionListNewReportWorkspaceSelectionPage use new SelectionList
NewReportWorkspaceSelectionPage use new SelectionListNewReportWorkspaceSelectionPage use new SelectionList
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-11-20.at.11.28.35.movAndroid: mWeb ChromeScreen.Recording.2025-11-20.at.11.22.14.moviOS: HybridAppScreen.Recording.2025-11-20.at.11.28.46.moviOS: mWeb SafariScreen.Recording.2025-11-20.at.11.19.47.movMacOS: Chrome / SafariScreen.Recording.2025-11-20.at.11.15.01.movMacOS: DesktopScreen.Recording.2025-11-20.at.11.25.49.mov |
|
@zfurtak lint |
|
@grgia We're good to merge |
|
✋ 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/grgia in version: 9.2.62-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.62-5 🚀
|
Explanation of Change
Fixed Issues
$ #72970
PROPOSAL:
Tests
Create reportOffline 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
Android: mWeb Chrome
iOS: Native
ios.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
web-newReport.mov
MacOS: Desktop