Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.29k stars 2.72k forks source link

[HOLD for payment 2024-08-21] [$250] [FT] - Update onboarding flow to drop user into main chat screen after clicking "Get Started" on welcome video [Mobile] #45094

Closed LLPeckham closed 2 weeks ago

LLPeckham commented 1 month ago

Hi there, we need to update the onboarding flow on Mobile for when a user selects "Get paid back by my employer"

What's happening now/what's broken: User signs up for New Expensify on mobile app > is shown intent selection modal > clicks selection (get paid back by my employer) > enters name > is shown welcome video > is dropped into Concierge chat DM

In summary, we do not want the user to drop into the Concierge DM after clicking "Get started" on the welcome video modal, and instead be shown the main home screen where they can see the GBR (screenshot below). Note we are A/B testing so the GBR will either be shown from a Concierge DM or from Expensify DM.

Screenshot of current flow for reference: Screenshot 2024-07-09 at 4 45 17β€―PM

Expected: User signs up for new Expensify on mobile app > is shown intent selection modal > clicks selection (get paid back by my employer) > enters name > is shown welcome video > is dropped into the main chat home screen with GBR on onboarding task list

Screenshot 2024-07-09 at 11 26 42β€―AM

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0174265012f8ae6366
  • Upwork Job ID: 1810704059603315165
  • Last Price Increase: 2024-07-16
Issue OwnerCurrent Issue Owner: @zanyrenney
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @zanyrenney (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] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

daledah commented 1 month ago

Proposal

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

Update onboarding flow to drop user into main chat screen after clicking "Get Started" on welcome video [Mobile]

What is the root cause of that problem?

New feature request

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

Connect to ONYXKEYS.ONBOARDING_PURPOSE_SELECTED here https://github.com/Expensify/App/blob/50bea9020567feae078708f8933d2c7811c818e9/src/libs/Navigation/AppNavigator/Navigators/OnboardingModalNavigator.tsx#L37

const [onboardingPurposeSelected] = useOnyx(ONYXKEYS.ONBOARDING_PURPOSE_SELECTED);

And before this line (or after) https://github.com/Expensify/App/blob/50bea9020567feae078708f8933d2c7811c818e9/src/libs/Navigation/AppNavigator/Navigators/OnboardingModalNavigator.tsx#L45, early return if onboardingPurposeSelected is newDotEmployer (we'll export onboardingChoices and use onboardingChoices.EMPLOYER)

if (onboardingPurposeSelected === 'newDotEmployer') {
    return;
}

So it will not navigate to Concierge page if it's Get paid back by my employer purpose.

The onboardingPurposeSelected could optionally be used as a ref so we always have its latest value inside useEffect

What alternative solutions did you explore? (Optional)

Use useOnyx on the ONYXKEYS.ONBOARDING_PURPOSE_SELECTED key in OnboardingWelcomeVideo

In https://github.com/Expensify/App/blob/e6804ca72552dfc87ecbbc59d5efbd0d95b8b4e9/src/components/OnboardingWelcomeVideo.tsx#L10, get the onboardingPurposeSelected from the route, if the onboarding purpose indicates the Get paid back by my employer (this), pass onConfirm to the FeatureTrainingModal and in it, navigate to the home/LHN route

Also some enhancements:

Alternative for the recommended action here

Replace both lines by the navigation back to home, such as

Navigation.setShouldPopAllStateOnUP(true);
Navigation.goBack(ROUTES.HOME, true, true);

This will pop all the navigation state as well, we do the same in some other scenarios for example https://github.com/Expensify/App/blob/8d9b20b65f6bb00d5e317008f458ba87b5d9737e/src/libs/actions/Report.ts#L2215, https://github.com/Expensify/App/blob/8d9b20b65f6bb00d5e317008f458ba87b5d9737e/src/pages/home/ReportScreen.tsx#L546

We can explore only goBack without setShouldPopAllStateOnUP(true) but I like the setShouldPopAllStateOnUP more because it makes sure we have a clean navigation structure after the going back.

An enhancement we can make is we only do that going back logic if the user is not already at the Home screen, as there's no need to go back to Home screen if the user is already there. Also can limit that change to only small screen width, by isSmallScreenWidth

daledah commented 1 month ago

I updated proposal to add an alternative solution

daledah commented 1 month ago

I updated proposal to add alternative solution and add minor details

pankajsoftyoi commented 1 month ago

Proposal

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

Update onboarding flow to drop a user into the main chat screen after clicking "Get Started" on welcome video

What is the root cause of that problem?

New feature request

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

Remove the code that navigates users to the concierge chat when they complete the onboarding flow:

https://github.com/Expensify/App/blob/50bea9020567feae078708f8933d2c7811c818e9/src/libs/Navigation/AppNavigator/Navigators/OnboardingModalNavigator.tsx#L46

What alternative solutions did you explore? (Optional)

To skip this step for new users only, add this line outside of useEffect

const [onboardingPurposeSelected] = useOnyx(ONYXKEYS.ONBOARDING_PURPOSE_SELECTED);

https://github.com/Expensify/App/blob/50bea9020567feae078708f8933d2c7811c818e9/src/libs/Navigation/AppNavigator/Navigators/OnboardingModalNavigator.tsx#L42-L47

Then, add a conditional statement before Report.navigateToConciergeChat();

Navigation.isNavigationReady().then(() => {
    // Need to go back to previous route and then redirect to Concierge,
    // otherwise going back on Concierge will go to onboarding and then redirected to Concierge again
    Navigation.goBack();
    if (onboardingPurposeSelected === 'newDotEmployer') {
        Report.navigateToSystemChat()
        return;
    }
    Report.navigateToConciergeChat();
});
Pujan92 commented 1 month ago

@daledah's proposal to return early makes sense.

@mountiny Just wanted to confirm whether we need to navigate concierge here https://github.com/Expensify/App/blob/50bea9020567feae078708f8933d2c7811c818e9/src/libs/Navigation/AppNavigator/Navigators/OnboardingModalNavigator.tsx#L46

I see it is added here as a part of a solution for the deeplink onboarding issue but not sure if it's required. Without it, the onboarding modal gets opened and closed immediately due to goback, isn't that ok for the user instead of taking them to concierge?

https://github.com/Expensify/App/assets/14358475/a1c083df-8f83-4044-8813-9371a84238de

mountiny commented 1 month ago

@Pujan92 Hmm I think that in that case we usually do not want to see any action, in the video you could see that the page was transitioning in and out.

So it might be safer to go to the LHN home / the last accessed report in that case

Pujan92 commented 1 month ago

Thanks @mountiny, @daledah @pankajsoftyoi Could you plz update your proposal to navigate/back the user to the LHN home.

pankajsoftyoi commented 1 month ago

@Pujan92 Proposal updated https://github.com/Expensify/App/issues/45094#issuecomment-2220485175

daledah commented 1 month ago

I updated my proposal earlier but still need to test a bit more, @Pujan92 I'll let you know soon once all good to review again

Pujan92 commented 1 month ago

Report.navigateToSystemChat()

@pankajsoftyoi we need to come back to the home route, not the system chat.

Replace this line by the navigation back to home, such as Navigation.navigate(ROUTES.HOME);

@daledah I think navigate to home won't work as the TopmostBottomTabRoute is Home only, so it won't navigate to the same screen. Instead of navigate we can think of goBack.

melvin-bot[bot] commented 1 month ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

daledah commented 1 month ago

@daledah I think navigate to home won't work as the TopmostBottomTabRoute is Home only, so it won't navigate to the same screen. Instead of navigate we can think of goBack.

@Pujan92 Thanks for your feedback. I updated the alternative solution in the proposal, it should work well now.

Pujan92 commented 1 month ago

The below suggested change for smaller screens in @daledah's proposal seems to work correctly. I think we can proceed with it. We can make a minor change in Navigation.goBack(ROUTES.HOME, true, true); which can be handled in the PR.

Navigation.setShouldPopAllStateOnUP(true);
Navigation.goBack(ROUTES.HOME, true, true);

πŸŽ€πŸ‘€πŸŽ€ C+ reviewed

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @yuwenmemon, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot[bot] commented 1 month ago

πŸ“£ @daledah 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 πŸ“–

daledah commented 1 month ago

@Pujan92 this PR is ready for review.

melvin-bot[bot] commented 3 weeks ago

This issue has not been updated in over 15 days. @Pujan92, @mountiny, @zanyrenney, @daledah eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

mountiny commented 3 weeks ago

$250 to @daledah and to @Pujan92

zanyrenney commented 2 weeks ago

Needing to remake the job as it closed. 2024-08-21_10-40-33

zanyrenney commented 2 weeks ago

Hey @daledah Upwork's search filter is not working, can you please apply to the job directly? Thanks! 2024-08-21_10-48-34

Link is here - https://www.upwork.com/jobs/~01e554c2a63ca11b1d

daledah commented 2 weeks ago

@zanyrenney I can't open the job link, it says This job is private. Only freelancers invited by client can view this job.

Instead, could you send the offer directly to my profile here https://www.upwork.com/freelancers/~0138d999529f34d33f

Thx

zanyrenney commented 2 weeks ago

Hey @daledah the issue is that I can't search. The search bar in upwork isn't working to find you so i would have to click through all 500+ Expensify hires even with the profile link it doesn't help.

I have made the job public now, so please reapply there,

daledah commented 2 weeks ago

@zanyrenney Could you try searching for "Dai L"? Maybe the search term is not correct so I'm not showing up in the result.

Sorry currently I don't have any Upwork connects so could not apply.

melvin-bot[bot] commented 2 weeks ago

Payment Summary

Upwork Job

BugZero Checklist (@zanyrenney)

zanyrenney commented 2 weeks ago

https://www.upwork.com/jobs/~01d3afa424727566eb @daledah please accept.

daledah commented 2 weeks ago

@zanyrenney I accepted thanks

zanyrenney commented 2 weeks ago

please accept contract @daledah

daledah commented 2 weeks ago

@zanyrenney Done!

zanyrenney commented 2 weeks ago

No regression test required.

Payment summary: @daledah paid $250 via upwork @Pujan92 please request $250 via NewDot.

Thanks!

garrettmknight commented 1 week ago

@Pujan92's request has been paid via NewDot.