Repository navigation
Return to the intent list from work email Back after a refresh - #103532
Conversation
The work email Back button called Navigation.goBack() with no fallback route. After a refresh the onboarding modal only holds the work email screen, so Back popped the whole modal. Pass ONBOARDING_PURPOSE so goUp replaces into the intent list when it is no longer on the stack. Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@Pujan92 @iwiznia @carlosmiceli One of you needs to 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] |
|
@MelvinBot fix eslint |
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
… so popover cells pass ESLint Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com>
|
@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] |
|
Fixed the ESLint check in 6252ada. The 3 I changed both |
|
All checks now pass on 6252ada. I didn't push anything new. ESLint failed on the first commit with 3 The only remaining failure is |
|
I'll take a look later today |
| canEdit: boolean | undefined; | ||
| value?: T; | ||
| onSave?: (value: T) => void; | ||
| onSave?: (value: T) => InlineEditSaveResult; |
There was a problem hiding this comment.
@MelvinBot Do we really need to use generic here?
There was a problem hiding this comment.
Yes, this line on the generic type is the change ESLint needs. The change to the non-generic type isn't needed.
UsePopoverEditStateOptionsGeneric<T>already exists onmain.DateCell,CategoryCell, andTagCellcallusePopoverEditState<T>, so theironSaveis checked against this type. The non-generic impl exists only because OXC can't compile hooks with type params (lines 44-45).- I tested both changes locally. With only this line changed, ESLint and typecheck pass. With only the non-generic
onSaveat line 26 changed, all 3no-misused-promiseserrors return.
I kept line 26 aligned so the two option types match, but it can go back to void.
carlosmiceli, should I revert line 26?
|
It's fixed: Screen.Recording.2026-10-09.at.10.00.57.mov |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-10-09.at.10.00.57.mov |
|
All yours @iwiznia |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 iwiznia 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! 🧪🧪
|
Explanation of Change
The Back button on the onboarding work email step (shown for the "Join my company workspace" intent) called
Navigation.goBack()with no fallback route. Without a refresh, the onboarding stack is[PURPOSE, WORK_EMAIL], so Back pops to the intent list. After a refresh, the stack is rebuilt from the URL with only the work email screen, so Back popped the whole onboarding modal.This passes
ROUTES.ONBOARDING_PURPOSE.getRoute()as the fallback, matching the other onboarding steps. If the intent list is still on the stack, Back pops to it as before. If not,goUpreplaces the work email screen with the intent list inside the modal.Also adds a regression test in
tests/ui/WorkEmailOnboardingNavigation.tsxthat renders the work email screen alone on the stack (the post-refresh state) and asserts Back lands on the intent list. The test fails without the fix.Fixed Issues
$ #103530
PROPOSAL: #103530 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Offline tests
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas do they need to test for regressions.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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.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
AI Tests
npm run typecheck: passednpm run lint-changedand ESLint on both changed files: passednpm run spell-changed: passed (0 issues)tests/ui/WorkEmailOnboarding.tsx,tests/ui/WorkEmailOnboardingNavigation.tsx,tests/ui/WorkEmailOnboardingNativeBack.tsx: 55/55 passed. The new test fails onmainwithout the fix.npm test,npm run storybooksmoke test,oxfmtformat check, andreact-compiler-compliance-check(the last could not resolve its base ref in this environment). The source change only adds an argument to an existing callback.