feat: Refator USD flow to use useSubPage#90302
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
|
@dukenv0307 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6d3e94ddb
ℹ️ 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".
| (onfidoData: OnfidoData) => { | ||
| verifyIdentityForBankAccount(Number(bankAccountID), {...onfidoData, applicantID: onfidoApplicantID}, policyID); | ||
| updateReimbursementAccountDraft({isOnfidoSetupComplete: true}); | ||
| onSubmit?.(); |
There was a problem hiding this comment.
Wait for Onfido verification write before advancing flow
Calling onSubmit immediately in handleOnfidoSuccess advances to the Company step before verifyIdentityForBankAccount finishes, so a failed/slow API write (e.g., offline or backend error) can still move the user forward with identity verification not actually persisted. Previously this step transition was gated by the backend-updated reimbursement state; this change removes that guard and can leave users in later steps with an invalid verification state.
Useful? React with 👍 / 👎.
|
No new product considerations - removing my assignment and unsubscribing. |
|
I'll review it. don't know why Melvin removed me |
|
on it now |
Screen.Recording.2026-05-19.at.09.22.59.mov |
|
Looks good Screen.Recording.2026-05-19.at.09.37.32.mov |
Found the root cause, working on a fix right now |
|
@dukenv0307 I've fixed the error you've spotted and lint issue. Should be good to review further |
|
@MrMuzyk I can still reproduce the date picker issue on Safari Screen.Recording.2026-05-19.at.23.17.03.mov |
|
Let me double check 🤔 |
|
I think the page is having some issues right now so I'll get back to it tomorrow morning |
|
I wasn't able to fix it and Im not sure whether we want to spend more time on it now. While it is poor UX and should be fixed it is only under very specific condition on a single platform. It doesn't block user from interacting with the app and modal still can be closed if you click anywhere outside of it. In order to fix it on all other platforms I've used existing mechanism that was already implemented but not in use for this component - Im almost certain that the same bug can be reproduced in other places that use this prop. @arosiclair Could we maybe give it the same treatment as we did with flicker bug? Open a separate issue for it and revisit it later? |
|
Yeah I think that makes sense since the bug is not critical - it only occurs on mWeb Safari and you can just dismiss the popup when it happens. Made an issue here: #91311 |
|
@dukenv0307 Did everything else work fine? |
|
on it now |
0930dbb to
e9a2719
Compare
|
🚧 @arosiclair 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! 🧪🧪
|
|
✋ 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/arosiclair in version: 9.3.84-0 🚀
Bundle Size Analysis (Sentry): |
|
Deploy Blocker #91882 was identified to be related to this PR. |
|
Deploy Blocker ##91884 was identified to be related to this PR. |
|
🚀 Deployed to staging by https://github.com/arosiclair in version: 9.3.86-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to staging by https://github.com/arosiclair in version: 9.3.88-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.88-2 🚀
|
|
Deploy Blocker #92143 was identified to be related to this PR. |
Explanation of Change
Restores reverted #88193 together with fixes for newly found bugs
Fixed Issues
$ #79048
PROPOSAL:
Tests
Same as QA steps +
Scenario A:
Scenario B:
Scenario C:
Offline tests
QA Steps
Note
Known issues:
Prerequisites: being an admin of at least one workspace, the workspace tested should have USD as the workspace currency
Partially setup bank accounts
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: Native
android.mp4
Android: mWeb Chrome
android.mweb.mp4
iOS: Native
ios-native.mp4
iOS: mWeb Safari
ios-web.mp4
MacOS: Chrome / Safari
web.mp4