A4A: Add "Next Steps" Onboarding Tours #89081

nateweller · 2024-04-01T13:56:18Z

Resolves https://github.com/Automattic/jetpack-genesis/issues/292

Proposed Changes

Adds the guided tour components to the Sites screen.
- The tour will be shown when the page's query string includes tour=sites-walkthrough or tour=add-new-site. These are linked in the Overview page's "Next Steps" card.
- You can click step by step through each popover.
- If you open a site details pane half way through a tour, the popover component will be hidden automatically. Closing the site will resume the tour.
- Uses React Context to provide data about the current tour to all GuidedTourStep components.
- Uses individual GuidedTourStep components to ensure each step is only rendered when it's anchor is available.

Testing Instructions

From the A4A overview page, click the "get familiar with the sites management dashboard" and "learn how to add new sites" steps to start each tour.
Verify you are redirected to the sites screen with a tour parameter in the query string.
Click through the tour and verify each step renders appropriately.
Verify skipping the tour dismisses all steps.
Verify a dismissed tour does not re-start on page refresh.

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-aUh-p2)?

Screenshots

matticbot · 2024-04-01T13:57:20Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-103127&env=a8c-for-agencies

matticbot · 2024-04-01T14:01:02Z

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

Sections (~5555 bytes added 📈 [gzipped])

name                           parsed_size           gzip_size
a8c-for-agencies-sites             +7001 B  (+0.5%)    +2635 B  (+0.7%)
a8c-for-agencies-overview          +4166 B  (+2.1%)    +1399 B  (+2.3%)
a8c-for-agencies-purchases         +4047 B  (+0.9%)    +1374 B  (+1.0%)
a8c-for-agencies-plugins           +4047 B  (+2.5%)    +1374 B  (+2.6%)
a8c-for-agencies-marketplace       +4047 B  (+0.8%)    +1374 B  (+0.8%)
a8c-for-agencies-landing           +4047 B  (+4.7%)    +1355 B  (+5.2%)
jetpack-cloud-agency-sites-v2       -173 B  (-0.0%)     +481 B  (+0.1%)

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

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.

yashwin

Good job on this, @nateweller!

It mostly looks good. I have left a few comments. Let me know what you think.

Few things to note:

Could you please rebase this PR with the latest trunk?
I also see the PR has 1300+ lines added. We suggest keeping it to maximum of 500 lines, limiting the scope of changes as tightly as makes sense, and breaking it into multiple PRs, as it becomes very hard to review everything. But it is okay for now as I have reviewed most of the things.

client/a8c-for-agencies/components/guided-tour-step/style.scss

yashwin · 2024-04-03T07:01:35Z

client/a8c-for-agencies/sections/sites/sites-dataviews/style.scss

+.dataviews-wrapper {
+	.dataviews-no-results,
+	.dataviews-loading {
+		padding-top: 1rem;


[Non-blocker] It is better to use logical properties everywhere on this file since we can support RTL locales in Calypso.

client/a8c-for-agencies/sections/sites/sites-dataviews/style.scss

nateweller · 2024-04-03T20:21:03Z

Thanks for the review @yashwin! I've rebased trunk and addressed the color variables. I've left the sites dataviews alone for now. Let me know what you think 👍

andrii-lysenko

~~Haven't tested it manually but the code looks good and it already had previous approvals! 👍~~
It was a comment for another PR.

Wrong pr

cleacos · 2024-04-03T23:26:18Z

There is a failing test: https://teamcity.a8c.com/buildConfiguration/calypso_calypso_WebApp_Run_All_Unit_Tests/12230574?expandBuildDeploymentsSection=false&hideTestsFromDependencies=false&hideProblemsFromDependencies=false&expandBuildProblemsSection=true&showLog=12230574_10582_532.10460.10581.10608&logView=flowAware

andrii-lysenko · 2024-04-04T14:27:05Z

client/a8c-for-agencies/components/layout/index.tsx

@@ -1,5 +1,7 @@
 import classNames from 'classnames';
 import React, { ReactNode } from 'react';
+import GuidedTour from 'calypso/a8c-for-agencies/components/guided-tour';


I think this variable is unused.

andrii-lysenko · 2024-04-04T14:36:47Z

client/a8c-for-agencies/components/guided-tour-step/index.tsx

+type Props = {
+	id: string;
+	tourId: TourId;
+	context?: LegacyRef< HTMLElement | null >;


There's a typescript error caused by this. I think we can use just HTMLElement | null here.

andrii-lysenko

Thank you for working on this. Overall, it looks like a solid solution.

There's a number of small issues with typescript build we need to address prior to be able to fully test it though. Please let me know if these looks reasonable. Thanks!

andrii-lysenko · 2024-04-04T14:38:20Z

client/a8c-for-agencies/sections/sites/site-preview-pane/index.tsx

+			</div>
+			<GuidedTourStep
+				id="sites-walkthrough-site-preview-tabs"
+				tourId="sitesWalkthrough"


I see typescript shows an error here. Should it be sites-walkthrough?

andrii-lysenko · 2024-04-04T14:41:42Z

client/a8c-for-agencies/sections/sites/sites-dataviews/index.tsx

+	);
+
+	// Legacy refs for guided tour popovers
+	const [ introRef, setIntroRef ] = useState< React.LegacyRef< HTMLElement | null > >();


Here we should be able to use simply as well

const [ introRef, setIntroRef ] = useState< HTMLElement | null >();

andrii-lysenko · 2024-04-04T14:42:58Z

client/a8c-for-agencies/sections/sites/sites-dataviews/index.tsx

+						</SiteSort>
+						<GuidedTourStep
+							id="sites-walkthrough-intro"
+							tourId="sitesWalkthrough"


Typescript is complaining here. Should it be tourId="sites-walkthrough"?

andrii-lysenko · 2024-04-04T14:44:25Z

client/a8c-for-agencies/sections/sites/sites-header-actions/index.tsx

+					}
+				/>
+			</div>
+			<GuidedTourStep id="add-new-site" tourId="addSiteStep1" context={ tourStepRef } />


Should tourId here be 'add-new-site'?

andrii-lysenko · 2024-04-04T14:46:56Z

client/a8c-for-agencies/components/guided-tour-step/index.tsx

+				</div>
+				<div className="guided-tour__popover-footer-right-content">
+					<>
+						{ ( ( ! currentStep.nextStepOnTargetClick &&


It's a bit hard to follow here. Maybe we could move it to a constant defined above?

andrii-lysenko · 2024-04-04T14:50:29Z

client/a8c-for-agencies/sections/overview/body/next-steps/index.tsx

-				dispatch(
-					savePreference( A4A_ONBOARDING_TOURS_PREFERENCE_NAME[ 'sitesWalkthrough' ], true )
-				);
+				resetTour( [ 'sitesWalkthrough' ] );


should it be sites-walkthrough?

matticbot · 2024-04-04T19:24:08Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug add/a4a/onboarding-tours on your sandbox.

andrii-lysenko

Great work 👍 Thank you for addressing the comments. Looks great now. Let's merge it and iterate if needed.

Some small comments, that we don't need type assertions but they are not blocking.

andrii-lysenko · 2024-04-04T23:02:00Z

client/a8c-for-agencies/components/guided-tour-step/index.tsx

@@ -71,13 +71,13 @@ export function GuidedTourStep( { id, tourId, context, hideSteps, className }: P
 	// Add a click event listener to the context element if the step requires it
 	useEffect( () => {
 		if ( currentStep && currentStep.nextStepOnTargetClick && context ) {
-			context.addEventListener( 'click', nextStep );
+			( context as HTMLElement ).addEventListener( 'click', nextStep );


I don't think it's necessary here. Should be ok just to use context.addEventListener?

andrii-lysenko · 2024-04-04T23:03:42Z

client/a8c-for-agencies/sections/sites/sites-dataviews/index.tsx

@@ -111,14 +111,14 @@ const SitesDataViews = ( {
 						<SiteSort isSortable={ true } columnKey="site">
 							<span
 								className="sites-dataview__site-header sites-dataview__site-header--sort"
-								ref={ ( ref ) => setIntroRef( ref as React.LegacyRef< HTMLElement | null > ) }
+								ref={ ( ref ) => setIntroRef( ref as HTMLElement | null ) }


Similarly, no need for type assertion here.

nateweller added the [Status] In Progress label Apr 1, 2024

nateweller self-assigned this Apr 1, 2024

Base automatically changed from add/a4a/onboarding-next-steps-component to trunk April 1, 2024 19:27

nateweller force-pushed the add/a4a/onboarding-tours branch 4 times, most recently from 01e04e7 to 8bf0547 Compare April 2, 2024 05:00

cleacos mentioned this pull request Apr 2, 2024

A4A: Add next steps component to the overview section #88725

Closed

7 tasks

nateweller force-pushed the add/a4a/onboarding-tours branch 3 times, most recently from 40f880f to 80b1747 Compare April 2, 2024 17:58

nateweller requested a review from a team April 2, 2024 18:05

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

nateweller added A4A and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Apr 2, 2024

nateweller force-pushed the add/a4a/onboarding-tours branch from 55bedb3 to 9b41d64 Compare April 2, 2024 20:02

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

nateweller marked this pull request as ready for review April 2, 2024 23:24

nateweller added [Size] M Medium sized issue and removed [Status] In Progress labels Apr 2, 2024

yashwin reviewed Apr 3, 2024

View reviewed changes

nateweller added 4 commits April 3, 2024 14:15

Add onboarding tours

8b28a19

Use variable ordering for add new site step

754b762

Fix preference integration

2cae4a1

Remove commented out code

a553674

nateweller force-pushed the add/a4a/onboarding-tours branch from 6171336 to a553674 Compare April 3, 2024 20:17

Remove studio colors from guided tour step stylesheet

e7e7a8d

nateweller requested review from yashwin and a team April 3, 2024 20:21

andrii-lysenko previously approved these changes Apr 3, 2024

View reviewed changes

andrii-lysenko reviewed Apr 4, 2024

View reviewed changes

andrii-lysenko requested changes Apr 4, 2024

View reviewed changes

nateweller added 4 commits April 4, 2024 11:58

Merge remote-tracking branch 'origin' into add/a4a/onboarding-tours

13a5036

Address TS errors

79ced6f

Update Sort.direction type

8fbfa27

Misc fixes for guided tours

36b1ffb

andrii-lysenko approved these changes Apr 4, 2024

View reviewed changes

nateweller merged commit 12dd933 into trunk Apr 4, 2024
11 checks passed

nateweller deleted the add/a4a/onboarding-tours branch April 4, 2024 23:58

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 Apr 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A4A: Add "Next Steps" Onboarding Tours #89081

A4A: Add "Next Steps" Onboarding Tours #89081

nateweller commented Apr 1, 2024 •

edited

Loading

matticbot commented Apr 1, 2024 •

edited

Loading

matticbot commented Apr 1, 2024 •

edited

Loading

yashwin left a comment •

edited

Loading

yashwin Apr 3, 2024

nateweller commented Apr 3, 2024

andrii-lysenko left a comment •

edited

Loading

cleacos commented Apr 3, 2024

andrii-lysenko Apr 4, 2024

andrii-lysenko Apr 4, 2024

andrii-lysenko left a comment

andrii-lysenko Apr 4, 2024

andrii-lysenko Apr 4, 2024

andrii-lysenko Apr 4, 2024

andrii-lysenko Apr 4, 2024

andrii-lysenko Apr 4, 2024

andrii-lysenko Apr 4, 2024

matticbot commented Apr 4, 2024

andrii-lysenko left a comment •

edited

Loading

andrii-lysenko Apr 4, 2024

andrii-lysenko Apr 4, 2024

A4A: Add "Next Steps" Onboarding Tours #89081

A4A: Add "Next Steps" Onboarding Tours #89081

Conversation

nateweller commented Apr 1, 2024 • edited Loading

Proposed Changes

Testing Instructions

Pre-merge Checklist

Screenshots

matticbot commented Apr 1, 2024 • edited Loading

matticbot commented Apr 1, 2024 • edited Loading

yashwin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nateweller commented Apr 3, 2024

andrii-lysenko left a comment • edited Loading

Choose a reason for hiding this comment

cleacos commented Apr 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrii-lysenko left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matticbot commented Apr 4, 2024

andrii-lysenko left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nateweller commented Apr 1, 2024 •

edited

Loading

matticbot commented Apr 1, 2024 •

edited

Loading

matticbot commented Apr 1, 2024 •

edited

Loading

yashwin left a comment •

edited

Loading

andrii-lysenko left a comment •

edited

Loading

andrii-lysenko left a comment •

edited

Loading