[$250] Fix onboarding modal in macOS Chrome/Safari

[carlosmiceli]

The onboarding modal seems to have issues being displayed in macOS in Chrome and Safari (conversation, GH comment). Seems like there is an issue with screen size and/or refresh. Please fix it and ensure it's always displayed in all possible platforms. We found this while working on this issue: https://github.com/Expensify/App/issues/53326

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021865160679237341916
• Upwork Job ID: 1865160679237341916
• Last Price Increase: 2024-12-06

Issue Owner

Current Issue Owner: @hungvu193

[melvin-bot[bot]]

Triggered auto assignment to @slafortune (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~021865160679237341916

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @hungvu193 (External)

[Shahidullah-Muffakir]

Coming from https://github.com/Expensify/App/issues/53326#issuecomment-2524085623. I’ll work on this, thanks!

[melvin-bot[bot]]

📣 @Shahidullah-Muffakir You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[Shahidullah-Muffakir]

@carlosmiceli as per https://github.com/Expensify/App/issues/53284#issuecomment-2523889800 and after testing with same steps, the onboarding modal works as expected in staging when signing up through staging.expensify.com, after redirecting to staging.new.expensify.com the onboarding flow modal appears as expected. However, the issue persists in prod. Considering this, should we wait for the next production deploy to see if the issue resolves? Thank you.

[carlosmiceli]

should we wait for the next production deploy to see if the issue resolves?

Yes, that sounds like a good idea.

[carlosmiceli]

Not overdue.

[melvin-bot[bot]]

@carlosmiceli, @slafortune, @hungvu193, @Shahidullah-Muffakir Whoops! This issue is 2 days overdue. Let's get this updated quick!

[carlosmiceli]

Still discussing details in the PR.

[carlosmiceli]

@Shahidullah-Muffakir is this still occurring?

[melvin-bot[bot]]

@carlosmiceli, @slafortune, @hungvu193, @Shahidullah-Muffakir Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[hungvu193]

little bump @Shahidullah-Muffakir

[Shahidullah-Muffakir]

@Shahidullah-Muffakir is this still occurring?

Yes, still occurring.

[Shahidullah-Muffakir]

little bump @Shahidullah-Muffakir

PR will be ready by tomorrow.

[Shahidullah-Muffakir]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Fix onboarding modal in macOS Chrome/Safari.

What is the root cause of that problem?

1. In the hasCompletedGuidedSetupFlowSelector function, when the onboardingFlow object is empty or hasCompletedGuidedSetupFlowSelector is undefined, we are returning true: https://github.com/Expensify/App/blob/1466c022e3bb0ab2ffaaab12f78822a2aca827c8/src/libs/onboardingSelectors.ts#L14-L20
2. This results in the useOnboardingFlowRouter hook returning true: https://github.com/Expensify/App/blob/1466c022e3bb0ab2ffaaab12f78822a2aca827c8/src/libs/Navigation/AppNavigator/AuthScreens.tsx#L243 and as the isOnboardingCompleted is true, this condition fails: https://github.com/Expensify/App/blob/1466c022e3bb0ab2ffaaab12f78822a2aca827c8/src/libs/Navigation/AppNavigator/AuthScreens.tsx#L538-L542 Hence, the Onboarding Modal is not shown.

What changes do you think we should make in order to solve the problem?

The same way we return false when the onboarding object is empty in the case of: https://github.com/Expensify/App/blob/1466c022e3bb0ab2ffaaab12f78822a2aca827c8/src/libs/onboardingSelectors.ts#L51-L54, we should return false in the case of hasCompletedGuidedSetupFlowSelector as well if the onboarding is undefined. as:

function hasCompletedGuidedSetupFlowSelector(onboarding: OnyxValue<typeof ONYXKEYS.NVP_ONBOARDING>): boolean | undefined {
    // Onboarding is an empty object for old accounts and accounts migrated from OldDot
    if (Array.isArray(onboarding) || isEmptyObject(onboarding)) {
-         return true;
+        return false;
    }

    if (!isEmptyObject(onboarding) && onboarding?.hasCompletedGuidedSetupFlow === undefined) {
-        return true;
+       return false;
    }

@hungvu193 Before making the PR, could you review the changes I proposed, thanks.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We need to updated these tests as well, to return false instead of true: https://github.com/Expensify/App/blob/1466c022e3bb0ab2ffaaab12f78822a2aca827c8/tests/unit/OnboardingSelectorsTest.ts#L10-L22

[melvin-bot[bot]]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[mountiny]

@Shahidullah-Muffakir We cannot return false there, its intentional that we do not show the onboarding modal for users that do not have the nvp or of the nvp is an empty object

[Shahidullah-Muffakir]

We cannot return false there, its intentional that we do not show the onboarding modal for users that do not have the nvp or of the nvp is an empty object

Got it. Thanks for confirming

[mountiny]

Lets close this in favour of https://github.com/Expensify/App/issues/54322