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

Prototype: Move goals step to front of onboarding if onboarding/goals-first flag #96866

Merged
merged 3 commits into from
Dec 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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 ];
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ button {
.new-hosted-site,
.import-hosted-site,
.site-setup,
.onboarding.goals,
.site-migration,
.migration,
.migration-signup,
Expand Down
54 changes: 50 additions & 4 deletions client/landing/stepper/declarative-flow/onboarding.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -12,8 +13,12 @@ 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 { Flow, ProvidedDependencies } from './internals/types';
import { STEPS } from './internals/steps';
import { AssertConditionState, Flow, ProvidedDependencies } from './internals/types';

const SiteIntent = Onboard.SiteIntent;

const clearUseMyDomainsQueryParams = ( currentStepSlug: string | undefined ) => {
const isDomainsStep = currentStepSlug === 'domains';
Expand All @@ -32,7 +37,10 @@ const onboarding: Flow = {
isSignupFlow: true,
__experimentalUseBuiltinAuth: true,
useSteps() {
return stepsWithRequiredLogin( [
// We have already checked the value has loaded in useAssertConditions
const [ , isGoalsAtFrontExperiment ] = useGoalsFirstExperiment();

const steps = stepsWithRequiredLogin( [
{
slug: 'domains',
asyncComponent: () => import( './internals/steps-repository/unified-domains' ),
Expand All @@ -54,6 +62,13 @@ const onboarding: Flow = {
asyncComponent: () => import( './internals/steps-repository/processing-step' ),
},
] );

if ( isGoalsAtFrontExperiment ) {
// This step is not wrapped in `stepsWithRequiredLogin`
steps.unshift( STEPS.GOALS );
}

return steps;
},

useStepNavigation( currentStepSlug, navigate ) {
Expand Down Expand Up @@ -84,6 +99,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 );
Expand Down Expand Up @@ -164,8 +202,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 );
Expand Down Expand Up @@ -224,6 +263,13 @@ const onboarding: Flow = {

return { goBack, submit };
},
useAssertConditions() {
const [ isLoading ] = useGoalsFirstExperiment();

return {
state: isLoading ? AssertConditionState.CHECKING : AssertConditionState.SUCCESS,
};
},
};

export default onboarding;
21 changes: 20 additions & 1 deletion client/landing/stepper/declarative-flow/site-setup-flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,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';
Expand Down Expand Up @@ -66,7 +67,10 @@ const siteSetupFlow: Flow = {
},

useSteps() {
return [
// We have already checked the value has loaded in useAssertConditions
const [ , isGoalsAtFrontExperiment ] = useGoalsFirstExperiment();

const steps = [
STEPS.GOALS,
STEPS.INTENT,
STEPS.OPTIONS,
Expand Down Expand Up @@ -94,6 +98,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 );
Expand Down Expand Up @@ -719,6 +731,13 @@ const siteSetupFlow: Flow = {
};
}

const [ isLoadingGoalsFirstExp ] = useGoalsFirstExperiment();
if ( isLoadingGoalsFirstExp ) {
result = {
state: AssertConditionState.CHECKING,
};
}

return result;
},
};
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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 );
Expand Down
1 change: 1 addition & 0 deletions config/development.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": true,
Copy link
Contributor

@arthur791004 arthur791004 Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm...it seems the default value should be false on both dev and staging since it is far from complete and it changed the behavior after creating a new site with proxy...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed #97219

"p2/p2-plus": true,
"p2-enabled": false,
"page/export": true,
Expand Down
1 change: 1 addition & 0 deletions config/stage.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading