wp-calypso icon indicating copy to clipboard operation
wp-calypso copied to clipboard

Initial theme for ecommerce sites: twentytwentytwo

Open mreishus opened this issue 2 years ago • 2 comments

Proposed Changes

  • New ecommerce sites will now have their initial theme set separately compared to the rest of sites.
  • That Initial theme for ecommerce sites will be twentytwentytwo.

Mechanisms

There are multiple mechanisms for determining a new site's initial theme. We can use a themeSlugWithRepo, or a siteTypeTheme.

https://github.com/Automattic/wp-calypso/blob/633f99cb0f1c248ac8fb1934d84518a643fb5484/client/lib/signup/step-actions/index.js#L134-L142

Mechanisms (siteType)

One possibility (siteTypeTheme) is determining a site's "type", then looking that up in a predefined list, and finding the theme that corresponds to that site type. https://github.com/Automattic/wp-calypso/blob/633f99cb0f1c248ac8fb1934d84518a643fb5484/client/lib/signup/step-actions/index.js#L129 https://github.com/Automattic/wp-calypso/blob/633f99cb0f1c248ac8fb1934d84518a643fb5484/client/lib/signup/site-type.js#L62-L66

When creating a new site on /start, in my testing, the siteType is always empty string: https://github.com/Automattic/wp-calypso/blob/633f99cb0f1c248ac8fb1934d84518a643fb5484/client/lib/signup/step-actions/index.js#L127

The /start flow doesn't seem to set siteTypes at all, only the older /new flow. I could have set the siteType, but it's not clear to me exactly what a siteType is, or what other effects setting a siteType would have. So I tried the other mechanism:

Mechanisms (themeSlugWithRepo)

Another possibility is the themeSlugWithRepo. This is a dependency in some of the flows contained in the older onboarding framework. It can be set and passed like so:

https://github.com/Automattic/wp-calypso/blob/633f99cb0f1c248ac8fb1934d84518a643fb5484/client/signup/steps/plans/index.jsx#L111-L119

That area of the plans step seemed to be a collection of 'default theme overrides', so I thought it was a good place to put this fix. We look for ecommerce in the cart on the default onboarding flow and pass along the themeSlugWithRepo dependency to the new theme we want.

I had to add themeSlugWithRepo to both the providesDependencies and optionalDependencies attributes of the plans step in order to be able to pass this around. (To make something optional, you have to add it to both).

https://github.com/Automattic/wp-calypso/blob/633f99cb0f1c248ac8fb1934d84518a643fb5484/client/signup/README.md#L42-L43

Testing Instructions

  • Use /start to create a new ecommerce site. It should have the 2022 theme.
  • Use /start to create other sites, not using ecommerce. They should continue to have zoologist.
  • Anything else you can think of.

BTW, I'm not very familiar with this area of the code.

Related to #67562

mreishus avatar Sep 22 '22 15:09 mreishus

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

Sections (~40 bytes added 📈 [gzipped])

name              parsed_size           gzip_size
signup                  +40 B  (+0.0%)       +8 B  (+0.0%)
plugins                 +40 B  (+0.0%)       +9 B  (+0.0%)
plans                   +40 B  (+0.0%)       +9 B  (+0.0%)
jetpack-connect         +40 B  (+0.0%)       +6 B  (+0.0%)
gutenberg-editor        +40 B  (+0.0%)       +8 B  (+0.0%)
domains                 +40 B  (+0.0%)       +8 B  (+0.0%)
checkout                +40 B  (+0.0%)       +8 B  (+0.0%)
accept-invite           +40 B  (+0.0%)       +8 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 (~59 bytes added 📈 [gzipped])

name                                  parsed_size           gzip_size
async-load-signup-steps-plans              +140 B  (+0.0%)      +51 B  (+0.1%)
async-load-calypso-blocks-app-banner        +40 B  (+0.0%)       +8 B  (+0.0%)

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.

matticbot avatar Sep 22 '22 15:09 matticbot

The changes aren't working for me when I navigate to /start on the live branch while logged in, select a free */wordpress.com subdomain, and then select the monthly eCommerce plan. My site gets created with the pub/zoologist theme.

Seeing the same thing. In my testing it's because the slug is ecommerce-bundle-monthly, which the check for ecommerce-bundle doesn't cover. Presumably the isEcommerce() would fix.

mreishus avatar Sep 27 '22 21:09 mreishus

  • Changed direct slug check to use isEcommerce()
    • In my testing, this now correctly detects both monthly and annual eCommerce, and other plans like personal continue to fail the check as expected
  • Opened up to all flows
    • I don't know how to test these other flows. But the modular nature of the the steps gives me somewhat higher confidence of it working.

mreishus avatar Sep 27 '22 22:09 mreishus

The basic functionality worked as expected for me. Trying to see about testing other flows now.

markbiek avatar Sep 28 '22 14:09 markbiek

I think the Newsletter flow worked as intended but I hadn't been through that one before.

I started at calypso.localhost:3000/setup/?flow=newsletter, went through the basic Newsletter creation process and ended up with a site set to the Lettre theme (which appears to be correct for newsletters).

I also went through the Anchor-FM flow and that also worked as expected. My Anchor site had the theme I selected during setup.

markbiek avatar Sep 28 '22 14:09 markbiek

I wasn't able to ship this because it fails the pre-release e2e tests. I'll have to investigate the failure and possibly update the tests. Reverted in #68406.

mreishus avatar Sep 28 '22 14:09 mreishus

Reimplemented in #68411

mreishus avatar Sep 28 '22 18:09 mreishus