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

Live Preview: Add the upgrade notice on the sidebar for Premium and Woo themes #84676

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

okmttdhr
Copy link
Member

@okmttdhr okmttdhr commented Nov 30, 2023

Related to #79223, #83630

Proposed Changes

This PR adds the upgrade-needed notice on the Site Editor sidebar when live-previewing Premium and WooCommerce themes.

UI:
Screenshot 2023-12-01 at 15 46 44

Flow:

Screen.Recording.2023-12-01.at.15.51.57.mov
Screenshot 2023-12-01 at 15 51 11

Other tasks are listed in #79223.

Testing Instructions

  • Run install-plugin.sh wpcom-block-editor add/live-preview-upgrade-notice-sidebar on your sandbox.
  • Sandbox widgets.wp.com and your site.
  • Upgrade Notice for Premium themes
    • Prepare a site with the Personal plan or lower plan.
    • Access https://<your-site>/wp-admin/site-editor.php?wp_theme_preview=<premium-theme> to preview a premium theme (e.g., premium/outland).
    • Try to edit the homepage and see the upgrade notice.
    • Click Upgrade now.
    • Verify the Premium plan is in the cart and complete checkout.
    • Verify it redirects to the Site Editor.
    • Verify you don't see the upgrade notice and the plan is upgraded.
  • Upgrade Notice for WooCommerce themes
    • Prepare a site with the Premium plan or lower plan.
    • Access https://<your-site>/wp-admin/site-editor.php?wp_theme_preview=<woo-theme> to preview a woo commerce theme (e.g. premium/tsubaki).
    • Try to edit the homepage and see the upgrade notice.
    • Click Upgrade now.
    • Verify the Business plan is in the cart and complete checkout.
    • Verify it redirects to the Site Editor.
    • Verify you don't see the upgrade notice and the plan is upgraded.
  • No Upgrade Notices
    • Prepare a site with the Business plan or higher plan.
    • Access https://<your-site>/wp-admin/site-editor.php?wp_theme_preview=<premium-theme> to preview a premium theme (e.g., premium/outland).
      • If your site is on Atomic, you need to install the theme before previewing it.
    • Verify you don't see the upgrade notice.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • https://wpcalypso.wordpress.com/devdocs/docs/testing/index.md 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)?

Copy link

github-actions bot commented Nov 30, 2023

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

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/live-preview-upgrade-notice-sidebar on your sandbox.

@okmttdhr okmttdhr marked this pull request as ready for review December 1, 2023 06:58
@okmttdhr okmttdhr requested a review from a team December 1, 2023 06:58
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 1, 2023
@okmttdhr okmttdhr self-assigned this Dec 1, 2023
@okmttdhr okmttdhr force-pushed the add/live-preview-upgrade-notice-sidebar branch from 3cc9632 to edefc16 Compare December 4, 2023 05:49
Copy link
Contributor

@taipeicoder taipeicoder left a comment

Choose a reason for hiding this comment

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

Tested:

  • Upgrade Notice for Premium themes

    • Prepare a site with the Personal plan or lower plan.
    • Access https:///wp-admin/site-editor.php?wp_theme_preview= to preview a premium theme (e.g., premium/outland).
    • Try to edit the homepage and see the upgrade notice.
    • Click Upgrade now.
    • ✅ Verify the Premium plan is in the cart and complete checkout.
    • ✅ Verify it redirects to the Site Editor.
    • ✅ Verify you don't see the upgrade notice and the plan is upgraded.
  • Upgrade Notice for WooCommerce themes

    • Prepare a site with the Premium plan or lower plan.
    • Access https:///wp-admin/site-editor.php?wp_theme_preview= to preview a woo commerce theme (e.g. premium/tsubaki).
    • Try to edit the homepage and see the upgrade notice.
      Click Upgrade now.
    • ✅ Verify the Business plan is in the cart and complete checkout.
    • ✅ Verify it redirects to the Site Editor.
    • ✅ Verify you don't see the upgrade notice and the plan is upgraded.
  • No Upgrade Notices

    • Prepare a site with the Business plan or higher plan.
    • Access https:///wp-admin/site-editor.php?wp_theme_preview= to preview a premium theme (e.g., premium/outland).
      If your site is on Atomic, you need to install the theme before previewing it.
    • ✅ Verify you don't see the upgrade notice.

@okmttdhr okmttdhr force-pushed the add/live-preview-upgrade-notice-sidebar branch from edefc16 to 0b060a9 Compare December 5, 2023 05:56
@fushar
Copy link
Contributor

fushar commented Dec 5, 2023

I see a lot of duplication of logic on determining whether we're currently live-previewing or not. I have an idea to extract that logic from the component(s). Please check whether it makes sense or not. I tested it roughly and it seems to work.

The diff
diff --git a/apps/wpcom-block-editor/src/wpcom/features/live-preview/index.tsx b/apps/wpcom-block-editor/src/wpcom/features/live-preview/index.tsx
index 7f90649bad..b21779f947 100644
--- a/apps/wpcom-block-editor/src/wpcom/features/live-preview/index.tsx
+++ b/apps/wpcom-block-editor/src/wpcom/features/live-preview/index.tsx
@@ -5,7 +5,6 @@ import { registerPlugin } from '@wordpress/plugins';
 import { FC, useEffect } from 'react';
 import { useCanPreviewButNeedUpgrade } from './hooks/use-can-preview-but-need-upgrade';
 import { usePreviewingTheme } from './hooks/use-previewing-theme';
-import { LivePreviewUpgradeNotice } from './upgrade-notice';
 import { getUnlock } from './utils';
 
 const unlock = getUnlock();
@@ -18,76 +17,75 @@ const NOTICE_ID = 'wpcom-live-preview/notice';
  * @see https://github.com/Automattic/wp-calypso/issues/82218
  */
 const LivePreviewNotice: FC< {
-	previewingThemeName?: string;
-} > = ( { previewingThemeName } ) => {
+	canPreviewButNeedUpgrade: boolean;
+	previewingTheme?: ReturnType< typeof usePreviewingTheme >;
+	dashboardLink?: string;
+} > = ( { canPreviewButNeedUpgrade, previewingTheme, dashboardLink } ) => {
 	const { createWarningNotice, removeNotice } = useDispatch( 'core/notices' );
-
-	const siteEditorStore = useSelect( ( select ) => select( 'core/edit-site' ), [] );
-	const dashboardLink =
-		unlock &&
-		siteEditorStore &&
-		unlock( siteEditorStore ).getSettings().__experimentalDashboardLink;
+	const { set: setPreferences } = useDispatch( 'core/preferences' );
 
 	useEffect( () => {
-		// Do nothing in the Post Editor context.
-		if ( ! siteEditorStore ) {
-			return;
-		}
-		if ( ! previewingThemeName ) {
-			removeNotice( NOTICE_ID );
-			return;
-		}
+		// Suppress the "Looking for template parts?" notice in the Site Editor sidebar.
+		// The preference name is defined in https://github.com/WordPress/gutenberg/blob/d47419499cd58e20db25c370cdbf02ddf7cffce0/packages/edit-site/src/components/sidebar-navigation-screen-main/template-part-hint.js#L9.
+		setPreferences( 'core', 'isTemplatePartMoveHintVisible', false );
 
-		createWarningNotice(
-			sprintf(
-				// translators: %s: theme name
-				__(
-					'You are previewing the %s theme. You can try out your own style customizations, which will only be saved if you activate this theme.',
-					'wpcom-live-preview'
+		if ( canPreviewButNeedUpgrade ) {
+			// migrate logic for upgrade-needed notice here
+		} else {
+			createWarningNotice(
+				sprintf(
+					// translators: %s: theme name
+					__(
+						'You are previewing the %s theme. You can try out your own style customizations, which will only be saved if you activate this theme.',
+						'wpcom-live-preview'
+					),
+					previewingTheme?.name
 				),
-				previewingThemeName
-			),
-			{
-				id: NOTICE_ID,
-				isDismissible: false,
-				actions: dashboardLink && [
-					{
-						label: __( 'Back to themes', 'wpcom-live-preview' ),
-						url: dashboardLink,
-						variant: 'secondary',
-					},
-				],
-			}
-		);
+				{
+					id: NOTICE_ID,
+					isDismissible: false,
+					actions: dashboardLink && [
+						{
+							label: __( 'Back to themes', 'wpcom-live-preview' ),
+							url: dashboardLink,
+							variant: 'secondary',
+						},
+					],
+				}
+			);
+		}
 		return () => removeNotice( NOTICE_ID );
-	}, [ siteEditorStore, dashboardLink, createWarningNotice, removeNotice, previewingThemeName ] );
+	}, [
+		dashboardLink,
+		createWarningNotice,
+		removeNotice,
+		previewingTheme,
+		setPreferences,
+		canPreviewButNeedUpgrade,
+	] );
 	return null;
 };
 
 const LivePreviewNoticePlugin = () => {
-	const siteEditorStore = useSelect( ( select ) => select( 'core/edit-site' ), [] );
 	const previewingTheme = usePreviewingTheme();
 	const { canPreviewButNeedUpgrade, upgradePlan } = useCanPreviewButNeedUpgrade( {
 		previewingTheme,
 	} );
 
-	const { set: setPreferences } = useDispatch( 'core/preferences' );
-	useEffect( () => {
-		if ( ! siteEditorStore ) {
-			return;
-		}
-		if ( ! previewingTheme.name ) {
-			return;
-		}
-		// Suppress the "Looking for template parts?" notice in the Site Editor sidebar.
-		// The preference name is defined in https://github.com/WordPress/gutenberg/blob/d47419499cd58e20db25c370cdbf02ddf7cffce0/packages/edit-site/src/components/sidebar-navigation-screen-main/template-part-hint.js#L9.
-		setPreferences( 'core', 'isTemplatePartMoveHintVisible', false );
-	}, [ previewingTheme.name, setPreferences, siteEditorStore ] );
+	const siteEditorStore = useSelect( ( select ) => select( 'core/edit-site' ), [] );
+	const dashboardLink =
+		unlock &&
+		siteEditorStore &&
+		unlock( siteEditorStore ).getSettings().__experimentalDashboardLink;
 
-	if ( canPreviewButNeedUpgrade ) {
-		return <LivePreviewUpgradeNotice { ...{ previewingTheme, upgradePlan } } />;
+	if ( ! siteEditorStore ) {
+		return null;
+	}
+	if ( ! previewingTheme.name ) {
+		return null;
 	}
-	return <LivePreviewNotice { ...{ previewingThemeName: previewingTheme.name } } />;
+
+	return <LivePreviewNotice { ...{ previewingTheme, dashboardLink, canPreviewButNeedUpgrade } } />;
 };
 
 const registerLivePreviewPlugin = () => {

Copy link
Contributor

@fushar fushar left a comment

Choose a reason for hiding this comment

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

Works as intended.

@miksansegundo
Copy link
Contributor

Without upgrading first, the Activate button is not working, which is a known issue. #83630 (comment)

We show two notices asking users to upgrade when they try global style customizations. I'm aware that this is also a known issue that will be solved by hiding the Global Style notice. #83630 (comment)

Screenshot 2566-12-05 at 18 37 24

This PR introduces the new notice without removing the Global Style notice.

Screenshot 2566-12-05 at 18 38 21

The rest is working well on my tests 👍

className="wpcom-live-preview-upgrade-notice-view"
actions={ [
{
// TODO: Add the tracking event.
Copy link
Contributor

Choose a reason for hiding this comment

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

The tracks events are probably these:

  • calypso_upgrade_nudge_impression
  • calypso_upgrade_nudge_cta_click

@okmttdhr okmttdhr force-pushed the add/live-preview-upgrade-notice-sidebar branch from 80f06a9 to 999ed05 Compare December 6, 2023 01:58
@okmttdhr
Copy link
Member Author

okmttdhr commented Dec 6, 2023

Thank you for your implementation, @fushar!

I see a lot of duplication of logic on determining whether we're currently live-previewing or not. I have an idea to extract that logic from the component(s). Please check whether it makes sense or not. I tested it roughly and it seems to work.

I crafted a PR referring to your diff with a little tweak 🙂 #84892
You can take a look at it while I test it.

@okmttdhr
Copy link
Member Author

okmttdhr commented Dec 6, 2023

I'm merging this PR. Let's have a discussion about refactoring at #84892!

@okmttdhr okmttdhr merged commit c66700f into trunk Dec 6, 2023
12 checks passed
@okmttdhr okmttdhr deleted the add/live-preview-upgrade-notice-sidebar branch December 6, 2023 02:21
@github-actions 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 Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants