Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250] [HOLD payment 2024-08-09] [CRITICAL] [Guided setup] The onboarding flow can be dismissed by navigating to different route #44401

Closed
mountiny opened this issue Jun 25, 2024 · 36 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@mountiny
Copy link
Contributor

mountiny commented Jun 25, 2024

Problem

When new users sign up, they are presented with an onboarding flow. This flow is crucial in the user's onboarding journey and aims to set them up for success using the app. It also provides useful information for us. We need to ensure that every new user completes the onboarding flow.

However, the user is able to navigate to a different route in the app, either by going back into the browser or changing the URL directly to any other in-app URL and the modal is not shown again.

Solution

Find a generic navigation solution that will check the user's onboarding NVP value and whether the hasCompletedGuidedSetupFlow property is set to false, in which case we should present them with the modal.

Note that if its existing user is coming from oldDot, the onboarding NVP will be an empty array. So we only want to show the modal when the onboarding.hasCompletedGuidedSetupFlow === false

That is, however, true for any URL or deep link; if they go back in the browser, they should be re-navigated to the onboarding flow until it is completed. It's technically a blocking pattern for them.

cc @adamgrzybowski @WojtekBoman @kosmydel

Issue OwnerCurrent Issue Owner: @
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0174de33b05a0967f8
  • Upwork Job ID: 1821685945045066052
  • Last Price Increase: 2024-08-08
Issue OwnerCurrent Issue Owner: @anmurali
@mountiny mountiny added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 25, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 25, 2024
Copy link

melvin-bot bot commented Jun 25, 2024

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

Copy link

melvin-bot bot commented Jun 25, 2024

Current assignee @anmurali is eligible for the Bug assigner, not assigning anyone new.

@mountiny mountiny removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 25, 2024
@mountiny
Copy link
Contributor Author

Not looking for proposals really, would like to get the navigation team to find the best, performant and intuitive solution for this

@Kicu
Copy link
Contributor

Kicu commented Jun 27, 2024

FYI update: This is being worked on by @adamgrzybowski and @filip-solecki and I have also chimed in a bit.
However for the past 2 days we had an unusual amount of navigation related bugs and regressions so its taking more time.

@allgandalf
Copy link
Contributor

ahh, we merged a PR simplifying the Navigation, after the current deploy in progress , most of the issues should be resolved

@Kicu
Copy link
Contributor

Kicu commented Jun 27, 2024

@allgandalf can you please share a link to which PR, so we can understand what's been done there?

@allgandalf
Copy link
Contributor

allgandalf commented Jun 27, 2024

if you are facing problem with tab navigation then there's the issue #36476 / PR #39520

@Kicu
Copy link
Contributor

Kicu commented Jun 27, 2024

@allgandalf Im not sure we're talking about the same issue. No we're not facing a problem with tab navigation. This is an issue related to onboarding and that it can be dismissed before finishing it.

Neither of the tow PRs seem to be related to this issue? Also focus trap is quite old at this point

@allgandalf
Copy link
Contributor

However for the past 2 days we had an unusual amount of navigation related bugs and regressions so its taking more time.

Can you be a little brief? what is happening exactly

@Kicu
Copy link
Contributor

Kicu commented Jun 27, 2024

Sorry I don't really understand what want from me. Let's keep the discussion on this issue relevant to this issue I just gave a brief update that we're working on it.

@adamgrzybowski
Copy link
Contributor

@mountiny we should have a draft today

@adamgrzybowski
Copy link
Contributor

It's a bit tricky, but our solution seems to work so far

@adamgrzybowski
Copy link
Contributor

@mountiny we have a draft PR here. We have some issue with redirecting when the user types RHP route in the URL bar but besides that it works quite well. We will work on that tomorrow. BTW We "hacked" the browser back button so the user have to complete the flow but maybe it is worth to display some kind a message to explain why the back button is blocked? What do you think?

@mountiny
Copy link
Contributor Author

@adamgrzybowski great! thank you!

Yeah there should be an error on the modal shown. When you click outside of the modal now, we show it:
image

Can we show that too in this case?

@filip-solecki
Copy link
Contributor

@mountiny I don't know if this is proper message cause user can select something and then click back button. We cannot easily check if user select something. What do you think about some generic message like: Finish onboarding so you can use the app or similar?

@adamgrzybowski
Copy link
Contributor

@mountiny I haven't found a fix for the broken routes today. I will continue on Monday.

@trjExpensify trjExpensify moved this to Release 2: Summer 2024 (Aug) in [#whatsnext] #wave-collect Jul 1, 2024
Copy link

melvin-bot bot commented Jul 1, 2024

@anmurali, @mountiny, @adamgrzybowski, @allgandalf Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Jul 1, 2024
@allgandalf
Copy link
Contributor

This is been continuously worked on, Melvin!

@mountiny mountiny added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor and removed Reviewing Has a PR in review Weekly KSv2 External Added to denote the issue can be worked on by a contributor labels Aug 8, 2024
@melvin-bot melvin-bot bot changed the title [HOLD payment 2024-08-09] [CRITICAL] [Guided setup] The onboarding flow can be dismissed by navigating to different route [$250] [HOLD payment 2024-08-09] [CRITICAL] [Guided setup] The onboarding flow can be dismissed by navigating to different route Aug 8, 2024
Copy link

melvin-bot bot commented Aug 8, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 8, 2024
Copy link

melvin-bot bot commented Aug 8, 2024

Current assignee @allgandalf is eligible for the External assigner, not assigning anyone new.

@mountiny
Copy link
Contributor Author

mountiny commented Aug 8, 2024

This is ready for payment $250 to @allgandalf

@melvin-bot melvin-bot bot added the Overdue label Aug 12, 2024
@allgandalf
Copy link
Contributor

Can you pay this one @anmurali , thanks :)

@melvin-bot melvin-bot bot removed the Overdue label Aug 12, 2024
@mountiny mountiny removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 12, 2024
@melvin-bot melvin-bot bot added Overdue Daily KSv2 and removed Daily KSv2 Overdue labels Aug 14, 2024
@allgandalf
Copy link
Contributor

No checklist here, this was a cleanup from Guided setup

@melvin-bot melvin-bot bot added the Overdue label Aug 19, 2024
Copy link

melvin-bot bot commented Aug 20, 2024

@anmurali, @mountiny, @adamgrzybowski, @allgandalf Huh... This is 4 days overdue. Who can take care of this?

@mountiny
Copy link
Contributor Author

@anmurali ready to be paid

Copy link

melvin-bot bot commented Aug 22, 2024

@anmurali, @mountiny, @adamgrzybowski, @allgandalf 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

Copy link

melvin-bot bot commented Aug 26, 2024

@anmurali, @mountiny, @adamgrzybowski, @allgandalf 10 days overdue. I'm getting more depressed than Marvin.

@anmurali
Copy link

@melvin-bot melvin-bot bot removed the Overdue label Aug 28, 2024
@anmurali
Copy link

Paid.

@github-project-automation github-project-automation bot moved this from Release 2: Summer 2024 (Aug) to Done in [#whatsnext] #wave-collect Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Status: Done
Development

No branches or pull requests

6 participants