[No QA] refactor: IOURequestStepScan clean-up, phase 3: Consolidate isMobile() and add useDragAndDropSupport#83380
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors IOURequestStepScan to centralize mobile/desktop behavior checks and introduce a platform-specific useDragAndDropSupport helper for drag-and-drop availability.
Changes:
- Added
useDragAndDropSupportwith web + native implementations. - Replaced repeated
isMobile()calls with a cached boolean inIOURequestStepScan. - Updated layout logic to use the new drag-and-drop capability flag in places where desktop-only styling is applied.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/pages/iou/request/step/IOURequestStepScan/useDragAndDropSupport/index.ts | Adds web implementation for drag-and-drop support detection. |
| src/pages/iou/request/step/IOURequestStepScan/useDragAndDropSupport/index.native.ts | Adds native implementation (always false) for drag-and-drop support detection. |
| src/pages/iou/request/step/IOURequestStepScan/index.tsx | Uses useDragAndDropSupport() and consolidates isMobile() calls to drive UI/layout decisions. |
Comments suppressed due to low confidence (4)
src/pages/iou/request/step/IOURequestStepScan/index.tsx:600
styles.chooseFilesView(!canUseDragAndDrop)ties the choose-files padding variant to drag-and-drop capability rather than layout width. SincecanUseDragAndDropis a feature flag (desktop web) and not a responsive breakpoint, this changes behavior from the priorisSmallScreenWidth-based styling and can cause incorrect padding calculations. Consider restoring the responsive boolean (e.g., viauseResponsiveLayout()/shouldUseNarrowLayout) for thechooseFilesView(...)argument, and keepcanUseDragAndDroponly for gating drag-and-drop-specific UI.
const chooseFilesPaddingVertical = Number(styles.chooseFilesView(!canUseDragAndDrop).paddingVertical);
src/pages/iou/request/step/IOURequestStepScan/index.tsx:662
- This condition/argument combination is self-contradictory: when
canUseDragAndDropis true,!canUseDragAndDropis always false, so this will always applystyles.chooseFilesView(false)and never the other variant. If the intent is to select a narrow vs wide layout, pass the responsive boolean (as before) intochooseFilesView(...)instead of!canUseDragAndDrop.
style={[styles.flex1, canUseDragAndDrop && styles.chooseFilesView(!canUseDragAndDrop)]}
src/pages/iou/request/step/IOURequestStepScan/index.tsx:65
isMobileWebis derived fromisMobile()but the comment states it's intended to cover both mobile web and native apps. Either rename the variable to reflect the broader meaning (ifisMobile()is cross-platform), or update the comment/logic so the name and behavior match (e.g., keepisMobileWebonly for web and use a separate native/mobile flag where needed).
const isMobileWeb = isMobile();
src/pages/iou/request/step/IOURequestStepScan/index.tsx:601
isMobileWebis derived fromisMobile()but the comment states it's intended to cover both mobile web and native apps. Either rename the variable to reflect the broader meaning (ifisMobile()is cross-platform), or update the comment/logic so the name and behavior match (e.g., keepisMobileWebonly for web and use a separate native/mobile flag where needed).
// We use isMobileWeb here to explicitly hide the alternative methods component on both mobile web and native apps
const chooseFilesPaddingVertical = Number(styles.chooseFilesView(!canUseDragAndDrop).paddingVertical);
const shouldHideAlternativeMethods = isMobileWeb || alternativeMethodsHeight + desktopUploadViewHeight + chooseFilesPaddingVertical * 2 > containerHeight;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@trjExpensify no product considerations here |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| // eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth | ||
| const {isSmallScreenWidth} = useResponsiveLayout(); | ||
| const isMobileWeb = isMobile(); | ||
| const canUseDragAndDrop = useDragAndDropSupport(); |
There was a problem hiding this comment.
Hmm, since this hook doesn't use any React APIs or trigger re-renders, it might be better implemented as a utility function rather than a hook.
Also, since we use this on index.tsx file only, we can just simply use isMobileWeb here.
const canUseDragAndDrop = !isMobileWeb;
cc: @roryabraham
There was a problem hiding this comment.
If we refactor this further as I commented here, then we probably can just remove this.
| screenshotQuality={0} | ||
| /> | ||
| {canUseMultiScan && isMobile() ? ( | ||
| {canUseMultiScan && isMobileWeb ? ( |
There was a problem hiding this comment.
We can remove isMobileWeb condition from mobileCameraView since we only shows mobileCameraView if isMobileWeb is true.


Explanation of Change
Fixed Issues
$ #79929
PROPOSAL: N/A
Tests
Offline tests
QA Steps
// 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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari