fix: use in-app VisionCamera for chat attachments on Android and iOS#84770
fix: use in-app VisionCamera for chat attachments on Android and iOS#84770
Conversation
On Android, tapping "Take photo" in the attachment picker launches the system camera intent, which backgrounds Expensify. The OS can reclaim the app's memory during post-capture processing, causing a crash when returning from the camera. Replace the external camera intent with an in-app VisionCamera modal on Android. This keeps Expensify in the foreground during photo capture, eliminating the memory reclaim window. The change only affects Android; iOS continues to use the existing external camera. Fixed Issues: #84018 Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
|
Fixed the failing |
|
Fixed the failing check: the |
The AttachmentCamera component imports react-native-vision-camera which tries to initialize native modules that don't exist in the Jest test environment. This causes all test suites to fail with "system/camera-module-not-found" error. Add a mock similar to other native module mocks in the jest setup file. Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
|
Fixed the failing checks: Added a Jest mock for Root cause: The new Fix: Added a mock for |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot Let's do it for both Android and iOS |
Remove the Platform.OS === 'android' gate so the in-app camera is used on iOS as well, instead of falling back to the external system camera. Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
|
Done — the in-app VisionCamera is now used on both Android and iOS. The Changes in commit e32e587:
|
|
How is it looking? |
|
It seemed good when I tested earlier, will have a thorough look at the code and test again. |
|
@MelvinBot Mark this as ready for review |
|
PR has been marked as ready for review. I also updated the title to reflect that the in-app VisionCamera now covers both Android and iOS. |
|
@MelvinBot Mark this as ready for review |
|
@ShridharGoel 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] |
|
Marked this PR as ready for review. I also updated the PR description and test steps to reflect that the in-app VisionCamera is now used on both Android and iOS (previously the description mentioned iOS behavior was unchanged). |
|
@MelvinBot Is it possible to add a camera switcher option, so that the front camera can also be used? |
Replace the empty spacer in the bottom control bar with a camera flip button using the Rotate icon. Adds cameraPosition state to toggle between 'back' and 'front' devices via useCameraDevice. Flash visibility automatically adapts since front cameras typically lack a flash (already handled by the hasFlash check). Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
|
The failing build checks are unrelated to this PR's code changes and appear to be CI infrastructure issues. Analysis:
Evidence this is not caused by PR changes:
Recommendation: Re-run the failed workflows. If the failures persist, they should be investigated as a CI infrastructure issue separate from this PR. |
|
The failing build checks are unrelated to this PR's changes. Failing checks:
Evidence this is unrelated:
These are pre-existing CI/infrastructure failures, not caused by changes in this PR. |
|
@ShridharGoel can you please merge main again to see if that fixes this |
|
@MelvinBot Merge main |
|
@MelvinBot Merge main into this branch |
…aForAttachments Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
|
Merged latest main into this branch. The merge resolved cleanly. |
|
@srikarparsi Can you help in removing the extra text at the end of the author checklist? It keeps getting added back. |
|
Yes removed, is this ready for review apart from this? |
|
I can take care of the checklist failing |
|
Yes it's ready |
|
All checks on this PR are currently passing — no failing checks found. If you're seeing failures that haven't shown up yet, please re-run the checks and tag me again if needed. |
|
All checks on this PR are currently passing — no failing checks detected. If you're seeing a specific failure, it may have been resolved by a recent push or could be a transient/flaky issue. Please re-run the checks if needed and let me know if the problem recurs. |
|
@ShridharGoel can you please test and add screenshots |
| const device = useCameraDevice(cameraPosition, { | ||
| physicalDevices: ['wide-angle-camera', 'ultra-wide-angle-camera'], | ||
| }); | ||
| const format = useCameraFormat(device, [{photoAspectRatio: 4 / 3}, {photoResolution: 'max'}]); |
There was a problem hiding this comment.
Can we use a constant for this ratio?
There was a problem hiding this comment.
Done — extracted 4 / 3 to CONST.RECEIPT.PHOTO_ASPECT_RATIO and updated the usage.
| * Returns a Promise that resolves with the captured photo as an Asset-compatible object, | ||
| * or resolves with void if the user closes the camera without capturing. | ||
| */ | ||
| const launchInAppCamera = useCallback((): Promise<Asset[] | void> => { |
There was a problem hiding this comment.
Do we need useCallback for these and the below? I think react will handle them automatically?
There was a problem hiding this comment.
Makes sense, @MelvinBot what do you think? Update accordingly.
There was a problem hiding this comment.
Good call — all three (launchInAppCamera, handleCameraCapture, handleCameraClose) had empty dependency arrays, so useCallback wasn't adding value here. Removed the wrappers from all three.
@srikarparsi They are present in the description and the reviewer checklist respectively. |
Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
…pture, handleCameraClose Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
…aForAttachments Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
…ents' into claude-useVisionCameraForAttachments Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
|
Fixed the failing typecheck: merged main and removed the stale |
|
@MelvinBot revert the unrelated changes |
|
The TypeScript check failure was caused by a stale |
|
The
No unrelated changes remain. |
| Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(item?.reportID)); | ||
| } else if ('login' in item) { | ||
| navigateToAndOpenReport(item.login ? [item.login] : [], personalDetails, currentUserAccountID, introSelected, isSelfTourViewed, betas, false); | ||
| navigateToAndOpenReport(item.login ? [item.login] : [], currentUserAccountID, introSelected, isSelfTourViewed, betas, false); |
|
Reverted the merge of main. The branch is now back to the PR-only changes (without main merged in). Note: commit |
Explanation of Change
Slack thread for context: https://expensify.slack.com/archives/C05LX9D6E07/p1772818699627849
On Android, tapping "Take photo" in the chat attachment picker launches the system camera via an external intent, which backgrounds the Expensify app. When the OS is under memory pressure, it can reclaim Expensify's process while the system camera is active — particularly during post-capture processing when memory usage spikes. This causes a crash when the user returns from the camera.
This PR replaces the external camera intent with an in-app camera modal powered by
react-native-vision-camera(VisionCamera), which is already used in the receipt scan flow (IOURequestStepScan). By keeping Expensify in the foreground during photo capture, we eliminate the window in which the OS can reclaim the app's memory. The in-app camera is used on both Android and iOS for a consistent experience.Changes:
AttachmentCameracomponent (AttachmentCamera.tsx): A full-screen modal with VisionCamera viewfinder, shutter button, flash toggle, and back button. Handles camera permissions using the sameCameraPermissionmodule already used by receipt scan.AttachmentPicker/index.native.tsx: The "Take photo" menu item now opens the in-app camera modal instead of launching the external system camera on both Android and iOS. The captured photo flows through the samepickAttachmentprocessing pipeline (resize, validation, upload) as before.Fixed Issues
$ #84018
Tests
+button, then "Add attachment" → "Take photo"Offline tests
+→ "Add attachment" → "Take photo"QA Steps
+→ "Add attachment" → "Take photo"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./** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari