Global Styles: Show notice in privacy settings #69242

rcrdortiz · 2022-10-19T08:18:13Z

This PR implements the UI changes necessary for displaying a notice in the privacy settings when our users are using Global Styles and are required to upgrade their plan in order to make the customization changes they've made available publicly.

I've decided not to show the notice when the site is not launched because of two main reasons. This isn't specified in the design and I think this wasn't taken into consideration, the second reason is to reduce friction when launching a site, I experimented a bit with the UI and it becomes cluttered with buttons and we aren't providing a clear path that our users should follow, which could lead to a decrease in the number of sites that get launched.

Also, when a user launches a free site, he will already get prompted to purchase a domain and a plan.

Proposed Changes

We now check if the user has GS in use and if we should show a notice.
If the user qualifies, we show a notice that includes a button so that the user can upgrade his plan.

Testing Instructions

MAKE SURE TO INSTALL THE ETK OF THIS BRANCH.
MAKE SURE YOU ADD THE STICKER BLOG TO THE SITES YOU'LL WORK WITH.

Happy Case

Go to the general settings screen, privacy section. http://calypso.localhost:3000/settings/general/{siteSlug}
Make sure your site is FREE and that you have some GS customizations applied.
- When the site is not launched you should not see a notice
- When the site is launched you should see a notice.
  - Make sure the buttons on the notice work as expected.
Remove or reset your customizations.
You should not see the notice.

Mobile devices

Repeat the happy case using a mobile viewport (just the part where you interact with the notice).

Test un-launched site

Create a site and make sure to not launch it.
Make sure it has the corresponding blog sticker.
Go to the general settings screen, privacy section.
You should not see the notice.

Test tracking event when clicking on the upgrade button.

Go through the Happy Case
Open tracks and search for calypso_global_styles_gating_%
You should see some events named calypso_global_styles_gating_settings_notice_upgrade_click

Test network error when fetching global styles info

Block the global styles info request in Chrome's inspector and reload.
You should see no console errors.
You should see no errors on the screen.
You should see the last value cached by useQuery.

Test for regressions

Accent color should continue to work as before. See this PR for testing instructions

Test evidence

Pre-merge Checklist

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

Related to #

…obal styles in use and does not have a paid plan

github-actions · 2022-10-19T08:18:30Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

matticbot · 2022-10-19T08:18:44Z

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

App Entrypoints (~3644 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
entry-stepper              +1186 B  (+0.0%)     +805 B  (+0.1%)
entry-main                  -197 B  (-0.0%)       +4 B  (+0.0%)
entry-gutenboarding         -103 B  (-0.0%)      +15 B  (+0.0%)
entry-login                  -63 B  (-0.0%)      +10 B  (+0.0%)
entry-domains-landing        -63 B  (-0.0%)      +10 B  (+0.0%)
entry-browsehappy            -63 B  (-0.1%)      +10 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~9473 bytes added 📈 [gzipped])

name                             parsed_size           gzip_size
settings                            +37861 B  (+5.5%)   +10678 B  (+5.2%)
plans                                 -248 B  (-0.0%)    -1293 B  (-0.5%)
jetpack-cloud-pricing                 -248 B  (-0.0%)    -1128 B  (-0.6%)
checkout                              -248 B  (-0.0%)     -886 B  (-0.2%)
jetpack-connect                       -150 B  (-0.0%)     -631 B  (-0.2%)
signup                                 -47 B  (-0.0%)     +168 B  (+0.2%)
accept-invite                          -47 B  (-0.0%)     +168 B  (+0.2%)
plugins                                -39 B  (-0.0%)      -37 B  (-0.0%)
woocommerce-installation               -30 B  (-0.0%)      -16 B  (-0.0%)
themes                                 -30 B  (-0.0%)      -16 B  (-0.0%)
theme                                  -30 B  (-0.0%)      -16 B  (-0.0%)
stats                                  -30 B  (-0.0%)      -18 B  (-0.0%)
sites-dashboard                        -30 B  (-0.0%)      -16 B  (-0.0%)
site-purchases                         -30 B  (-0.0%)      -17 B  (-0.0%)
settings-writing                       -30 B  (-0.0%)      -16 B  (-0.0%)
settings-security                      -30 B  (-0.0%)      -16 B  (-0.0%)
settings-performance                   -30 B  (-0.0%)      -16 B  (-0.0%)
settings-jetpack                       -30 B  (-0.0%)      -16 B  (-0.0%)
settings-discussion                    -30 B  (-0.0%)      -16 B  (-0.0%)
scan                                   -30 B  (-0.0%)      -16 B  (-0.0%)
purchases                              -30 B  (-0.0%)      -17 B  (-0.0%)
purchase-product                       -30 B  (-0.0%)      -16 B  (-0.0%)
posts-custom                           -30 B  (-0.0%)      -17 B  (-0.0%)
posts                                  -30 B  (-0.0%)      -17 B  (-0.0%)
people                                 -30 B  (-0.0%)      -16 B  (-0.0%)
migrate                                -30 B  (-0.0%)      -16 B  (-0.0%)
media                                  -30 B  (-0.0%)      -18 B  (-0.0%)
marketplace                            -30 B  (-0.0%)      -29 B  (-0.0%)
marketing                              -30 B  (-0.0%)      -16 B  (-0.0%)
jetpack-social                         -30 B  (-0.0%)      -16 B  (-0.0%)
jetpack-search                         -30 B  (-0.0%)      -16 B  (-0.0%)
jetpack-cloud-settings                 -30 B  (-0.0%)      -16 B  (-0.0%)
jetpack-cloud-plugin-management        -30 B  (-0.0%)      -16 B  (-0.0%)
jetpack-cloud-partner-portal           -30 B  (-0.0%)      -16 B  (-0.0%)
jetpack-cloud-agency-dashboard         -30 B  (-0.0%)      -16 B  (-0.0%)
jetpack-cloud                          -30 B  (-0.0%)      -16 B  (-0.0%)
hosting                                -30 B  (-0.0%)      -16 B  (-0.0%)
home                                   -30 B  (-0.0%)      -16 B  (-0.0%)
help                                   -30 B  (-0.0%)      -16 B  (-0.0%)
google-my-business                     -30 B  (-0.0%)      -16 B  (-0.0%)
email                                  -30 B  (-0.0%)      -16 B  (-0.0%)
earn                                   -30 B  (-0.0%)      -16 B  (-0.0%)
domains                                -30 B  (-0.0%)      -16 B  (-0.0%)
backup                                 -30 B  (-0.0%)      -16 B  (-0.0%)
add-ons                                -30 B  (-0.0%)      -16 B  (-0.0%)
activity                               -30 B  (-0.0%)      -16 B  (-0.0%)
pages                                  -22 B  (-0.0%)      -16 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 (~63 bytes removed 📉 [gzipped])

name                                                              parsed_size           gzip_size
async-load-masterbar-cart-masterbar-cart-wrapper                        -30 B  (-0.0%)      -16 B  (-0.0%)
async-load-design-playground                                            -30 B  (-0.0%)      -16 B  (-0.0%)
async-load-design-blocks                                                -30 B  (-0.0%)      -19 B  (-0.0%)
async-load-design                                                       -30 B  (-0.0%)      -16 B  (-0.0%)
async-load-calypso-reader-sidebar                                       -30 B  (-0.0%)      -16 B  (-0.0%)
async-load-calypso-post-editor-editor-media-modal                       -30 B  (-0.0%)      -16 B  (-0.0%)
async-load-calypso-my-sites-current-site-stale-cart-items-notice        -30 B  (-0.0%)      -16 B  (-0.1%)
async-load-calypso-my-sites-current-site-notice                         -30 B  (-0.0%)      -16 B  (-0.1%)
async-load-calypso-components-web-preview-component                     -30 B  (-0.0%)      -11 B  (-0.0%)
async-load-calypso-components-jetpack-sidebar                           -30 B  (-0.0%)      -16 B  (-0.0%)
async-load-calypso-blocks-jitm-templates-sidebar-banner                 -30 B  (-0.0%)      -16 B  (-0.1%)
async-load-calypso-blocks-jitm-templates-notice                         -30 B  (-0.0%)      -16 B  (-0.1%)
async-load-calypso-blocks-jitm-templates-default                        -30 B  (-0.0%)      -16 B  (-0.1%)
async-load-calypso-blocks-inline-help-popover                           -30 B  (-0.0%)      -16 B  (-0.0%)
async-load-calypso-blocks-editor-checkout-modal                         -30 B  (-0.0%)      -49 B  (-0.0%)
async-load-automattic-help-center                                       -30 B  (-0.0%)      -16 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.

…k in some pages

…the state of Global Styles Info

client/state/sites/hooks/use-premium-global-styles.ts

rcrdortiz · 2022-10-20T12:28:35Z

client/my-sites/site-settings/form-general.jsx

@@ -260,6 +261,12 @@ export class SiteSettingsFormGeneral extends Component {
 		} );
 	};

+	trackAdvancedCustomizationUpgradeClick = () => {
+		this.props.recordTracksEvent( 'calypso_global_styles_gating_settings_notice_upgrade_click', {


We'll need to add this event to Tracks.

matticbot · 2022-10-20T13:28:53Z

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit add/global-styles-notice-in-privacy-settings on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

mmtr

Nice work! Overall this tests very well, but I think we should address a few things before shipping.

You need to consider that the copy of the notice might be longer in other languages and wrapped in two lines. The layout is a bit off in such cases right now:

I think we should add the endpoint in a separate PR so we can deploy a new ETK version before deploying the Calypso changes that start fetching data from it
Looks like we make a recurrent request to the endpoint. That's not usually our approach when fetching data from an endpoint due to performance reasons (the only exception is when the fetched data is very critical and needs to be reflected in real time, but this not the case). The data should be loaded only once when the component first renders. I realize this can cause issues like the notice not showing up if you have the settings page open and are editing the Global Styles in a separate tab, but that's edge-case scenario and it's fine to don't cover it.

...toolkit/editing-toolkit-plugin/wpcom-global-styles/api/class-global-styles-info-rest-api.php

client/my-sites/site-settings/form-general.jsx

client/my-sites/site-settings/style.scss

client/state/sites/hooks/use-premium-global-styles.ts

Co-authored-by: Miguel Torres <miguel.torres@automattic.com>

rcrdortiz · 2022-10-21T13:00:07Z

I think we should add the endpoint in a separate PR so we can deploy a new ETK version before deploying the Calypso changes that start fetching data from it

No need to do this. useQuery will use the default values and not show the notice until the ETK endpoint is live, which shouldn't be a few minutes after at most.

Co-authored-by: Miguel Torres <miguel.torres@automattic.com>

rcrdortiz · 2022-10-21T13:17:10Z

Looks like we make a recurrent request to the endpoint. That's not usually our approach when fetching data from an endpoint due to performance reasons (the only exception is when the fetched data is very critical and needs to be reflected in real time, but this not the case). The data should be loaded only once when the component first renders. I realize this can cause issues like the notice not showing up if you have the settings page open and are editing the Global Styles in a separate tab, but that's edge-case scenario and it's fine to don't cover it.

Exactly how are you making it make more than one request? I'm unable to reproduce it. Maybe what you're experiencing is related to how useQuery handles state internally 🤔, with normal usage of the settings screen, I can't seem to reproduce it.

requests.mp4

mmtr · 2022-10-21T13:27:16Z

No need to do this. useQuery will use the default values and not show the notice until the ETK endpoint is live.

I got a WSOD if the endpoint is not live:

Exactly how are you making it make more than one request? I'm unable to reproduce it.

Oh sorry, I thought it was a recurring request, but apparently there is a new request every time I come back to the tab after switching to a different tab:

Screen.Recording.2022-10-21.at.15.21.08.mov

It's not a big deal, but I don't think that's the normal behavior we have in other components.

rcrdortiz · 2022-10-21T13:34:09Z

It's not a big deal, but I don't think that's the normal behavior we have in other components.

Do you happen to know how to avoid that? It has to be related to useQuery but I can't seem to find anything about it.

…cted when the translation is long

rcrdortiz · 2022-10-21T14:44:42Z

You need to consider that the copy of the notice might be longer in other languages and wrapped in two lines. The layout is a bit off in such cases right now

This should be working now for longer translated texts.

… siteSlug

mmtr

Thanks for addressing all the feedback!

Looks like there is a regression in the notices showed during onboarding.

For example, /setup/designSetup?siteSlug=<SITE_SLUG> used to display a premium badge on non-default variations but it's missing:

Also, when going to /setup/newsletterSetup?flow=newsletter there is a request to the API endpoint despite the site not being created. Where is that siteId coming from?

rcrdortiz · 2022-10-21T19:57:33Z

Thanks for addressing all the feedback!

Looks like there is a regression in the notices showed during onboarding.

For example, /setup/designSetup?siteSlug=<SITE_SLUG> used to display a premium badge on non-default variations but it's missing:

Also, when going to /setup/newsletterSetup?flow=newsletter there is a request to the API endpoint despite the site not being created. Where is that siteId coming from?

I can't reproduce the regression you mentioned. Could you share what steps you're following?

works.mp4

For the second thing you mention, there seems to be a bug with useSelector( getSelectedSiteId ); or with the actual newsletterSetup flow, the selected site isn't getting cleaned up from the store when navigating to /setup/newsletterSetup?flow=newsletter so the logic we have for determining the siteId is returning the last siteId (since that is what we have in the store). Is it possible to navigate to the newsletter flow from within Calypso? Or is this something that won't happen in real-life scenarios?

mmtr · 2022-10-24T08:36:49Z

I can't reproduce the regression you mentioned. Could you share what steps you're following?

I just created a new site, added the blog sticker and started the design step during the site setup process. However, I cannot reproduce it now, so maybe I was half-sandboxed, or maybe it was a caching issue. I guess it's fine to ignore for now.

Is it possible to navigate to the newsletter flow from within Calypso? Or is this something that won't happen in real-life scenarios?

No idea how the newsletter flow starts, but I think it's something that can happen in a real-time scenario since we cannot guarantee that the user didn't visit Calypso previously (even if the flows starts outside of Calypso).

I think the problem is that all these onboarding flows use a different way of storing site data and completely ignores what's in the traditional state. I wonder if there is any selector that tells us whether the user is onboarding so we can discard whatever getSelectedSiteId returns.

mmtr · 2022-10-24T09:06:38Z

I think the problem is that all these onboarding flows use a different way of storing site data and completely ignores what's in the traditional state. I wonder if there is any selector that tells us whether the user is onboarding so we can discard whatever getSelectedSiteId returns.

Maybe select( ONBOARD_STORE ).getState()?

…eId. I also added some protection agains a malformed API valid response

rcrdortiz · 2022-10-25T10:07:06Z

I think the problem is that all these onboarding flows use a different way of storing site data and completely ignores what's in the traditional state. I wonder if there is any selector that tells us whether the user is onboarding so we can discard whatever getSelectedSiteId returns.

Maybe select( ONBOARD_STORE ).getState()?

Thanks for the insight! It kinda worked, we also need to check for some other conditions like not having site query args. It should be working now.

mmtr

Pretty close!!

client/state/sites/hooks/use-premium-global-styles.ts

mmtr

Works great!

mmtr · 2022-10-25T13:04:33Z

client/my-sites/site-settings/form-general.jsx

+							{ translate( 'View site' ) }
+						</Button>
+						<Button
+							className="is-primary"


Nitpick (non-blocking): The same can be accomplished by setting a primary prop.

Suggested change

className="is-primary"

primary

a8ci18n · 2022-10-26T09:17:28Z

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

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

a8ci18n · 2022-11-03T16:30:44Z

Translation for this Pull Request has now been finished.

We now show a notice in the privacy setting screen if the user has gl…

585e556

…obal styles in use and does not have a paid plan

rcrdortiz changed the title ~~We now show a notice in the privacy setting screen if the user has gl…~~ Global Styles: Show notice in privacy settings Oct 19, 2022

mmtr and others added 3 commits October 19, 2022 10:49

Decouple usePremiumGlobalStyles from useLocation since it doesn't wor…

54b20c8

…k in some pages

We now fetch the status of global styles from the corresponding API

9dafa20

We now fetch the status of global styles from the corresponding API

47a6fbc

rcrdortiz changed the title ~~Global Styles: Show notice in privacy settings~~ [WIP] Global Styles: Show notice in privacy settings Oct 19, 2022

Updated usePremiumGlobalStyles so that it uses useQuery for managing …

f5c2065

…the state of Global Styles Info