-
Notifications
You must be signed in to change notification settings - Fork 2k
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: Disable the Global Styles limit when live previewing Woo and Premium themes #84924
Conversation
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. |
apps/editing-toolkit/editing-toolkit-plugin/wpcom-global-styles/index.php
Outdated
Show resolved
Hide resolved
array_filter( | ||
$software_sets, | ||
function ( $software_set ) { | ||
return $software_set->product_slug === 'woo-on-plans'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update to check woo-on-plans
here as well? 🙇 : https://github.com/Automattic/wp-calypso/blob/fba40b1/apps/wpcom-block-editor/src/wpcom/features/live-preview/hooks/use-previewing-theme.ts#L13
I realized that would be safer now (but might be changed to support bundle themes).
#84483 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'm on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits comments 👍
Thanks for working on this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice solution! Tested:
-
Upgrade Notice for Premium themes
- Prepare a simple site with the Personal plan or lower plan.
- Sandbox that site
- Access https:///wp-admin/site-editor.php?wp_theme_preview= to preview a premium theme (e.g., premium/outland).
- ✅ Change global styles and expect to see only the upgrade notice "You are previewing..."
- ✅ Repeat the test on atomic uploading the ETK plugin for this branch
-
Upgrade Notice for Woo themes
- Prepare a simple site with the Personal plan or lower plan.
- Sandbox that site
- Access https:///wp-admin/site-editor.php?wp_theme_preview= to preview a premium theme (e.g., premium/tsubaki).
- ✅ Change global styles and expect to see only the upgrade notice "You are previewing..."
- ✅ Repeat the test on atomic uploading the ETK plugin for this branch
@@ -62,6 +62,16 @@ function wpcom_should_limit_global_styles( $blog_id = 0 ) { | |||
return false; | |||
} | |||
|
|||
// Do not limit Global Styles when live previewing a Woo theme without a Business plan or higher. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's simpler to just check for "Do not limit Global Styles when live previewing a premium theme, and the site has a premium plan or higher", since we only want to know about these conditions in particular:
- Is live previewing.
- Is premium theme.
wpcom_site_has_feature( WPCOM_Features::GLOBAL_STYLES, $blog_id )
istrue
.
This way we don't need to check whether the premium theme is Premium or WooCommerce 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I have implemented that logic but struggled while testing it because wpcom_site_has_feature( WPCOM_Features::GLOBAL_STYLES, $blog_id )
always returns false
when a sandboxed site has a premium plan or higher.
I've asked T-Rex about that in p1701937616146419-slack-C04DZ8M0GHW
… theme with a Premium plan or higher
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
I updated the code to make it simpler but won't work when testing on our sandbox. I vote for deploying and testing it on production. Thoughts? |
apps/editing-toolkit/editing-toolkit-plugin/wpcom-global-styles/index.php
Outdated
Show resolved
Hide resolved
apps/editing-toolkit/editing-toolkit-plugin/wpcom-global-styles/index.php
Show resolved
Hide resolved
Yes, let's deploy 🚢 |
…s/index.php Co-authored-by: Griffith Chen <griffith.chen@automattic.com>
1b3b86c
to
e093b46
Compare
It's working fine 😍 |
…o and Premium themes (#84924) * Disable Global Styles limit for live previewing woo and premium themes on Dotcom * Simplify by not limiting Global Styles when live previewing a Premium theme with a Premium plan or higher * Update apps/editing-toolkit/editing-toolkit-plugin/wpcom-global-styles/index.php Co-authored-by: Griffith Chen <griffith.chen@automattic.com> * Update naming and comments to be more clear --------- Co-authored-by: Griffith Chen <griffith.chen@automattic.com>
Related to #79223 #83630 #84676
Closes #84899
Proposed Changes
BTW. I reported a minor issue on the copy of the Styles Welcome Guide that has been fixed.
Testing Instructions
In your sandbox run
install-plugin.sh etk fix/live-preview-unique-notice
Upgrade Notice for Premium themes
https://<your-site>/wp-admin/site-editor.php?wp_theme_preview=<premium-theme>
to preview a premium theme (e.g., premium/outland).Screen.Recording.2566-12-06.at.20.11.47.mov
Upgrade Notice for Woo themes
Screen.Recording.2566-12-06.at.20.13.25.mov
Pre-merge Checklist