Migrate ExplanationModal to CenteredModalLayout#93292
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c0368e9b9
ℹ️ 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".
| <CenteredModalLayout onBackdropPress={completeHybridAppOnboarding}> | ||
| <View style={contentStyle}> | ||
| <FeatureTrainingContent | ||
| title={translate('onboarding.explanationModal.title')} | ||
| description={translate('onboarding.explanationModal.description')} | ||
| secondaryDescription={translate('onboarding.explanationModal.secondaryDescription')} | ||
| confirmText={translate('footer.getStarted')} | ||
| videoURL={CONST.WELCOME_VIDEO_URL} | ||
| onClose={handleClose} |
There was a problem hiding this comment.
Complete onboarding when the route is dismissed
This only marks the hybrid onboarding as completed for the custom backdrop and CTA paths. Because the modal is now a transparent navigator screen instead of the Modal component, dismissing it via browser/Android back (or any navigator pop) bypasses both onBackdropPress and FeatureTrainingContent's onClose, leaving tryNewDot.isHybridAppOnboardingCompleted false and never notifying OldDot; the user can close the explanation without the completion side effect. Add a before-remove/unmount handler for this screen or otherwise route all dismissals through completeHybridAppOnboarding().
Useful? React with 👍 / 👎.
|
@Expensify/design I think I found a bug here (non transparent overlay on narrow screens) that I also fixed in this PR. Just wanted to confirm that it wasn't intentional. 2 changes:
Before / After vid: Screen.Recording.2026-06-12.at.12.25.30.mov |
|
@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] |
|
Wow, I didn't even realize we still had that modal. I'm guessing the video is quite dated? cc @Expensify/marketing in case you have thoughts. |
The video I made today 😢 Also all training modals have the same issue. |
|
No product review needed |
|
Is it a migration modal? I don't think I've seen it out in the wild either. 🤔 |
Yeah that looks very old I vaguely remember us having the Intro to NewDot video in that one. I think it was used in the very first wave of the Onboarding project |
|
I haven't been able to get to this modal from the flow either. On web you can see it on https://staging.new.expensify.com/onboarding/explanation As far as the code reads it should appear when user is transitioning from OldDot, and NOT when it's not a singular entry (like for travel) |
Has anyone confirmed how and when this modal shows in production? 🤔 I don't think it's related to the tryNewDot.nudgeMigration NVP is it? Sounds like it might just be old, and needs to be killed - but would be good to confirm when it's actually expected to be shown to be sure. |
|
@trjExpensify if the account has Screen.Recording.2026-06-15.at.14.10.47.movI do not have access to such account, so triggered it by hardcoding |
That NVP is used for regular onboarding, so I don't think this has anything to do with migration or nudging. I would think we can probably just kill the mobile modal. CC: @Julesssss for a second opinion. |
|
Is this ready for review, or are we still finalizing the modal? |
Yeah, it's old and I think better to just delete. Though we may need to keep checking for the NVP. Currently if we detect that NVP we SKIP showing the regular onboarding modal, so we should make sure we continue to skip that for existing users who switched for the first time. |
|
@parasharrajat a quick summary:
EDIT: Remove explanation modal PR is open |
|
This can be closed |




Explanation of Change
This PR extracts
FeatureTrainingContentso screens render content directly without a JS Modal wrapper. Then it migratesExplanationModal.Fixed Issues
$ #90475
PROPOSAL:
Tests
Pre requisite:
add in
src/hooks/useOnboardingFlow.ts#69to trigger modalOffline tests
Same as tests
QA Steps
Pre requisite:
On web you can add
/onboarding/explanation. On mobile it requires switching from OldDot to new w/o onboarding complete, then follow test steps.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
Screen.Recording.2026-06-11.at.18.25.49.mov
Android: mWeb Chrome
Screen.Recording.2026-06-12.at.12.19.48.mov
iOS: Native
Screen.Recording.2026-06-11.at.18.27.15.mov
iOS: mWeb Safari
Screen.Recording.2026-06-12.at.12.17.00.mov
MacOS: Chrome / Safari
Screen.Recording.2026-06-11.at.18.25.19.mov