From 2836b87de3e0247fb565a96950ca55f080ca8a73 Mon Sep 17 00:00:00 2001 From: Philip Jackson Date: Thu, 28 Nov 2024 20:54:21 +1300 Subject: [PATCH 1/3] Move goals step to front of onboarding if onboarding/goals-first flag --- .../declarative-flow/internals/global.scss | 1 + .../stepper/declarative-flow/onboarding.ts | 51 +++++++++++++++++-- .../declarative-flow/site-setup-flow.ts | 21 +++++++- .../declarative-flow/test/site-setup-flow.tsx | 7 +-- config/development.json | 1 + 5 files changed, 74 insertions(+), 7 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/global.scss b/client/landing/stepper/declarative-flow/internals/global.scss index d75fc2068d4b84..bfd36f391dbbbd 100644 --- a/client/landing/stepper/declarative-flow/internals/global.scss +++ b/client/landing/stepper/declarative-flow/internals/global.scss @@ -73,6 +73,7 @@ button { .new-hosted-site, .import-hosted-site, .site-setup, +.onboarding.goals, .site-migration, .migration, .migration-signup, diff --git a/client/landing/stepper/declarative-flow/onboarding.ts b/client/landing/stepper/declarative-flow/onboarding.ts index f0cb1a221868a9..e7688e8ded347b 100644 --- a/client/landing/stepper/declarative-flow/onboarding.ts +++ b/client/landing/stepper/declarative-flow/onboarding.ts @@ -1,4 +1,5 @@ -import { OnboardSelect } from '@automattic/data-stores'; +import { isEnabled } from '@automattic/calypso-config'; +import { OnboardSelect, Onboard } from '@automattic/data-stores'; import { ONBOARDING_FLOW } from '@automattic/onboarding'; import { useDispatch, useSelect } from '@wordpress/data'; import { addQueryArgs, getQueryArg, getQueryArgs, removeQueryArgs } from '@wordpress/url'; @@ -13,8 +14,11 @@ import { STEPPER_TRACKS_EVENT_STEP_NAV_SUBMIT } from '../constants'; import { ONBOARD_STORE } from '../stores'; import { stepsWithRequiredLogin } from '../utils/steps-with-required-login'; import { recordStepNavigation } from './internals/analytics/record-step-navigation'; +import { STEPS } from './internals/steps'; import { Flow, ProvidedDependencies } from './internals/types'; +const SiteIntent = Onboard.SiteIntent; + const clearUseMyDomainsQueryParams = ( currentStepSlug: string | undefined ) => { const isDomainsStep = currentStepSlug === 'domains'; const isPlansStepWithQuery = @@ -27,12 +31,22 @@ const clearUseMyDomainsQueryParams = ( currentStepSlug: string | undefined ) => } }; +const useGoalsFirstExperiment = (): [ boolean, boolean ] => { + // Currently loading isn't required because we're using a feature flag, but in the future + // we may need to take account of loading time. + const isLoading = false; + + return [ isLoading, isEnabled( 'onboarding/goals-first' ) ]; +}; + const onboarding: Flow = { name: ONBOARDING_FLOW, isSignupFlow: true, __experimentalUseBuiltinAuth: true, useSteps() { - return stepsWithRequiredLogin( [ + const [ , isGoalsAtFrontExperiment ] = useGoalsFirstExperiment(); + + const steps = stepsWithRequiredLogin( [ { slug: 'domains', asyncComponent: () => import( './internals/steps-repository/unified-domains' ), @@ -54,6 +68,13 @@ const onboarding: Flow = { asyncComponent: () => import( './internals/steps-repository/processing-step' ), }, ] ); + + if ( isGoalsAtFrontExperiment ) { + // Note that this step is not wrapped in `stepsWithRequiredLogin` + steps.unshift( STEPS.GOALS ); + } + + return steps; }, useStepNavigation( currentStepSlug, navigate ) { @@ -84,6 +105,29 @@ const onboarding: Flow = { const submit = async ( providedDependencies: ProvidedDependencies = {} ) => { switch ( currentStepSlug ) { + case 'goals': { + const { intent, skip } = providedDependencies; + + if ( skip ) { + // TODO Implement skipping to dashboard + return; + } + + switch ( intent ) { + case SiteIntent.Import: + // TODO Implement exit to site migration + return; + + case SiteIntent.DIFM: + // TODO Implement exit to DIFM + return; + + default: { + return navigate( 'domains' ); + } + } + } + case 'domains': setSiteUrl( providedDependencies.siteUrl ); setDomain( providedDependencies.suggestion ); @@ -164,8 +208,9 @@ const onboarding: Flow = { case 'create-site': return navigate( 'processing', undefined, true ); case 'processing': { - const destination = addQueryArgs( '/setup/site-setup/goals', { + const destination = addQueryArgs( '/setup/site-setup', { siteSlug: providedDependencies.siteSlug, + ...( isEnabled( 'onboarding/goals-first' ) && { flags: 'onboarding/goals-first' } ), } ); persistSignupDestination( destination ); setSignupCompleteFlowName( flowName ); diff --git a/client/landing/stepper/declarative-flow/site-setup-flow.ts b/client/landing/stepper/declarative-flow/site-setup-flow.ts index 4a8569c4706b50..3da06037e939f0 100644 --- a/client/landing/stepper/declarative-flow/site-setup-flow.ts +++ b/client/landing/stepper/declarative-flow/site-setup-flow.ts @@ -1,3 +1,4 @@ +import { isEnabled } from '@automattic/calypso-config'; import { Onboard } from '@automattic/data-stores'; import { Design, isAssemblerDesign, isAssemblerSupported } from '@automattic/design-picker'; import { MIGRATION_FLOW } from '@automattic/onboarding'; @@ -47,6 +48,14 @@ function isLaunchpadIntent( intent: string ) { return intent === SiteIntent.Write || intent === SiteIntent.Build; } +const useGoalsFirstExperiment = (): [ boolean, boolean ] => { + // Currently loading isn't required because we're using a feature flag, but in the future + // we may need to take account of loading time. + const isLoading = false; + + return [ isLoading, isEnabled( 'onboarding/goals-first' ) ]; +}; + const siteSetupFlow: Flow = { name: 'site-setup', isSignupFlow: false, @@ -66,7 +75,9 @@ const siteSetupFlow: Flow = { }, useSteps() { - return [ + const [ , isGoalsAtFrontExperiment ] = useGoalsFirstExperiment(); + + const steps = [ STEPS.GOALS, STEPS.INTENT, STEPS.OPTIONS, @@ -94,6 +105,14 @@ const siteSetupFlow: Flow = { STEPS.ERROR, STEPS.DIFM_STARTING_POINT, ]; + + if ( isGoalsAtFrontExperiment ) { + // The user has already seen the goals step in the `onboarding` flow + // TODO Ensure that DESIGN_CHOICES is at the front if the user is Big Sky eligible + steps.splice( 0, 4 ); + } + + return steps; }, useStepNavigation( currentStep, navigate ) { const isGoalsHoldout = useIsGoalsHoldout( currentStep ); diff --git a/client/landing/stepper/declarative-flow/test/site-setup-flow.tsx b/client/landing/stepper/declarative-flow/test/site-setup-flow.tsx index e5692c96b9d8f6..fce970aeaa5c88 100644 --- a/client/landing/stepper/declarative-flow/test/site-setup-flow.tsx +++ b/client/landing/stepper/declarative-flow/test/site-setup-flow.tsx @@ -1,6 +1,7 @@ /** * @jest-environment jsdom */ +import { renderHook } from '@testing-library/react'; import { STEPS } from '../internals/steps'; import siteSetupFlow from '../site-setup-flow'; import { getFlowLocation, renderFlow } from './helpers'; @@ -27,9 +28,9 @@ describe( 'Site Setup Flow', () => { * It's totally fine to change this test if the flow changes. But please make sure to update and test the site-setup-wg accordingly. */ describe( 'First steps should be goals and intent capture', () => { - const steps = siteSetupFlow.useSteps(); - const firstStep = steps[ 0 ]; - const secondStep = steps[ 1 ]; + const { result } = renderHook( () => siteSetupFlow.useSteps() ); + const firstStep = result.current[ 0 ]; + const secondStep = result.current[ 1 ]; it( 'should be goals', () => { expect( firstStep.slug ).toBe( STEPS.GOALS.slug ); diff --git a/config/development.json b/config/development.json index 223bcf1b483565..0127172d8e2c17 100644 --- a/config/development.json +++ b/config/development.json @@ -170,6 +170,7 @@ "onboarding/trail-map-feature-grid-structure": false, "onboarding/trail-map-feature-grid": false, "onboarding/user-on-stepper-hosting": true, + "onboarding/goals-first": false, "p2/p2-plus": true, "p2-enabled": false, "page/export": true, From f6ecbb1e333566b509e9c2faeafc9ab7d13d9487 Mon Sep 17 00:00:00 2001 From: Philip Jackson Date: Fri, 29 Nov 2024 12:37:07 +1300 Subject: [PATCH 2/3] Ensure experiment assignment loads before showing first step --- .../helpers/use-goals-first-experiment.ts | 32 +++++++++++++++++++ .../stepper/declarative-flow/onboarding.ts | 21 ++++++------ .../declarative-flow/site-setup-flow.ts | 18 +++++------ 3 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 client/landing/stepper/declarative-flow/helpers/use-goals-first-experiment.ts diff --git a/client/landing/stepper/declarative-flow/helpers/use-goals-first-experiment.ts b/client/landing/stepper/declarative-flow/helpers/use-goals-first-experiment.ts new file mode 100644 index 00000000000000..d8fcdbe31c6fc1 --- /dev/null +++ b/client/landing/stepper/declarative-flow/helpers/use-goals-first-experiment.ts @@ -0,0 +1,32 @@ +import { isEnabled } from '@automattic/calypso-config'; +import { useEffect, useState } from 'react'; + +/** + * Check whether the user should have the "goals first" onboarding experience. + * Returns [ isLoading, isGoalsAtFrontExperiment ] + * + * The experience is currently gated behind a feature flag which is loaded immediately, + * but we want to code as if loading time might be involved. So this hook has a fake + * timer. + * Note: It's important that there be no timer in the production experience + * i.e. when the feature flag is disabled. + */ +export function useGoalsFirstExperiment(): [ boolean, boolean ] { + const [ isLoading, setIsLoading ] = useState( true ); + useEffect( () => { + if ( ! isEnabled( 'onboarding/goals-first' ) ) { + return; + } + + const id = setTimeout( () => setIsLoading( false ), 700 ); + return () => { + clearTimeout( id ); + }; + }, [] ); + + if ( ! isEnabled( 'onboarding/goals-first' ) ) { + return [ false, false ]; + } + + return [ isLoading, true ]; +} diff --git a/client/landing/stepper/declarative-flow/onboarding.ts b/client/landing/stepper/declarative-flow/onboarding.ts index e7688e8ded347b..e7cb9dc9c8a6f6 100644 --- a/client/landing/stepper/declarative-flow/onboarding.ts +++ b/client/landing/stepper/declarative-flow/onboarding.ts @@ -13,9 +13,10 @@ import { import { STEPPER_TRACKS_EVENT_STEP_NAV_SUBMIT } from '../constants'; import { ONBOARD_STORE } from '../stores'; import { stepsWithRequiredLogin } from '../utils/steps-with-required-login'; +import { useGoalsFirstExperiment } from './helpers/use-goals-first-experiment'; import { recordStepNavigation } from './internals/analytics/record-step-navigation'; import { STEPS } from './internals/steps'; -import { Flow, ProvidedDependencies } from './internals/types'; +import { AssertConditionState, Flow, ProvidedDependencies } from './internals/types'; const SiteIntent = Onboard.SiteIntent; @@ -31,19 +32,12 @@ const clearUseMyDomainsQueryParams = ( currentStepSlug: string | undefined ) => } }; -const useGoalsFirstExperiment = (): [ boolean, boolean ] => { - // Currently loading isn't required because we're using a feature flag, but in the future - // we may need to take account of loading time. - const isLoading = false; - - return [ isLoading, isEnabled( 'onboarding/goals-first' ) ]; -}; - const onboarding: Flow = { name: ONBOARDING_FLOW, isSignupFlow: true, __experimentalUseBuiltinAuth: true, useSteps() { + // We have already checked the value has loaded in useAssertConditions const [ , isGoalsAtFrontExperiment ] = useGoalsFirstExperiment(); const steps = stepsWithRequiredLogin( [ @@ -70,7 +64,7 @@ const onboarding: Flow = { ] ); if ( isGoalsAtFrontExperiment ) { - // Note that this step is not wrapped in `stepsWithRequiredLogin` + // This step is not wrapped in `stepsWithRequiredLogin` steps.unshift( STEPS.GOALS ); } @@ -269,6 +263,13 @@ const onboarding: Flow = { return { goBack, submit }; }, + useAssertConditions() { + const [ isLoading ] = useGoalsFirstExperiment(); + + return { + state: isLoading ? AssertConditionState.CHECKING : AssertConditionState.SUCCESS, + }; + }, }; export default onboarding; diff --git a/client/landing/stepper/declarative-flow/site-setup-flow.ts b/client/landing/stepper/declarative-flow/site-setup-flow.ts index 3da06037e939f0..a1fd3d2c4fc830 100644 --- a/client/landing/stepper/declarative-flow/site-setup-flow.ts +++ b/client/landing/stepper/declarative-flow/site-setup-flow.ts @@ -1,4 +1,3 @@ -import { isEnabled } from '@automattic/calypso-config'; import { Onboard } from '@automattic/data-stores'; import { Design, isAssemblerDesign, isAssemblerSupported } from '@automattic/design-picker'; import { MIGRATION_FLOW } from '@automattic/onboarding'; @@ -21,6 +20,7 @@ import { useSiteData } from '../hooks/use-site-data'; import { useCanUserManageOptions } from '../hooks/use-user-can-manage-options'; import { ONBOARD_STORE, SITE_STORE, USER_STORE, STEPPER_INTERNAL_STORE } from '../stores'; import { shouldRedirectToSiteMigration } from './helpers'; +import { useGoalsFirstExperiment } from './helpers/use-goals-first-experiment'; import { useLaunchpadDecider } from './internals/hooks/use-launchpad-decider'; import { STEPS } from './internals/steps'; import { redirect } from './internals/steps-repository/import/util'; @@ -48,14 +48,6 @@ function isLaunchpadIntent( intent: string ) { return intent === SiteIntent.Write || intent === SiteIntent.Build; } -const useGoalsFirstExperiment = (): [ boolean, boolean ] => { - // Currently loading isn't required because we're using a feature flag, but in the future - // we may need to take account of loading time. - const isLoading = false; - - return [ isLoading, isEnabled( 'onboarding/goals-first' ) ]; -}; - const siteSetupFlow: Flow = { name: 'site-setup', isSignupFlow: false, @@ -75,6 +67,7 @@ const siteSetupFlow: Flow = { }, useSteps() { + // We have already checked the value has loaded in useAssertConditions const [ , isGoalsAtFrontExperiment ] = useGoalsFirstExperiment(); const steps = [ @@ -738,6 +731,13 @@ const siteSetupFlow: Flow = { }; } + const [ isLoadingGoalsFirstExp ] = useGoalsFirstExperiment(); + if ( isLoadingGoalsFirstExp ) { + result = { + state: AssertConditionState.CHECKING, + }; + } + return result; }, }; From af0199d8782ac815b30eedb38fe781594657f786 Mon Sep 17 00:00:00 2001 From: Philip Jackson Date: Sat, 7 Dec 2024 10:20:52 -0500 Subject: [PATCH 3/3] Enable on staging --- config/development.json | 2 +- config/stage.json | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/config/development.json b/config/development.json index 0127172d8e2c17..5fd5523795259f 100644 --- a/config/development.json +++ b/config/development.json @@ -170,7 +170,7 @@ "onboarding/trail-map-feature-grid-structure": false, "onboarding/trail-map-feature-grid": false, "onboarding/user-on-stepper-hosting": true, - "onboarding/goals-first": false, + "onboarding/goals-first": true, "p2/p2-plus": true, "p2-enabled": false, "page/export": true, diff --git a/config/stage.json b/config/stage.json index 9c7274294c7f48..87f92e6a6da089 100644 --- a/config/stage.json +++ b/config/stage.json @@ -138,6 +138,7 @@ "onboarding/trail-map-feature-grid-copy": false, "onboarding/trail-map-feature-grid-structure": false, "onboarding/trail-map-feature-grid": false, + "onboarding/goals-first": true, "p2/p2-plus": true, "p2-enabled": true, "page/export": true,