Display new UI for alternate methods of scanning receipts#71340
Display new UI for alternate methods of scanning receipts#71340inimaga merged 38 commits intoExpensify:mainfrom
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
6f9073b to
5f03a26
Compare
|
@parasharrajat 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] |
| const shouldShowDownloadApp = !hasLoggedIntoMobileApp; | ||
| const shouldShowAddPhoneNumber = !hasPhoneNumber; | ||
| const shouldShowTextReceipts = hasPhoneNumber; |
There was a problem hiding this comment.
Remove extra variables and directly use the conditions.
src/hooks/useHasPhoneNumber.ts
Outdated
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import useOnyx from './useOnyx'; | ||
|
|
||
| const useHasPhoneNumber = () => { |
There was a problem hiding this comment.
| const useHasPhoneNumber = () => { | |
| const useHasPhoneNumberLogin = () => { |
src/hooks/useHasPhoneNumber.ts
Outdated
| .map((login) => Str.removeSMSDomain(login)) | ||
| .find((login) => Str.isValidE164Phone(login)); |
There was a problem hiding this comment.
Can be done in one loop. Use .some
| .map((login) => Str.removeSMSDomain(login)) | ||
| .find((login) => Str.isValidE164Phone(login)); | ||
|
|
||
| const smsLoginExists = Object.keys(loginList ?? {}).some((login) => Str.isSMSLogin(login)); |
There was a problem hiding this comment.
How is it different from validPhoneInLoginList?
src/languages/pt-BR.ts
Outdated
| }), | ||
| }, | ||
| receipt: { | ||
| alternativeMethodsTextReceipts: ({phoneNumber}: ReceiptAlternativeMethodsPhoneNumberParams) => `Text receipts to ${phoneNumber} (US numbers only)`, |
There was a problem hiding this comment.
Convert to a single string per sentence. Don't break into parts. Use RenderHTML component to render that whole sentense.
| /> | ||
| </DragAndDropConsumer> | ||
| {!shouldHideDownloadAppBanner && <DownloadAppBanner onLayout={(e) => setDownloadAppBannerHeight(e.nativeEvent.layout.height)} />} | ||
| {!shouldHideAlternativeMethods && <ReceiptAlternativeMethods onLayout={(e) => setAlternativeMethodsHeight(e.nativeEvent.layout.height)} />} |
There was a problem hiding this comment.
DownloadAppBanner Remove this component if we are not using it anymore.
| onLayout?: (event: LayoutChangeEvent) => void; | ||
| }; | ||
|
|
||
| function ReceiptAlternativeMethods({onLayout}: ReceiptAlternativeMethodsProps) { |
There was a problem hiding this comment.
Didn't get the usage of displayName here, can you clarify?
There was a problem hiding this comment.
| function ReceiptAlternativeMethods({onLayout}: ReceiptAlternativeMethodsProps) { | |
| ReceiptAlternativeMethods.displayName = "ReceiptAlternativeMethods"; |
Add this at the bottom of this component.
| const {environmentURL} = useEnvironment(); | ||
| const {translate} = useLocalize(); | ||
| const {hasLoggedIntoMobileApp, isLastMobileAppLoginLoaded} = useHasLoggedIntoMobileApp(); | ||
| const {hasPhoneNumberLogin, isLoaded: isPhoneNumberLoaded} = useHasPhoneNumberLogin(); |
There was a problem hiding this comment.
Can you rename isLoaded to isPhoneNumberLoaded in the hook
| title={translate('common.getTheApp')} | ||
| subtitle={translate('common.scanReceiptsOnTheGo')} |
There was a problem hiding this comment.
let's remove these strings from the translations if not used in the app.
src/languages/es.ts
Outdated
| alternativeMethodsTextReceipts: ({phoneNumber}: ReceiptAlternativeMethodsPhoneNumberParams) => | ||
| `<muted-text-label>Text receipts to ${phoneNumber} (US numbers only)</muted-text-label>`, | ||
| alternativeMethodsForwardReceipts: ({email}: {email: string}) => `<muted-text-label>Forward receipts to <a href="mailto:${email}">${email}</a></muted-text-label>`, | ||
| alternativeMethodsDownloadApp: ({downloadUrl}: {downloadUrl: string}) => `<muted-text-label><a href="${downloadUrl}">Download the app</a> to scan from your phone</muted-text-label>`, | ||
| alternativeMethodsTitle: 'Other ways to add receipts:', | ||
| alternativeMethodsAddPhoneNumber: ({phoneNumber, contactMethodsUrl}: ReceiptAlternativeMethodsPhoneNumberParams & {contactMethodsUrl: string}) => | ||
| `<muted-text-label><a href="${contactMethodsUrl}">Add your number</a> to text receipts to ${phoneNumber}</muted-text-label>`, | ||
| desktopSubtitleMultiple: 'or drag and drop them here', | ||
| desktopSubtitleSingle: 'or drag and drop them here', |
There was a problem hiding this comment.
Have you confirmed this on slack? Can you please add the slack discussion link in PR checklist
|
Please use meaningful commit messages. |
src/languages/es.ts
Outdated
| } from './params'; | ||
| import type {TranslationDeepObject} from './types'; | ||
|
|
||
| type ReceiptAlternativeMethodsPhoneNumberParams = { |
There was a problem hiding this comment.
Move this type to params and import on each file.
|
@ShridharGoel please add QA steps and test steps. You can see other PRs for examples. Also, fix the checks. |
|
I manually ran the script and it keeps failing unfortunately. Can you please create an openAI key and run the script yourself like it was described here? |
|
@parasharrajat The other translations are added now. |
parasharrajat
left a comment
There was a problem hiding this comment.
@ShridharGoel It is all messed up. Can you carefully check all translation files and sync them.
- Same key structure.
- same key location.
- All translation files have same changes except strings. in different languages.
|
@parasharrajat Updated the order of strings. |
parasharrajat
left a comment
There was a problem hiding this comment.
May be you are not seeing what I am seeing but there are still issues.
| dragReceiptBeforeEmail: 'Drag a receipt onto this page, forward a receipt to ', | ||
| dragReceiptsBeforeEmail: 'Drag receipts onto this page, forward receipts to ', | ||
| dragReceiptAfterEmail: ' or choose a file to upload below.', | ||
| dragReceiptsAfterEmail: ' or choose files to upload below.', |
There was a problem hiding this comment.
This change is not consistent.
|
Also, attach recordings of the flow impacted on all platforms. |
parasharrajat
left a comment
There was a problem hiding this comment.
Reviewer Checklist
- I have verified the author checklist is complete (all boxes are checked off).
- I verified the correct issue is linked in the
### Fixed Issuessection above - I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the
Testssection - I verified the steps for Staging and/or Production testing are in the
QA stepssection - I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
- I verified the steps for local testing are in the
- I checked that screenshots or videos are included for tests on all platforms
- I included screenshots or videos for tests on all platforms
- I verified tests pass on all platforms & I tested again on:
- Android: Native
- Android: mWeb Chrome
- iOS: Native
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
- If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
- I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e.
toggleReportand notonIconClick). - I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g.
myBool && <MyComponent />. - I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to
src/languages/*files and using the translation method - I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the
Waiting for Copylabel for a copy review on the original GH to get the correct copy. - I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in
STYLE.md) were followed
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e.
- If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
- I verified that this PR follows the guidelines as stated in the Review Guidelines
- I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like
Avatar, I verified the components usingAvatarhave been tested & I retested again) - I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
- I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
- If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a
/** comment above it */ - The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to
thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor) - Any internal methods bound to
thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick) - All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
- If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
- If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e.
StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
- If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
- If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like
Avataris modified, I verified thatAvataris working as expected in all cases) - If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
- If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
- If the PR modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added
Designlabel so the design team can review the changes.
- If a new page is added, I verified it's using the
ScrollViewcomponent to make it scrollable when more elements are added to the page. - If the
mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps. - I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.
🎀 👀 🎀 C+ reviewed
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@shawnborton @trjExpensify for future reference, you can run translations on any given PR by triggering this workflow: https://github.com/Expensify/App/actions/workflows/generateTranslations.yml
|
|
Oh, great to know! |
|
🚀 Deployed to staging by https://github.com/inimaga in version: 9.2.27-0 🚀
|
|
Hi @ShridharGoel , the new upload receipts UI on the scan tab is seen on Web & desktop only. No changes on mweb, iOS & Android apps. Is it expected, or do we need to log an issue? |
|
That's expected |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.2.27-6 🚀
|


Explanation of Change
Display new UI for alternate methods of scanning receipts.
Fixed Issues
$ #70693
PROPOSAL: #70693 (comment)
Tests
Update Onyx data via console to simulate the different scenarios (else you can use different accounts which have the scenarios):
1. No Mobile App • No Phone Number
Expectation:
2. Mobile App Logged In • No Phone Number
Expectation:
3. No Mobile App • Phone Number Present
Expectation:
4. Mobile App Logged In • Phone Number Present
Expectation:
Download the app should link to: https://new.expensify.com/settings/about/app-download-links
Add your number should link to: https://new.expensify.com/settings/profile/contact-methods
Verify that no errors appear in the JS console
Offline tests
QA Steps
Scenario A – No mobile app • No phone number
Scenario B – Mobile app used • No phone number
Scenario C – No mobile app • Phone number added
Scenario D – Mobile app used • Phone number added
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.Screenshots/Videos
Android: Native
Android: mWeb Chrome
Screen.Recording.2025-10-07.at.1.10.06.PM.mov
iOS: Native
Simulator.Screen.Recording.-.iPhone.16.Plus.-.2025-10-07.at.13.13.01.mp4
iOS: mWeb Safari
Simulator.Screen.Recording.-.iPhone.16.Plus.-.2025-10-07.at.13.11.56.mp4
MacOS: Chrome / Safari
Screen.Recording.2025-10-07.at.1.10.41.PM.mov
MacOS: Desktop
Screen.Recording.2025-10-07.at.1.15.23.PM.mov