wp-calypso
wp-calypso copied to clipboard
Automattic
Site Management Panel: Add the guide tour to the primary button
Related to https://github.com/Automattic/dotcom-forge/issues/7405
Proposed Changes
- Refactor the GuidedTour components to make it reusable
- Add the GuidedTourStep component to the primary button (WP Admin or My Home) on the site management panel
| WP Admin | My Home |
|---|---|
Testing Instructions
- Go to /sites
- Select any site
- Make sure you can see the guide tour on the primary button
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)?
Jetpack Cloud live (direct link)
|
|
https://calypso.live?image=registry.a8c.com/calypso/app:build-109136&env=jetpack |
Automattic for Agencies live (direct link)
|
|
https://calypso.live?image=registry.a8c.com/calypso/app:build-109136&env=a8c-for-agencies |
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:
Sections (~963 bytes added 📈 [gzipped])
name parsed_size gzip_size
sites-dashboard +2019 B (+0.2%) +584 B (+0.2%)
site-monitoring +2019 B (+0.2%) +584 B (+0.2%)
hosting +2019 B (+0.1%) +584 B (+0.1%)
github-deployments +2019 B (+0.2%) +584 B (+0.2%)
dev-tools +2019 B (+0.2%) +584 B (+0.2%)
plugins +770 B (+0.0%) +233 B (+0.0%)
jetpack-cloud-plugin-management +770 B (+0.0%) +233 B (+0.0%)
a8c-for-agencies-sites +752 B (+0.0%) +204 B (+0.0%)
a8c-for-agencies-settings +749 B (+0.4%) +192 B (+0.3%)
a8c-for-agencies-referrals +749 B (+0.2%) +192 B (+0.1%)
a8c-for-agencies-purchases +749 B (+0.2%) +192 B (+0.1%)
a8c-for-agencies-plugins +749 B (+0.5%) +192 B (+0.4%)
a8c-for-agencies-partner-directory +749 B (+0.5%) +192 B (+0.4%)
a8c-for-agencies-migrations +749 B (+0.4%) +192 B (+0.3%)
a8c-for-agencies-marketplace +749 B (+0.1%) +192 B (+0.1%)
a8c-for-agencies-landing +749 B (+0.8%) +222 B (+0.8%)
a8c-for-agencies-overview +477 B (+0.2%) +133 B (+0.2%)
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.
@cleacos I made changes to let the GuidedTours components be reusable between Calypso and A4A. What do you think?
I made changes to let the GuidedTours components be reusable between Calypso and A4A. What do you think?
@arthur791004, that is amazing! It's something I had in mind to do... at some point. Thank you for going ahead and doing it.
I haven't tested it on the A4A side yet. I guess it should work as before. I could test it later or next week.
@arthur791004 I noticed that the guided tour shows up again after page refresh, I think it's because endTour (src) only triggers when users clicks on the Skip button, which is not enabled here.
I noticed that the guided tour shows up again after page refresh, I think it's because endTour (src) only triggers when users clicks on the Skip button, which is not enabled here.
It's expected. The endTour will be called when you click the primary button (e.g.: Got it) as well. The DismissibleCard also updates the user's preference only when they click the action button.
That's good to know! I'm trying the PR in via the calypso.live link, and I when click the Got it button and refresh the page, the tooltip shows up again. I don't see the preference API being called or the event track triggering, can you reproduce it on your end?
That's good to know! I'm trying the PR in via the calypso.live link, and I when click the Got it button and refresh the page, the tooltip shows up again. I don't see the preference API being called or the event track triggering, can you reproduce it on your end?
hmmm...I found it won't end the tour when you finish the last step 😞
In my testing, the tooltip kind of "jumps" while the sidebar animation is still ongoing. Not sure if this is a blocker?
@arthur791004 This diff should fix this issue; feel free to adapt
diff
diff --git a/client/a8c-for-agencies/components/items-dashboard/item-preview-pane/index.tsx b/client/a8c-for-agencies/components/items-dashboard/item-preview-pane/index.tsx
index 071ca6e79c0..4ee8a26d2f1 100644
--- a/client/a8c-for-agencies/components/items-dashboard/item-preview-pane/index.tsx
+++ b/client/a8c-for-agencies/components/items-dashboard/item-preview-pane/index.tsx
@@ -83,6 +83,7 @@ export default function ItemPreviewPane( {
<ItemPreviewPaneHeader
closeItemPreviewPane={ closeItemPreviewPane }
itemData={ itemData }
+ isPreviewLoaded={ !! selectedFeature.preview }
extraProps={ itemPreviewPaneHeaderExtraProps }
/>
<div ref={ setNavRef }>
diff --git a/client/a8c-for-agencies/components/items-dashboard/item-preview-pane/item-preview-pane-header/index.tsx b/client/a8c-for-agencies/components/items-dashboard/item-preview-pane/item-preview-pane-header/index.tsx
index 7a9a3eac22f..801a14e3b12 100644
--- a/client/a8c-for-agencies/components/items-dashboard/item-preview-pane/item-preview-pane-header/index.tsx
+++ b/client/a8c-for-agencies/components/items-dashboard/item-preview-pane/item-preview-pane-header/index.tsx
@@ -16,12 +16,14 @@ const ICON_SIZE_REGULAR = 24;
interface Props {
closeItemPreviewPane?: () => void;
itemData: ItemData;
+ isPreviewLoaded: boolean;
className?: string;
extraProps?: ItemPreviewPaneHeaderExtraProps;
}
export default function ItemPreviewPaneHeader( {
itemData,
+ isPreviewLoaded,
closeItemPreviewPane,
className,
extraProps,
@@ -77,6 +79,7 @@ export default function ItemPreviewPaneHeader( {
<extraProps.headerButtons
focusRef={ focusRef }
itemData={ itemData }
+ isPreviewLoaded={ isPreviewLoaded }
closeSitePreviewPane={ closeItemPreviewPane || ( () => {} ) }
/>
) : (
diff --git a/client/sites-dashboard-v2/site-preview-pane/preview-pane-header-buttons.tsx b/client/sites-dashboard-v2/site-preview-pane/preview-pane-header-buttons.tsx
index e8c0e00a954..e34790bd6d2 100644
--- a/client/sites-dashboard-v2/site-preview-pane/preview-pane-header-buttons.tsx
+++ b/client/sites-dashboard-v2/site-preview-pane/preview-pane-header-buttons.tsx
@@ -10,10 +10,16 @@ import type { ItemData } from 'calypso/a8c-for-agencies/components/items-dashboa
type Props = {
focusRef: React.RefObject< HTMLButtonElement >;
itemData: ItemData;
+ isPreviewLoaded: boolean;
closeSitePreviewPane?: () => void;
};
-const PreviewPaneHeaderButtons = ( { focusRef, closeSitePreviewPane, itemData }: Props ) => {
+const PreviewPaneHeaderButtons = ( {
+ focusRef,
+ itemData,
+ isPreviewLoaded,
+ closeSitePreviewPane,
+}: Props ) => {
const adminButtonRef = useRef();
const { adminLabel, adminUrl } = useSiteAdminInterfaceData( itemData.blogId );
const { __ } = useI18n();
@@ -31,21 +37,25 @@ const PreviewPaneHeaderButtons = ( { focusRef, closeSitePreviewPane, itemData }:
>
{ adminLabel }
</Button>
- <GuidedTourStep
- id="site-management-panel-admin-button"
- tourId="siteManagementPanel"
- context={ adminButtonRef.current }
- title={ sprintf(
- // translators: %s is the label of the admin
- __( 'Link to %s' ),
- adminLabel
- ) }
- description={ sprintf(
- // translators: %s is the label of the admin
- __( 'Navigate seamlessly between your site management panel and %s with just one click' ),
- adminLabel
- ) }
- />
+ { isPreviewLoaded && (
+ <GuidedTourStep
+ id="site-management-panel-admin-button"
+ tourId="siteManagementPanel"
+ context={ adminButtonRef.current }
+ title={ sprintf(
+ // translators: %s is the label of the admin
+ __( 'Link to %s' ),
+ adminLabel
+ ) }
+ description={ sprintf(
+ // translators: %s is the label of the admin
+ __(
+ 'Navigate seamlessly between your site management panel and %s with just one click'
+ ),
+ adminLabel
+ ) }
+ />
+ ) }
</>
);
};
- Resolved the jumping tours by https://github.com/Automattic/wp-calypso/pull/91100/commits/0f097ce4e6fcff0c48bd0b2348455f8ec3d79f22
- Resolved the tour is not marked completed by https://github.com/Automattic/wp-calypso/pull/91100/commits/58bba85402747978c2988a316ba6f7a61f4ecfbc
- Resolved the mobile issue by https://github.com/Automattic/wp-calypso/pull/91100/commits/dee426c062583cd6fdcf7ace297334877fc320e5
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 feat/add-guided-tours-to-site-dashboard on your sandbox.
@cleacos When you have time, could you kindly review it again? Thank you!
Also I notice this style issue (horizontal padding) at A4A with the Preview Pane opened:
It's outdated as it's resolved by https://github.com/Automattic/wp-calypso/pull/91034
By the way, I forgot to say that this is a great refactoring code to make it reusable for everyone. Thanks, @arthur791004, for going ahead.
@arthur791004 Wrapping/encapsulating the hook with the useConditionalGuidedTours, as I suggested above, didn't work?
Wrapping/encapsulating the hook with the useConditionalGuidedTours, as I suggested above, didn't work?
It still put the hook inside the condition 😅
Question:
preferenceNames={ A4A_ONBOARDING_TOURS_PREFERENCE_NAME }
eventNames={ A4A_ONBOARDING_TOURS_EVENT_NAMES }
Do those values affect Dotcom guided tours? they seem coupled to A4A
Do those values affect Dotcom guided tours? they seem coupled to A4A
What do you mean?
For instance, eventNames has information about tracking events for A4A. If you use the Guided Tour at Dotcom, it will track events from Dotcom as if they were A4A events. Is that right?
dotcom has its event names but the properties will be the same.
export const CALYPSO_ONBOARDING_TOURS_EVENT_NAMES: Record< string, string > = {
startTour: 'calypso_site_dashboard_start_tour',
endTour: 'calypso_site_dashboard_end_tour',
};