refactor: IOURequestStepScan clean-up, phase 6: move multi-scan state to hook and use key for reset (v2)#87242
Merged
roryabraham merged 2 commits intoExpensify:mainfrom Apr 8, 2026
Conversation
|
@bernhardoj 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] |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors IOURequestStepScan by moving multi-scan state ownership into useReceiptScan and removing the parent-driven multi-scan props, with a reset mechanism via React key.
Changes:
- Move
isMultiScanEnabled/isStartingScanderivation intouseReceiptScan(route-name driven) and update web/native scan screens to consume hook state. - Update mobile scan hook API to also manage/clear
receiptFileswhen toggling multi-scan. - Adjust unit/UI tests and
IOURequestStartPageto align with the new state ownership and reset strategy.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/hooks/useReceiptScan.test.ts | Updates tests for new hook params/state, but includes a now-misaligned test description. |
| tests/unit/hooks/useMobileReceiptScan.test.ts | Adds assertions around clearing receiptFiles when disabling multi-scan. |
| tests/ui/IOURequestStepScanTest.tsx | Updates UI test to enable multi-scan via UI interaction; contains a route typing/cast mismatch. |
| src/pages/iou/request/step/IOURequestStepScan/types.ts | Removes multi-scan props from component params and introduces routeName; updates mobile hook param types. |
| src/pages/iou/request/step/IOURequestStepScan/index.tsx | Switches web scan to use hook-owned multi-scan state and passes routeName into the hook. |
| src/pages/iou/request/step/IOURequestStepScan/index.native.tsx | Same as web: uses hook-owned multi-scan state and passes setReceiptFiles into mobile hook. |
| src/pages/iou/request/step/IOURequestStepScan/hooks/useReceiptScan.ts | Introduces hook-local isMultiScanEnabled state and computes isStartingScan from routeName. |
| src/pages/iou/request/step/IOURequestStepScan/hooks/useMobileReceiptScan.ts | Clears receiptFiles when disabling multi-scan and makes setters required. |
| src/pages/iou/request/step/IOURequestStepScan/components/MobileWebCameraView.tsx | Makes multi-scan props required and wires setReceiptFiles into mobile scan hook. |
| src/pages/iou/request/IOURequestStartPage.tsx | Removes parent multi-scan state and uses key to force-reset IOURequestStepScan. |
Comments suppressed due to low confidence (1)
src/pages/iou/request/step/IOURequestStepScan/types.ts:48
routeNameis typed asstring, which removes type-safety for the route-name comparison logic inside the hook. Consider typing this as the Money Request route-name union (e.g.WithWritableReportOrNotFoundProps<...>['route']['name']) or passing a boolean likeisStartingScan/isEmbeddedInStartPageinstead.
backToReport: string | undefined;
/** The route name to determine if scan is starting */
routeName: string;
/** Callback to replace receipt and navigate back when editing */
updateScanAndNavigate: (file: FileObject, source: string) => void;
/** Returns a source URL for the file based on platform */
getSource: (file: FileObject) => string;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
@samranahm please address the copilot review |
bernhardoj
reviewed
Apr 8, 2026
Contributor
|
The Let's type the routeName as |
Contributor
|
Recording to make sure the multi scan button is only shown on the create flow. web.mp4 |
JmillsExpensify
previously approved these changes
Apr 8, 2026
Contributor
JmillsExpensify
left a comment
JmillsExpensify
left a comment
There was a problem hiding this comment.
Makes sense that multi-scan wouldn't be available in this view.
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.mp4Android: mWeb Chromeandroid.mweb.mp4iOS: HybridAppios.mp4iOS: mWeb Safariios.mweb.mp4MacOS: Chrome / Safariweb.mp4 |
roryabraham
approved these changes
Apr 8, 2026
Contributor
|
🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
Contributor
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Contributor
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
63 tasks
Contributor
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.3.55-0 🚀
Bundle Size Analysis (Sentry): |
Contributor
|
This PR is an internal refactoring that moves multi-scan state management ( No user-facing behavior changes — the receipt scanning and multi-scan features work identically before and after this PR. No new features, renamed UI elements, or changed flows. No help site changes are required. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Fixed Issues
$ #79929
PROPOSAL:
Tests
Offline tests
Same as test
QA Steps
same as test
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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.mp4
macOS.Chrome.mp4