fix: Update empty receipt section in the expense RHP#79545
fix: Update empty receipt section in the expense RHP#79545NikkiWines merged 4 commits intoExpensify:mainfrom
Conversation
|
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 |
|
@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] |
![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: 51d7927039
ℹ️ 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".
| </View> | ||
| </View> | ||
| {isDisplayedInWideRHP && !disabled && <ReceiptAlternativeMethods />} |
There was a problem hiding this comment.
In the wide RHP flow, ReceiptEmptyState is rendered as a Pressable that navigates to the receipt upload on press, and ReceiptAlternativeMethods (which contains <a> links) is now nested inside that pressable. On web/desktop, click events bubble, so clicking “Download the app”/mailto/sms links will also trigger the parent onPress and immediately navigate to the scan/receipt route, making those links effectively unusable in that context. Consider moving ReceiptAlternativeMethods outside the pressable or disabling the parent’s interaction for that sub-tree (e.g., pointerEvents="box-none" or a non-pressable wrapper) so link clicks don’t fire the upload navigation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think this is correct, shouldn't we do something like:
{isDisplayedInWideRHP && !disabled && (
<View onClick={(e) => e.stopPropagation()}>
<ReceiptAlternativeMethods />
</View>
)}
There was a problem hiding this comment.
From testing, pressing the links inside ReceiptAlternativeMethods doesn't bubble to the parent Pressable, so wrapping with stopPropagation is unnecessary. Either way looks fine to me.
There was a problem hiding this comment.
Ok, great! Thanks for confirming 🙌
|
Let me know when this is ready for @Expensify/design to review and I will make a build 👍 |
|
@dannymcclain This is ready for @Expensify/design to review |
|
The code looks fine to me. Wating for the design team to review before completing the checklist |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp
Android: mWeb ChromeScreen.Recording.2026-01-26.at.14.19.13.moviOS: HybridApp
iOS: mWeb SafariScreen.Recording.2026-01-26.at.14.18.52.movMacOS: Chrome / Safari
|
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Looking pretty good! Here are a couple nitpicky comments: We can remove this 4px of padding: We need to add 8px of padding beneath the "other ways" block: |
|
@nkdengineer Kindly bump |
|
@dannymcclain I updated. |
|
🚧 @dannymcclain has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Latest build is looking good to me. cc @dubielzyk-expensify too. One tiny thing, can we remove this period? I noticed our regular upload message (during scan) doesn't use punctuation, so I don't think we need it here either. |
|
@dannymcclain I updated. |
|
Adhoc branch looking great to me 👍 |
|
@dubielzyk-expensify @dannymcclain When clicking on the empty receipt, a similar modal will be displayed as a small RHP. Is it okay? Screen.Recording.2026-01-20.at.10.55.26.mov |
|
That feels a bit duplicative to me. Is that what it was like before? Part of me just wonders if we should directly show the OS dialog as those screen are now practically the same. Mobile goes straight to camera so I'd expect this to be equally direct. Keen to hear what @dannymcclain thinks |
I have same thought with you |
|
Yeah, I agree with you @dubielzyk-expensify. Feels unnecessary with the empty state. |
|
Totally agree! |
|
Cool. Let's change to open the system file picker dialog then instead. It'd also allow for drag n drop anyways so we got both methods covered then 👍 |
|
@nkdengineer you good to make that change? |
|
@trjExpensify Handle the picker here is complex so I think we can handle it as a follow-up after we merge this PR. |
|
@trjExpensify Any thoughts on my comment above? |
|
I'm cool to separate the PRs for it, we should do it as a fast follower though. 👍 |
| </View> | ||
| </View> | ||
| {isDisplayedInWideRHP && !disabled && <ReceiptAlternativeMethods />} |
There was a problem hiding this comment.
I think this is correct, shouldn't we do something like:
{isDisplayedInWideRHP && !disabled && (
<View onClick={(e) => e.stopPropagation()}>
<ReceiptAlternativeMethods />
</View>
)}
|
@nkdengineer over to you! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/NikkiWines in version: 9.3.10-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.10-6 🚀
|
7 participants
Explanation of Change
fix: Update empty receipt section in the expense RHP
Fixed Issues
$ #79189
PROPOSAL: #79189 (comment)
Tests
Offline tests
Same
QA Steps
Same as test
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
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari