Update signup goals page skip to dashboard set site_intent to build #72883

agrullon95 · 2023-02-01T17:56:19Z

Testing/Review instructions

Test: short
Review: short

Proposed Changes

Currently, users that click the Skip to dashboard link on the top right-hand corner of the Goals onboarding step will be navigated to the home screen with the following site options:

site_intent = ""
launchpad_screen = false

The changes in this PR will set the site_intent site option to build if the user clicks Skip to dashboard on the Goals page. This will also enable the launchpad and set the launchpad_screen site option to full before the user is navigated to the home screen.

P2 post to give heads up: paYE8P-2yP-p2

Goals step:

Testing Instructions

Checkout this branch
Navigate to the onboarding flow (Click < Switch Site on the Calypso sidebar and then click Add a new site)
Create your free site and you should land on the Goals step.
Do not select a goal and click Skip to dashboard. The site_intent and launchpad_screen values should be updated. The home screen should redirect you to the launchpad screen (There may be a bug that requires multiple refreshes before you redirected - )
Create a new site and test another goal such as Write & Publish to verify the correct site_intent value is set

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

Related to #72870

github-actions · 2023-02-01T17:56:52Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-59144

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-59144&env=jetpack

matticbot · 2023-02-01T17:57:31Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~56 bytes added 📈 [gzipped])

name                 parsed_size           gzip_size
entry-stepper              +71 B  (+0.0%)      +39 B  (+0.0%)
entry-gutenboarding        +71 B  (+0.0%)      +10 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~143 bytes added 📈 [gzipped])

name             parsed_size           gzip_size
site-setup-flow       +125 B  (+0.0%)      +31 B  (+0.0%)
sites-dashboard        +71 B  (+0.0%)      +46 B  (+0.0%)
signup                 +71 B  (+0.0%)      +17 B  (+0.0%)
settings               +71 B  (+0.0%)      +46 B  (+0.0%)
jetpack-connect        +71 B  (+0.0%)      +32 B  (+0.0%)
hosting                +71 B  (+0.0%)      +46 B  (+0.0%)
home                   +71 B  (+0.0%)      +46 B  (+0.0%)
accept-invite          +71 B  (+0.0%)      +17 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~46 bytes added 📈 [gzipped])

name               parsed_size           gzip_size
async-load-design        +71 B  (+0.0%)      +46 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

sharpirate · 2023-02-01T18:17:41Z

Testing

Go to http://calypso.localhost:3000/start
On the Goals page click on "Skip to dashboard"
On My Home refresh we are redirected to Launchpad which indicates the site options were updated successfully.
Tested with the Write & Publish flow continues to work when selecting Goals.

jeyip · 2023-02-01T18:22:42Z

Testing

Requirements

Do not select a goal and click Skip to dashboard. The site_intent and launchpad_screen values should be updated. The home screen should redirect you to the launchpad screen
Create a new site and test another goal such as Write & Publish to verify the correct site_intent value is set
Could complete Launchpad tasks and launch site as expected
Launchpad iframe overlay, url copy to clipboard button, customize pill, and go to admin button continued to work as expected

Browsers

Chrome

Addison-Stavlo

This tests well on my end, minus the other issue tracked elsewhere that shouldnt block this PR. #72872

Addison-Stavlo · 2023-02-01T18:26:39Z

client/landing/stepper/declarative-flow/site-setup-flow.ts

+					// If user clicks "Skip to Dashboard" on goals step,
+					// Default site_intent to "build"
+					const updatedSiteIntent = intent ? intent : SiteIntent.Build;


Note this isnt specifically tied to this dashboard button on the goals step, so im curious if there are any other areas this change could potentially bleed into?

That said, testing around and escaping out of other flows early I see the intent the same as expected. So it seems to be looking good in my testing.

jeyip · 2023-02-01T18:28:46Z

client/landing/stepper/declarative-flow/site-setup-flow.ts

+
+					// If user clicks "Skip to Dashboard" on goals step,
+					// Default site_intent to "build"
+					const updatedSiteIntent = intent ? intent : SiteIntent.Build;


Since we're setting a default value for any flow with some falsey value for intent, would we inadvertently enable launchpad for situations where it's undesired?

I can't think of anything off the top of my head, but I suppose, if we wanted to be safer, we could only set default values for general onboarding

jeyip

I left one comment. +1 from me when we decide how we want to approach the concern

… available

agrullon95 · 2023-02-02T17:16:11Z

client/landing/stepper/declarative-flow/site-setup-flow.ts

@@ -520,6 +523,7 @@ const siteSetupFlow: Flow = {

 				case 'goals':
 					// Skip to dashboard must have been pressed
+					setIntent( SiteIntent.Build );


Sets site_intent to build in redux store if user clicks "Skip to dashboard"

Im not understanding the need for the other changes, but this seems like the main thing we need here?

Yes this is the main change, but I needed to add the other changes in order to get the latest site_intent value.

agrullon95 · 2023-02-02T17:17:17Z

client/landing/stepper/declarative-flow/site-setup-flow.ts

@@ -191,7 +194,7 @@ const siteSetupFlow: Flow = {
 			navigate( 'processing' );

 			// Clean-up the store so that if onboard for new site will be launched it will be launched with no preselected values
-			resetOnboardStoreWithSkipFlags( [ 'skipPendingAction' ] );
+			resetOnboardStoreWithSkipFlags( [ 'skipPendingAction', 'skipIntent' ] );


Added skipIntent flag to prevent the site intent from being deleted before its used in the setPendingAction function above.

I dont see this reset being called in many other places. So it seems like we are adding skipIntent to the main case here?

Yes im adding it here to ensure Im still able to retrieve the site intent value from redux inside the exitFlow pendingActions code.

Without adding this and modifying redux, I noticed that site intent value was always an empty string when I used the getIntent selector.

agrullon95 · 2023-02-02T17:18:00Z

packages/data-stores/src/onboard/reducer.ts

@@ -294,6 +294,9 @@ const intent: Reducer< string, OnboardAction > = ( state = '', action ) => {
 	if ( action.type === 'SET_INTENT' ) {
 		return action.intent;
 	}
+	if ( action.type === 'RESET_ONBOARD_STORE' && action?.skipFlags?.includes( 'skipIntent' ) ) {


If we reset the onboard store and include the skipIntent flag, then we don't want to delete the site_intent state value.

sharpirate · 2023-02-02T17:36:18Z

✅ Still tests well after the changes

Addison-Stavlo · 2023-02-03T18:44:33Z

client/landing/stepper/declarative-flow/site-setup-flow.ts

+
+					const siteIntent = getIntent();
 					const siteId = site?.ID;
 					const pendingActions = [
-						setIntentOnSite( siteSlug, intent ),
+						setIntentOnSite( siteSlug, siteIntent ),


why cant we use the regular intent here (and below)? Is there a reason we need to have both:

const intent = useSelect( ( select ) => select( ONBOARD_STORE ).getIntent() ); const { getIntent } = useSelect( ( select ) => select( ONBOARD_STORE ) );

Im not able to get the correct value from using the existing intent variable. It appears as an empty string.

I was going to ask the same question. Any thoughts on why? I assume it may a JS timing issue related to how and when pendingAction is invoked? Like in one case, you're getting the value of intent NOW as you set up the function (essentially acts as a closure and stores the current value), and in the other, your fetching site intent later, when the method itself is fired.

I will say it seems odd in this file to be fetching intent here twice via two different methods, in two different places.

I think one other thing I'm puzzle by here is that before this PR, we WERE using the first/original intent value. Does that mean that code was not working as expected?

I'm a bit concerned about whether this change (since it clearly does change the behavior and possible value of intent) will have unexpected impact on other existing flows beyond what we're wanting to do just for ours.

Reviewed this @agrullon95 and just did some extensive testing of other scenarios and flows to confirm I'm not seeing any issues.

Addison-Stavlo · 2023-02-03T18:49:36Z

client/landing/stepper/declarative-flow/site-setup-flow.ts

@@ -191,7 +194,7 @@ const siteSetupFlow: Flow = {
 			navigate( 'processing' );

 			// Clean-up the store so that if onboard for new site will be launched it will be launched with no preselected values
-			resetOnboardStoreWithSkipFlags( [ 'skipPendingAction' ] );
+			resetOnboardStoreWithSkipFlags( [ 'skipPendingAction', 'skipIntent' ] );


I dont see this reset being called in many other places. So it seems like we are adding skipIntent to the main case here?

Addison-Stavlo

Behavior tests well on my end 👍

To be safe and communicative here, would you mind writing a P2 about this and crossposting for dotcom and related teams? essentially that we are making this "skip" from this goals page default to build, similar to how "other" flow defaults to build? Mostly as a heads up, but also to double check for any concerns with this we may have missed?

…tent

edanzer

This tests as expected. I have some concerns that these code changes could plausibly impact how intent is set in a range of circumstances. I reviewed with @agrullon95 and I tested both the intended scenario (Skip to Dashboard) but also a wide mix of other flows and scenarios, and everything tests as expected.

In another comment @Addison-Stavlo mentioned maybe posting about this. Not sure if it's worth flagging one of the people who worked on the site-setup-flow file to confirm they're good with it too?

All that said, given all the checks and testing, I'm also approving now.

agrullon95 · 2023-03-02T20:55:27Z

@edanzer - I added a link to the P2 in the description where I gave other teams a heads-up about these changes

…tent

…et site_intent to build (#72883)" This reverts commit ee1cfb4.

…et site_intent to build (#72883)" (#74053) This reverts commit ee1cfb4.

Update signup goals page skip to dashboard set intent to build

6f5e82f

agrullon95 requested a review from a team February 1, 2023 17:56

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 1, 2023

agrullon95 self-assigned this Feb 1, 2023

sharpirate approved these changes Feb 1, 2023

View reviewed changes

Addison-Stavlo approved these changes Feb 1, 2023

View reviewed changes

jeyip reviewed Feb 1, 2023

View reviewed changes

agrullon95 added 3 commits February 2, 2023 11:41

Undo site intent default to build changes in exitFlow

31bb1c4

Set intent to build if user skips to dashboard on goals page

baf7a22

Update intent reducer to prevent state deletion if skipIntent flag is…

83e448d

… available

agrullon95 commented Feb 2, 2023

View reviewed changes

Addison-Stavlo reviewed Feb 3, 2023

View reviewed changes

Merge branch 'trunk' into add/onboarding-signup-goals-default-site-in…

90e58b9

…tent

edanzer approved these changes Mar 1, 2023

View reviewed changes

Merge branch 'trunk' into add/onboarding-signup-goals-default-site-in…

6150c64

…tent

agrullon95 merged commit ee1cfb4 into trunk Mar 3, 2023

agrullon95 deleted the add/onboarding-signup-goals-default-site-intent branch March 3, 2023 14:44

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 3, 2023

agrullon95 added a commit that referenced this pull request Mar 3, 2023

Revert "Onboarding flow: Update signup goals page skip to dashboard s…

e702a6f

…et site_intent to build (#72883)" This reverts commit ee1cfb4.

agrullon95 mentioned this pull request Mar 3, 2023

Revert "Update signup goals page skip to dashboard set site_intent to build" #74053

Merged

agrullon95 added a commit that referenced this pull request Mar 3, 2023

Revert "Onboarding flow: Update signup goals page skip to dashboard s…

f2f0bc4

…et site_intent to build (#72883)" (#74053) This reverts commit ee1cfb4.

agrullon95 restored the add/onboarding-signup-goals-default-site-intent branch March 3, 2023 16:21

jeyip mentioned this pull request Mar 5, 2023

Launchpad: add onboarding signup goals default site intent v3 #74082

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update signup goals page skip to dashboard set site_intent to build #72883

Update signup goals page skip to dashboard set site_intent to build #72883

agrullon95 commented Feb 1, 2023 •

edited

Loading

github-actions bot commented Feb 1, 2023 •

edited by matticbot

Loading

matticbot commented Feb 1, 2023 •

edited

Loading

sharpirate commented Feb 1, 2023

jeyip commented Feb 1, 2023 •

edited

Loading

Addison-Stavlo left a comment

Addison-Stavlo Feb 1, 2023

jeyip Feb 1, 2023 •

edited

Loading

jeyip left a comment

agrullon95 Feb 2, 2023 •

edited

Loading

Addison-Stavlo Feb 3, 2023

agrullon95 Feb 3, 2023

agrullon95 Feb 2, 2023

Addison-Stavlo Feb 3, 2023

agrullon95 Feb 3, 2023

agrullon95 Feb 2, 2023 •

edited

Loading

sharpirate commented Feb 2, 2023

Addison-Stavlo Feb 3, 2023

agrullon95 Feb 3, 2023 •

edited

Loading

edanzer Mar 1, 2023

edanzer Mar 1, 2023

edanzer Mar 1, 2023

edanzer Mar 1, 2023

Addison-Stavlo Feb 3, 2023

Addison-Stavlo left a comment

edanzer left a comment

agrullon95 commented Mar 2, 2023

Update signup goals page skip to dashboard set site_intent to build #72883

Update signup goals page skip to dashboard set site_intent to build #72883

Conversation

agrullon95 commented Feb 1, 2023 • edited Loading

Testing/Review instructions

Proposed Changes

Testing Instructions

Pre-merge Checklist

github-actions bot commented Feb 1, 2023 • edited by matticbot Loading

matticbot commented Feb 1, 2023 • edited Loading

sharpirate commented Feb 1, 2023

Testing

jeyip commented Feb 1, 2023 • edited Loading

Testing

Requirements

Browsers

Addison-Stavlo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeyip Feb 1, 2023 • edited Loading

Choose a reason for hiding this comment

jeyip left a comment

Choose a reason for hiding this comment

agrullon95 Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrullon95 Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

sharpirate commented Feb 2, 2023

Choose a reason for hiding this comment

agrullon95 Feb 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Addison-Stavlo left a comment

Choose a reason for hiding this comment

edanzer left a comment

Choose a reason for hiding this comment

agrullon95 commented Mar 2, 2023

agrullon95 commented Feb 1, 2023 •

edited

Loading

github-actions bot commented Feb 1, 2023 •

edited by matticbot

Loading

matticbot commented Feb 1, 2023 •

edited

Loading

jeyip commented Feb 1, 2023 •

edited

Loading

jeyip Feb 1, 2023 •

edited

Loading

agrullon95 Feb 2, 2023 •

edited

Loading

agrullon95 Feb 2, 2023 •

edited

Loading

agrullon95 Feb 3, 2023 •

edited

Loading