[Odometer] Image capture for mWeb#83236
[Odometer] Image capture for mWeb#83236jakubkalinski0 wants to merge 12 commits intoExpensify:mainfrom
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…eterPhotoInformation at the bottom of a camera
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
melvin-bot
bot
requested review from
DylanDylann
and removed request for
a team
|
@DylanDylann 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] |
melvin-bot
bot
requested review from
trjExpensify
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| * On phones that have ultra-wide lens, react-webcam uses ultra-wide by default. | ||
| * The last deviceId is of regular len camera. | ||
| */ | ||
| const requestCameraPermission = useCallback(() => { |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The functions requestCameraPermission, setupCameraPermissionsAndCapabilities, getScreenshot, clearTorchConstraints, capturePhoto, the camera permission useEffect, and the mobile camera view JSX are nearly line-for-line copies from IOURequestStepScan/index.tsx (lines ~149-560). This creates significant maintenance overhead -- any bug fix or improvement must be applied in both places.
Extract the shared camera logic (permission handling, screenshot capture, torch management) into a reusable custom hook (e.g., useMobileCamera) and the camera UI into a shared component. The hook could return { cameraPermissionState, videoConstraints, cameraRef, capturePhoto, toggleFlashlight, isTorchAvailable, ... } and each consumer would only provide its own onCapture callback.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
It's a correct observation but the idea for this PR and the previous PR introducing [Odometer] Image capture for native was to make that feature work in isolation without introducing changes to other parts of the App. The main reason for such course of action was to make testing easier and eschew from creating huge PRs which would be really hard to test in a reasonable timeline and could introduce many bugs to both Scan and Odometer resulting in longer development.
After main implementation of Odometer is done we are going to take care of this issue here [Odometer] DRY shared camera logic between IOURequestStepScan and Odometer image capture
(I will leave this comment unresolved to add more context for anyone in the future)
src/pages/iou/request/step/IOURequestStepOdometerImage/index.tsx
Outdated
Show resolved
Hide resolved
src/pages/iou/request/step/IOURequestStepOdometerImage/index.tsx
Outdated
Show resolved
Hide resolved
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b8a11a845
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| [], | ||
| ); | ||
|
|
||
| const mobileCameraView = () => ( |
There was a problem hiding this comment.
❌ CLEAN-REACT-PATTERNS-4 (docs)
mobileCameraView is an internal render helper function (arrow function returning JSX) that is called in the component's return statement via mobileCameraView(). It closes over the entire component scope, preventing the React Compiler from independently memoizing it.
Extract this into its own component (e.g., <MobileCameraView />) with explicit props. This would also help with the code duplication issue -- the shared camera logic between this and IOURequestStepScan could live in a shared hook, with MobileCameraView being a reusable camera UI component.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
It's a correct observation but the idea for this PR and the previous PR introducing [Odometer] Image capture for native was to make that feature work in isolation without introducing changes to other parts of the App. The main reason for such course of action was to make testing easier and eschew from creating huge PRs which would be really hard to test in a reasonable timeline and could introduce many bugs to both Scan and Odometer resulting in longer development.
After main implementation of Odometer is done we are going to take care of this issue here [Odometer] DRY shared camera logic between IOURequestStepScan and Odometer image capture
(I will leave this comment unresolved to add more context for anyone in the future)
1 participant
Explanation of Change
This PR implements the next part of the Odometer feature, aligned with Phase 1 task 1.D. This PR focuses on the mWeb implementation of the image capture flow.
src/pages/iou/request/step/IOURequestStepOdometerImage/index.tsxto covermWeb. We intentionally did not reuseIOURequestStepScanbecause that screen includes receipt‑specific behaviors (SmartScan, receipt state logic). The implementation covers all the following aspects:Due to duplicated logic across
IOURequestStepOdometerImageandIOURequestStepScanthere is a plan to introduce more abstraction for the camera related logic/features in a followup once the Odometer project is done.Fixed Issues
$ #77266
PROPOSAL: N/A
Tests
Important
Caution
Use Android emulator or a physical device for testing - explanation above ^
Prerequisites:
Turn off camera permissions for Expensify -> the most convenient for you would probably be to set
"Ask every time"FAB-> go to"Track distance"-> choose"Odometer"tab`Thumbnail-> you should see the screen shown below"Continue"-> verify that you are shown a native prompt asking you to allow Expensify to use the camera"Only this time"(pressing"While using this app"would behave the same but will require you to turn off camera permissions for Expensify once again later) -> verify that you now have access to the came (the screen from step 2 is gone and you see a working camera display like the one below)Important
If your device has no back camera then you will see a following screen (same behavior as in the scan component)
Important
If your device has no flash then the
Bolticon of flash in the top right corner will be hidden - in order to test the flash itself you have to do it on a physical deviceOdometer pagewith the photo you have just taken being displayed correctly in the thumbnailthumbnailonce again then you are brought to theImage Previewand it displays the photo correctlyThree dots menuand verify that when you press"Replace"then you are brought to theCamera photo captureand that it works correctly"While using this app"earlier)"Don't allow"when asked about camera permissions -> verify that you are shown this screen with further instructions"these instructions"and verify that you are brought to this"help.expensify"page"help.expensify", i.e. change your browser camera permissions accordingly and refresh the App pageCamera photo capture-> verify that it works properlyGalleryicon at the bottom left corner and verify that all the options work properlyImportant
Proper functionality for
Camera Camcorderwhen you try to submit a video is to show the following prompt as we do not allow videos for Odometer Image (same behavior as Scan)End odometer photo(although if everything worked properly forStart photothen it will work the same for theEnd photo)End odometer photoWarning
When testing
End odometer photoafter theStart odometer phot"your camera permissions forExpensifywere probably set to"Don't allow"after the step 9 so you will have to change it to"Ask every time"when testing the second photo flowOffline tests
Same as Tests
QA Steps
Same as Tests
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: mWeb Chrome
Steps 1 through 7:
1to7.webm
Steps 8 through 12:
8to12.webm
Steps 13:
13.webm
iOS: mWeb Safari
output.mp4