Make bundled theme generic (pre checkout) #84483

renatho · 2023-11-23T20:50:52Z

Related to Automattic/themes#7498

Proposed Changes

It's a refactor to make the bundle theme more generic, so we can easily add other themes on the future. Basically, all the settings are part of the new client/my-sites/theme/bundle-settings.tsx.
- This doesn't include the installation flow yet. I decided to explore it as a separate thing and raise another PR to avoid more complexity in this one.
~~I also added the "[Status] String Freeze" label to this PR, so we make sure we don't break the translations of strings that are already in the site but had their structure changed.~~ Make bundled theme generic (pre checkout) #84483 (comment)

Testing Instructions

Themes

Navigate to http://calypso.localhost:3000/themes
Select a site.
Search for "Tsubaki".
Make sure it continues working as properly, as previously.

Design Picker and checkout

Navigate to the http://calypso.localhost:3000/start
Navigate through the flow until get to the Design Picker.
Filter by store.
Check the Tsubaki theme and make sure it continues working properly, as previously.
Choose the theme, click on "Upgrade Plan", and make sure it goes to the checkout to buy the business plan.
Complete the checkout, and check that it returns to the theme and that you can continue.

Screenshots

Themes

Design Picker

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

About the first item, I didn't test exhaustively the browser support because it shouldn't change anything for the browser.

matticbot · 2023-11-23T20:51:27Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

matticbot · 2023-11-23T21:03:26Z

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

Sections (~3656 bytes added 📈 [gzipped])

name                     parsed_size           gzip_size
theme                        +5582 B  (+0.8%)    +2399 B  (+1.1%)
themes                       +3368 B  (+0.4%)    +1407 B  (+0.5%)
import-flow                  +3304 B  (+0.1%)    +1367 B  (+0.1%)
tailored-ecommerce-flow        -98 B  (-0.0%)      -55 B  (-0.0%)
link-in-bio-tld-flow           -98 B  (-0.0%)      -55 B  (-0.0%)

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

Async-loaded Components (~2860 bytes added 📈 [gzipped])

name                                     parsed_size           gzip_size
async-load-signup-steps-theme-selection      +3372 B  (+1.0%)    +1439 B  (+1.4%)
async-load-design-blocks                     +3368 B  (+0.1%)    +1442 B  (+0.2%)
async-load-signup-steps-store-features        -110 B  (-0.2%)      -21 B  (-0.2%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

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.

renatho · 2023-11-24T21:11:35Z

packages/design-picker/src/constants.ts

@@ -80,5 +80,5 @@ export const DEFAULT_ASSEMBLER_DESIGN = {
 export const FREE_THEME = 'free';
 export const PREMIUM_THEME = 'premium';
 export const DOT_ORG_THEME = 'dot-org';
-export const WOOCOMMERCE_THEME = 'woocommerce';
+export const BUNDLED_THEME = 'bundled';


Is it a good name?

Good question. I'd consider flipping it around so we're looking at THEME_WITH_BUNDLED_SOFTWARE, but that looks nothing like the other constants. 🤔

Yeah! 🤔
I'll keep this one for now, but if any idea comes to your mind, feel free to let me know that I can change it. 😉

renatho · 2023-11-24T21:45:46Z

client/state/themes/selectors/get-theme-software-set.js

+			return [];
+		}
+
+		return themeSoftwareSetTaxonomy.map( ( item ) => item.slug );


Is it the slug the correct reference we should use here? I went with this approach based on this interface I found in the code during the refactor:

wp-calypso/packages/design-picker/src/types.ts

Line 73 in 230e743

export interface SoftwareSet {

renatho · 2023-11-24T22:26:40Z

client/components/theme-type-badge/index.tsx

@@ -44,6 +46,8 @@ const ThemeTypeBadge = ( {
 		} );
 	}, [ type, themeId, isLockedStyleVariation ] );

+	const themeSoftware = themeSoftwareSet[ 0 ];


In some places, we are hard coding the themeSoftwareSet[ 0 ] to get the first one. Do we plan to have multiple plugins bundled with a team? Sensei LMS + WooCommerce, for example. If so, we need some more refactor.

You can have multiple plugins already - the Woo software set includes Woo and Woo Payments plugins.

I think the question is more 'do we plan to have multiple software sets for a theme'. There aren't any plans that I know of, and from a data pov it could just be a new software set that contains e.g. woo+sensei plugins, it's from a UI perspective it gets trickier - which badge do we display etc.

I don't think we should worry too much about this right now, lets assume that there are always zero or one software sets for any given theme.

For this thread, it may be worth considering a way to identify the primary software set: what does the user think they are getting/installing when they pick this theme?

I think the question is more 'do we plan to have multiple software sets for a theme'.

Sorry! That's correct!

I think the question is more 'do we plan to have multiple software sets for a theme'.

Could you elaborate a little more your idea @daledupreez? I'm not sure if I understand it.
Do you mean improving the UI with the details of what would be installed? Or something in the showcase editor to set the primary software set? Or something else not related to this at all? 🤭

renatho · 2023-11-24T22:27:51Z

client/my-sites/theme/bundle-settings.tsx

+	'woo-on-plans': {
+		name: 'WooCommerce',
+		badgeIcon: (
+			<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">


The original icon was changed to this one, so it uses the currentColor to set the icon as white, and the "Woo" text is transparent. It will make it easier to create the icons generically for all bundles.

renatho · 2023-11-27T14:13:41Z

client/my-sites/theme/bundle-settings.tsx

+}
+
+const bundleSettings: BundleSettings = {
+	'woo-on-plans': {


I was planning to change the 'woo-on-plans' to a constant. But it appears in many places, so I didn't want to risk an accidental issue for now.

renatho · 2023-11-27T14:30:52Z

client/components/theme-type-badge/tooltip.tsx

+			const bundleName = bundleSettings[ themeSoftware ].name;
+
+			if ( isIncludedCurrentPlan ) {
+				message = translate( 'This %(bundleName)s theme is included in your plan.', {


Currently, for many of the texts, I'm just interpolating the bundle name. If we want a specific text for any part, we can add it to the bundleSettings.

…e details through props

…ic for any bundle

It will be documented and discussed in the PR.

The reference for this name appears in many places, so I'm keeping at for now to avoid any accidental issue.

okmttdhr · 2023-11-30T04:32:25Z

packages/data-stores/src/starter-designs-queries/use-starter-designs-query.ts

-	const is_bundled_with_woo_commerce = ( design.software_sets || [] ).some(
-		( { slug } ) => slug === 'woo-on-plans'
-	);


Hello, @renatho! I have a question 🙋‍♀️
The original function is_bundled_with_woo_commerce checks for the presence of woo-on-plans. However, the PR modifies this to check if software_sets.length > 0.
As far as checking useBundleSettings, there will be additional types of software_sets. In that case, these changes will break the original behavior in the future, am I correct?

Hi @okmttdhr! Correct! That's the idea.
We want to consider the theme as a bundled theme when it has any software set. And apply the specific settings for each software set.
Currently, it has only the woo-on-plans. But in a following PR, we also want to introduce a software set for Sensei LMS, which will have its specific settings.

Thanks for your comment, @renatho!
This PR changes that instead of checking for woo-on-plans, we should only verify if software_sets.length > 0 (as seen in design-setup/unified-design-picker.tsx for example). This approach means that the condition will be true if any new software sets introduced in the future not limited to Woo.

And I assume this is the intended purpose of this PR. I'm highlighting this to clarify my understanding 🙂

That's completely correct! 🙂
For context, there are some more details in pekYwv-3rN-p2 and in p1700605581879429-slack-C048CUFRGFQ.

dsas

Doing some testing and after purchasing a business plan via /themes I don't get sent back to /setup/site-setup/designSetup?siteSlug=SLUG.wordpress.com&theme=tsubaki to activate the Woo theme, I go to the congrats page ahead. This isn't the case on staging - it works as expected.

It works as expected if I choose a theme and upgrade via /start

…make-bundled-theme-generic

renatho · 2023-11-30T20:03:07Z

Doing some testing and after purchasing a business plan via /themes I don't get sent back to /setup/site-setup/designSetup?siteSlug=SLUG.wordpress.com&theme=tsubaki to activate the Woo theme, I go to the congrats page ahead. This isn't the case on staging - it works as expected.

@dsas, I couldn't reproduce the issue. Could you confirm if you followed some different steps compared to the following video? Or maybe you can also see the issue in my video and I misunderstood something?

Screen.Recording.2023-11-30.at.16.47.33.mov

(you will just see an extra refresh on the checkout page that I did for the credits)

And I didn't record, but after going through the WC setup wizard, I was redirected to the congrats page. Is this the page you saw directly after the checkout?

dsas · 2023-12-01T12:07:46Z

@renatho I can't reproduce the issue either, and I didn't record what I was doing. Perhaps I got a step wrong, or it was related to some trunk changes somehow

dsas

Ah wait, I found the issue again. After the checkout I'm not being redirected to the theme, but to a 'Congrats' page.

Screen.Recording.2023-12-01.at.12.11.53.mov

It looks like this is a problem with the wpcalypso.wordpress.com environment. It happens when I use wpcalypso.wordpress.com directly, or when I test this PR with the calypso live link. It doesn't happen when I'm testing this PR locally, or if I'm just on WordPress.com.

renatho · 2023-12-01T14:19:02Z

It looks like this is a problem with the wpcalypso.wordpress.com environment. It happens when I use wpcalypso.wordpress.com directly, or when I test this PR with the calypso live link. It doesn't happen when I'm testing this PR locally, or if I'm just on WordPress.com.

To confirm, I tested it in a different PR not related to these changes: #84175, and I could also see the same issue there. So it's confirmed that it's some issue because of the test environment.

Thank you everyone for the reviews! I'll wait until Monday to deploy it to see if there's any other feedback, and to avoid the deployment on Friday. :)

dsas · 2023-12-01T14:54:43Z

I opened #84767 to look at the test environment issue

a8ci18n · 2023-12-05T00:42:33Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/9876186

Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.

Thank you @renatho for including a screenshot in the description! This is really helpful for our translators.

a8ci18n · 2023-12-09T10:32:05Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/9876186

Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.

Thank you @renatho for including a screenshot in the description! This is really helpful for our translators.

* Refactor WooCommerceBundledBadge component to be generic receiving the details through props * Update Woo icon to simplify styles * Refactor sass code * Add container for the icon * Move WooCommerce badge settings to a settings file * Add selector to get theme software set * Make WooCommerce theme bundle generic, in order to allow other bundles * Return slugs instead of names * Refactor occurrences of is_bundled_with_woo_commerce to make it generic for any bundle * Remove TODOs since it will have multiple occurrences It will be documented and discussed in the PR. * Refactor messages to be generic using bundle settings * Fix missing variable * Update badge icon to a component * Remove todo comment The reference for this name appears in many places, so I'm keeping at for now to avoid any accidental issue. * Fix design picker badge * Add a separate text for upsell description * Add setting for bundled badge color * Update property name to match other properties casing * Move variables to the scope where it's used * Update icon and color settings to be generic * Make bundle modal generic * Decrease padding around svg With this the image matches a little better the current state of the image in the modal * Add comment to document field * Add translator comments * Transform new selector to typescript * Import only FunctionComponent type Co-authored-by: daledupreez <dale@automattic.com> * Update sass file to not use name concating The purpose is to be easier to find the classes in a search operation. * Move bundle settings to a hook So it can re-render when translations change. * Remove separation of the static and dynamic part The benefit isn't worth the extra complexity. * Improve vertical alignment of the icon * Remove not used check * Fix feature check to check based on the software set --------- Co-authored-by: daledupreez <dale@automattic.com>

a8ci18n · 2023-12-18T09:30:27Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/9876186

Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.

Thank you @renatho for including a screenshot in the description! This is really helpful for our translators.

a8ci18n · 2023-12-24T21:29:45Z

Translation for this Pull Request has now been finished.

renatho self-assigned this Nov 23, 2023

renatho force-pushed the change/make-bundled-theme-generic branch from 547ca45 to 0412ddc Compare November 23, 2023 20:53

renatho force-pushed the change/make-bundled-theme-generic branch 3 times, most recently from a8708ff to 5031f33 Compare November 24, 2023 18:22

renatho commented Nov 24, 2023

View reviewed changes

renatho force-pushed the change/make-bundled-theme-generic branch from 05a7950 to 15f999e Compare November 24, 2023 22:04

renatho commented Nov 24, 2023

View reviewed changes

renatho commented Nov 27, 2023

View reviewed changes

renatho added 14 commits November 27, 2023 15:59

Refactor WooCommerceBundledBadge component to be generic receiving th…

ba3ec8e

…e details through props

Update Woo icon to simplify styles

9e7f290

Refactor sass code

0e40d2e

Add container for the icon

c30612a

Move WooCommerce badge settings to a settings file

b91c5f8

Add selector to get theme software set

e4c7787

Make WooCommerce theme bundle generic, in order to allow other bundles

0ba30a3

Return slugs instead of names

99d23c2

Refactor occurrences of is_bundled_with_woo_commerce to make it gener…

e111b4e

…ic for any bundle

Remove TODOs since it will have multiple occurrences

6c729e2

It will be documented and discussed in the PR.

Refactor messages to be generic using bundle settings

0aa92a8

Fix missing variable

0437af7

Update badge icon to a component

15f6ce6

Remove todo comment

895cbe2

The reference for this name appears in many places, so I'm keeping at for now to avoid any accidental issue.

renatho force-pushed the change/make-bundled-theme-generic branch from 7e6fe0f to 38b814a Compare November 27, 2023 18:59

Fix design picker badge

4c9659c

Improve vertical alignment of the icon

9e39037

okmttdhr reviewed Nov 30, 2023

View reviewed changes

Remove not used check

6cf6789

dsas reviewed Nov 30, 2023

View reviewed changes

renatho added 2 commits November 30, 2023 16:34

Fix feature check to check based on the software set

d314aab

Merge branch 'trunk' of github.com:Automattic/wp-calypso into change/…

5439351

…make-bundled-theme-generic

renatho requested review from daledupreez and dsas November 30, 2023 20:03

dsas approved these changes Dec 1, 2023

View reviewed changes

dsas requested changes Dec 1, 2023

View reviewed changes

dsas approved these changes Dec 1, 2023

View reviewed changes

renatho merged commit afad6f6 into trunk Dec 4, 2023
14 checks passed

renatho deleted the change/make-bundled-theme-generic branch December 4, 2023 13:46

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 4, 2023

renatho mentioned this pull request Dec 6, 2023

Add missing translators comment #84925

Merged

7 tasks

okmttdhr mentioned this pull request Dec 6, 2023

Live Preview: Disable the Global Styles limit when live previewing Woo and Premium themes #84924

Merged

7 tasks

renatho mentioned this pull request Dec 8, 2023

Refactor the bundle installation to be generic #84886

Merged

7 tasks

renatho changed the title ~~Make bundled theme generic~~ Make bundled theme generic (pre checkout) Dec 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make bundled theme generic (pre checkout) #84483

Make bundled theme generic (pre checkout) #84483

renatho commented Nov 23, 2023 •

edited

Loading

matticbot commented Nov 23, 2023 •

edited

Loading

matticbot commented Nov 23, 2023 •

edited

Loading

renatho Nov 24, 2023

daledupreez Nov 28, 2023

renatho Nov 29, 2023

renatho Nov 24, 2023

renatho Nov 24, 2023 •

edited

Loading

dsas Nov 28, 2023 •

edited

Loading

daledupreez Nov 28, 2023

renatho Nov 29, 2023

renatho Nov 24, 2023 •

edited

Loading

renatho Nov 27, 2023

renatho Nov 27, 2023

okmttdhr Nov 30, 2023

renatho Nov 30, 2023

okmttdhr Dec 4, 2023

renatho Dec 4, 2023

dsas left a comment

renatho commented Nov 30, 2023

dsas commented Dec 1, 2023

dsas left a comment •

edited

Loading

renatho commented Dec 1, 2023

dsas commented Dec 1, 2023

a8ci18n commented Dec 5, 2023

a8ci18n commented Dec 9, 2023 •

edited

Loading

a8ci18n commented Dec 18, 2023 •

edited

Loading

a8ci18n commented Dec 24, 2023

Make bundled theme generic (pre checkout) #84483

Make bundled theme generic (pre checkout) #84483

Conversation

renatho commented Nov 23, 2023 • edited Loading

Proposed Changes

Testing Instructions

Screenshots

Pre-merge Checklist

matticbot commented Nov 23, 2023 • edited Loading

matticbot commented Nov 23, 2023 • edited Loading

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

renatho Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

dsas Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renatho Nov 24, 2023 • edited Loading

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

dsas left a comment

Choose a reason for hiding this comment

renatho commented Nov 30, 2023

dsas commented Dec 1, 2023

dsas left a comment • edited Loading

Choose a reason for hiding this comment

renatho commented Dec 1, 2023

dsas commented Dec 1, 2023

a8ci18n commented Dec 5, 2023

a8ci18n commented Dec 9, 2023 • edited Loading

a8ci18n commented Dec 18, 2023 • edited Loading

a8ci18n commented Dec 24, 2023

renatho commented Nov 23, 2023 •

edited

Loading

matticbot commented Nov 23, 2023 •

edited

Loading

matticbot commented Nov 23, 2023 •

edited

Loading

renatho Nov 24, 2023 •

edited

Loading

dsas Nov 28, 2023 •

edited

Loading

renatho Nov 24, 2023 •

edited

Loading

dsas left a comment •

edited

Loading

a8ci18n commented Dec 9, 2023 •

edited

Loading

a8ci18n commented Dec 18, 2023 •

edited

Loading