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 Analysis: The build error is: This is a missing Expo autolinking script in Evidence: This PR only modifies files in The other failing check ( Recommendation: Re-running the failed |
|
The failing check Analysis: The error is Evidence: The exact same error is occurring on the The other failing check ( |
|
@MelvinBot rewrite the checklist in the description, ensuring that it is written fully and formatted correctly. |
|
Done — I've rewritten the PR description checklist. Changes:
|
|
The failing Analysis: The Android build fails with: This is a CI environment / dependency resolution issue — the Evidence: The PR changes are limited to The only other failing check is Re-running the failed workflow should resolve this. |
|
@MelvinBot Copy paste the below into the checklist section instead of what you have currently: Use this
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / Safari |
|
Updated the PR Author Checklist and Screenshots/Videos sections as requested. |
|
The failing check Analysis: The build fails with: This is a pre-existing CI infrastructure issue with the Expo autolinking Gradle script not being found in Evidence: This PR only changes TypeScript components ( The only other failing check is |
|
The failing checks are unrelated to this PR's changes. Failing checks:
Evidence this is infrastructure-wide:
This PR's changes are purely TypeScript/React Native files ( All code-quality checks pass: ESLint, typecheck, prettier, spellcheck, all 8 test jobs, storybook, perf-tests, and both HybridApp builds ( |
|
@MelvinBot remove the extra text at the end of the description (anything after the MacOS screenshot section) |
|
@mountiny This is ready for your review |
|
Cleaned up the PR description — removed the extra checklist items and |
|
The failing build checks are unrelated to this PR's changes. Failing checks:
Analysis:
This is a build infrastructure issue — the expo autolinking script is missing from Evidence this is not PR-related:
This appears to be a transient CI infrastructure issue. Re-running the failed checks should resolve it. |
|
@ShridharGoel can you please sync main to fix the actions? |
|
@MelvinBot Merge main |
…aForAttachments Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
|
Merged main into this branch. The merge resolved cleanly (155 files changed from main). |
|
@srikarparsi can you please handle the review as i am going ooo |
|
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 |
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