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

Signup: resumeProgress will resume steps from different flows #34299

Closed
andrewserong opened this issue Jun 26, 2019 · 19 comments
Closed

Signup: resumeProgress will resume steps from different flows #34299

andrewserong opened this issue Jun 26, 2019 · 19 comments
Assignees
Labels
[Feature] Signup & Account Creation All screens and flows for making a new WordPress.com account. Frontend

Comments

@andrewserong
Copy link
Member

To best reproduce this issue, checkout this PR, with changes to the blog flow that skips the site-type step: #34298

Steps to reproduce

Check out the PR / branch update/blog-flow: #34298

  1. Go to http://calypso.localhost:3000/start/blog
  2. Proceed to the domains step
  3. Go to http://calypso.localhost:3000/start/
  4. Attempt to click Back to reach the site-type step. You currently can't get there, as the NavigationLink will take you back to the blog flow.

What I expected

The blog flow is typically reached via the WordPress.com homepage, and selecting the blog plan. However start can be reached from many more places, e.g. while logged in and going to create a new site, or clicking Get Started from the homepage.

I would have two expectations with the above issue:

  1. I would expect that if I clicked Start with Blogger, then go back to the homepage and click Start with Premium, that I would be starting the flow from scratch.
  2. I would expect that if I clicked Back from either of these flows, I would be going back through the steps of that particular flow, instead of switching to another one.

What happened instead

  1. Selecting the other flow after already completing steps in the blog flow, resumes my progress to the last step I was on in the blog flow, but I am now in a different flow. If using the above PR, it is not possible to click Back to get to the site-type step and change site types.

Browser / OS version

Chrome 75, macOS 10.14.5

Context / Source

Additional context and issues related to this:

A possible workaround for this particular issue could be to attempt to skip resumeProgress if the current flow is different to the flow of the last completed step?

@andrewserong andrewserong added [Feature] Signup & Account Creation All screens and flows for making a new WordPress.com account. Frontend labels Jun 26, 2019
@andrewserong andrewserong self-assigned this Jun 26, 2019
@adamziel
Copy link
Contributor

adamziel commented Jun 26, 2019

First - thank you for thoroughly exploring this behavior and writing a super clear description of it.

I didn't take a deep code dive before writing this comment, but it feels like this is the same (or a very similar issue) to the one @spen described here: #34211 (comment)

Personally I've always found this behavior annoying as by definition it redirects you to a different place than you expected. I wonder what's a canonical use case for this behavior. If there's none, or if the current one is not worth all the trouble - we could remove the entire guessing thing. CC @ramonjd @michaeldcain @fditrapani

@fditrapani
Copy link
Contributor

Personally I've always found this behavior annoying as by definition it redirects you to a different place than you expected.

That's definitely annoying and we don't want that. Is this a temporary issue that will be resolved once we merge all the flows?

I know we have logic that is supposed to bring you back to where you left off and that's fine if it sends you to the right place. I can't remember where this was documented, but we also discussed shortening the window which this code runs to something like an hour. So if you come back a day later than it takes you back to the beginning. Does that sound familiar @michaeldcain?

@adamziel
Copy link
Contributor

adamziel commented Jun 26, 2019

@fditrapani it doesn't seem like a crucial feature but it may have a real maintenance cost. What's your take on removing it?

@fditrapani
Copy link
Contributor

I would be open to it but need to understand the impact first:

  • What would happen if someone were to refresh their screen? and
  • Is this a temporary issue that will be resolved once we merge all the flows?

@spen spen assigned spen and unassigned spen Jun 26, 2019
@ramonjd
Copy link
Member

ramonjd commented Jun 27, 2019

  1. Go to http://calypso.localhost:3000/start/

Just to clarify, are we referring to the browser's back button in step 4, after having refreshed the page?

I'm taken to the site type step every time, which I guess is not what I expected either as the previous page in my mind's history is the site style step. ❓

One other thing I noticed: there is often no relationship between the navigation and browser back buttons.

We use the label 'Back', when we really mean "the last step in the last flow we can find, if we can find it".

Jun-27-2019 15-36-56

We go to great pains and maintain lots of code to keep track of fulfilled steps and the navigation back button's functionality, when I wonder if we should rather display a progress bar with navigable links to steps in the known flow, and let the browser's back and forward buttons do their jobs.

I can't say for certain, but I speculate it might help address the problem we're facing in this issue.

I wonder what's a canonical use case for this behavior. If there's none, or if the current one is not worth all the trouble - we could remove the entire guessing thing

Let's start with defining one if none exists. My vote is for having a UX-driven list of requirements for the flow navigation, one against which we can measure our current set up, and also confidently discard current code that does not meet those requirements.

For example: The back button (should we decide to keep it) should take the user back to the previous step in the current flow (not the previous page in the browser's history). Based on this definition, we might choose to update the Back button's label:

Screen Shot 2019-06-27 at 4 27 16 pm

Are we taking the user to the last-known, unfulfilled step when they return to start after a period of time? Let's tell them, and give them the option to start again at least.

I'm aware that all this is easier said than done. I'm happy to kickstart a conversation on our p2, and throw in some awful ideas to coax out better ones from the crowd.

So if you come back a day later than it takes you back to the beginning

👍 I like this option as an initial way to mitigate the experience.

@andrewserong
Copy link
Member Author

@ramonjd thanks for looking at this in detail!

Just to clarify, are we referring to the browser's back button in step 4, after having refreshed the page?

This was clicking on our back button / NavigationLink component, but it seems that Spen was having difficulty replicating the exact issue, too. Just in case, to see the exact issue I'm referring to, you'll need to be on the update/blog-flow branch, which skips the site-type step.

My vote is for having a UX-driven list of requirements for the flow navigation

Great idea. And a P2 discussion sounds like a good way for us to hone in on an approach... I really like the progress bar / navigable links idea, too to help orient users within the flow, and fallback to browser back/forward behaviour instead of maintaining our own.

@adamziel
Copy link
Contributor

cc @shaunandrews

@fditrapani
Copy link
Contributor

fditrapani commented Jun 27, 2019

Thanks for the thoughtful responses so far @andrewserong and @ramonjd. Let's keep the conversation on this github issue since we have built some context already and feel free to pull in anyone else you think needs to be part of it.

My vote is for having a UX-driven list of requirements for the flow navigation

The requirements are fairly simple. We have a back button, that replicates the browser back, to reassure people that nothing is going to blow up if they press our back button. It's there for psychological safety.

When you press back, it should go back to the previous screen you were on — seeing anything else would be confusing. Coincidentally, this should also align with the previous step in the flow that you're on because it's a sequential flow.

Let me know if you all have any additional questions about this.

Is this a temporary issue that will be resolved once we merge all the flows?

Back to this question. Are we encountering this issue because we're doing weird things by forking the flow? If this is the case, I wouldn't de-prioritize this issue in favour of merging the flows so we get back to a single flow.

@spen
Copy link
Contributor

spen commented Jun 27, 2019

I ended up being able to repro this issue by following along with @andrewserong's instructions, I've added the bolds, though I'm sure the issue can be reproduced without my additions:

I put in some logs...

// signup/main.jsx
render() {
  console.log( this.props.flowName );
  ...
}

...and found that when we manual go to http://calypso.localhost:3000/start/ in the above instructions, the flow name is onboarding - then clicking the back button switches the flow back to /blog/

The resumeProgress method, which I thought might be the culprit here looks at this.props.flowName and page.redirects to that flow (and the specific step) correctly...
Without clicking anything at all though, I did notice that hovering over the back button shows the start/blog/... link in the hyperlink.
From there I started looking at the file signup/navigation-link/index.jsx and noticed line 82 of this method:

getBackUrl() {
if ( this.props.direction !== 'back' ) {
return;
}
if ( this.props.backUrl ) {
return this.props.backUrl;
}
const previousStep = this.getPreviousStep();
const stepSectionName = get(
find( this.props.signupProgress, { stepName: previousStep.stepName } ),
'stepSectionName',
''
);
return getStepUrl(
previousStep.lastKnownFlow || this.props.flowName,
previousStep.stepName,
stepSectionName,
getLocaleSlug()
);
}

That looks like it's the root of our issue. We're saving the lastKnownflow as part of the step, then recalling it in some buried/nested file to generate our link.

Forgive me for basically just thinking out loud above here - I hope the process helps!

@ramonjd
Copy link
Member

ramonjd commented Jun 28, 2019

We have a back button, that replicates the browser back, to reassure people that nothing is going to blow up if they press our back button.

Thanks @fditrapani ! This is a great start from which to work.

At the moment our 'Back' button doesn't behave like the browser's in that, should you land on https://wordpress.com/start with some progress in the state, you're immediately whisked away to /site-style-with-preview for example.

Hitting the browser back will have a different outcome to hitting our 'Back' button, hence why I suggested explicitly labelling the back button with whatever it's going to do next.

We're saving the lastKnownflow as part of the step, then recalling it in some buried/nested file to generate our link.

I can provide some insight into this madness. lastKnownflow was a property we added to try to make sure we flipped back to the last known flow at the site type step.

The reason at the time was because, if you clicked e-commerce or business, you were taken to two different flows (e-commerce and/or onboarding-for-business).

Clicking back to the site type step kept you on those flows, and when you clicked on professional to use the onboarding flow it didn't work because the registered flow was still e-commerce or whatever.

lastKnownflow was meant to mitigate this by allowing us to switch back to the last known flow at a flow fork, which is kind of what the site type step is at the moment.

@spen
Copy link
Contributor

spen commented Jun 28, 2019

Clicking back to the site type step kept you on those flows, and when you clicked on professional to use the onboarding flow it didn't work because the registered flow was still e-commerce or whatever.

lastKnownflow was meant to mitigate this by allowing us to switch back to the last known flow at a flow fork, which is kind what the site type step is at the moment.

Totally makes sense! I think that now that we know the cause for that back button link forcing us to the old flow (before resuming progress on revisiting) we should be able to make use of that when it's needed (as you describe) but avoid it when we're revisiting from a different flow.

@michaeldcain
Copy link
Member

Is this a temporary issue that will be resolved once we merge all the flows?

@fditrapani brings up a good point here. Is this more related to flow forking or dependency pre-filling? Is there an event on the back button?

andrewserong added a commit that referenced this issue Jun 28, 2019
…om the same flow as the current flowName

This attempts to make `resumeProgress` less eager to resume the progress. In this change, progress should only resume if we're certain that the user is resuming the same flow that they were last on. Otherwise, they will start from the beginning of the flow again.

This is a WIP commit to try out an approach for dealing with issue #34299
@andrewserong
Copy link
Member Author

I think this issue is related to both flow forking, and to users who might start a sign up from one path (e.g. they select blogger from the pricing page), but then come back again later and select a different flow (e.g. they select Get Started from the wordpress.com).

@andrewserong
Copy link
Member Author

I have a very simple draft PR experimenting with matching the filtered steps from the progress state to ensure that the last known flow is the same as the current flow. From a bit of testing locally, it makes the resumeProgress a lot less eager to force you back to the same place.

#34356

This is just one possible approach to mitigate this particular issue, and I'll continue playing with it next week.

@shaunandrews
Copy link
Contributor

shaunandrews commented Jul 1, 2019

We have a back button, that replicates the browser back, to reassure people that nothing is going to blow up if they press our back button.

Agreed with @fditrapani here.

--

Based on this definition, we might choose to update the Back button's label:

Regardless of the behavior of the back button, I really like the idea of improving the label. @ramonjd's suggestion of "Back to {{step title here}}" might be a little too verbose. We could take a cue from iOS and just shorten the label to "{{step title here}", like this:

image

This could get tricky on smaller screens, where our horizontal space is limited. We could remove the label and/or adjust the layout to make room.

Another option is to remove the back button and list the steps available:

image

I like this option, as its more clear what's happening. But, if steps change, we'll need to do something to accommodate. That is, can we accurately list the future steps without knowing the answer to the current question?

This also presents some challenges on mobile. We could get creative and show the list in some sort of dropdown, or revert to a more standard back button.

@ramonjd
Copy link
Member

ramonjd commented Jul 1, 2019

Another option is to remove the back button and list the steps available:

+1

This is very clear, and tells the user what to expect both previous to and immediately after their current position in the flow.

When we switch flows, sometimes the flow steps will change. For example: select Online store at the site type step, and the step count will reduce from 6 to 3.

I can imagine that being confusing, but I'm just guessing.

We could take a cue from iOS and just shorten the label to "{{step title here}",

This I feel would be a great start, and achievable in a short space of time. 👍 I can add a task to our board if folks are okay with it.

@andrewserong
Copy link
Member Author

There's some great discussion happening on this issue! I really like the suggested updates to the Back button and the proposal for a list of sign up steps.

The original problem that I flagged in this issue (that resumeProgress was resuming steps from another flow) has now been resolved in: #34356

We obviously want to keep the discussion going as to other improvements. @ramonjd did you want to open up a new issue for the tasks we're picking up out of this one, then reference this issue, and we can close this one? (Otherwise, I'm happy to!)

@andrewserong andrewserong removed their assignment Jul 2, 2019
@ramonjd
Copy link
Member

ramonjd commented Jul 2, 2019

then reference this issue, and we can close this one? (Otherwise, I'm happy to!)

Go for it @andrewserong ! Much appreciated.

@andrewserong
Copy link
Member Author

Thanks everyone for your discussion here! Let's continue over in: #34390

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Signup & Account Creation All screens and flows for making a new WordPress.com account. Frontend
Projects
None yet
Development

No branches or pull requests

7 participants