-
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
Plans grid: Add premium themes for Starter in comparison mode #86861
Conversation
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 |
client/lib/plans/features-list.tsx
Outdated
: i18n.translate( 'Premium themes' ); | ||
const shouldShowInPersonalPlan = | ||
config.isEnabled( 'themes/tiers' ) && | ||
( ( localeSlug && config< string >( 'english_locales' ).includes( localeSlug ) ) || |
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.
The locale check was implemented to either show the old or new plan name depending on if it was translated for the locale (this was part of the general plan rename project).
Since this is behind a feature flag, and the translation is new. Do we need the locale check here?
I think we could just do:
const shouldShowInStarterPlan = config.isEnabled( 'themes/tiers' ) && isPersonalPlan( planSlug );
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 added it just in case we enable the flag in production before the translations have been completed (the plans grid is a critical screen where we need to be 100% sure that everything is translated).
i18n.hasTranslation( 'Switch between all of our premium design themes.' ); | ||
return shouldShowNewString | ||
? i18n.translate( | ||
'Unlimited access to all of our advanced premium themes, including designs specifically tailored for businesses.' | ||
) | ||
: i18n.translate( | ||
'Access to all of our advanced premium theme templates, including templates specifically tailored for businesses.' | ||
); | ||
? i18n.translate( 'Switch between all of our premium design themes.' ) | ||
: i18n.translate( 'Switch between a collection of premium design themes.' ); |
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.
Aren't both translations new (I just realized a few lines below this one that it's not the case 😆 )? Why do we need the shouldShowTranslation
check in this case?
I'm also not sold on the wording, premium design themes
sounds strange to me. In the Starter plan section we're just referring to them as Premium themes
. Would it make sense to use the same naming convention?
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.
the "premium design themes" wording has always been there....and it's always sounded weird to me. +1 to not repeating that
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.
Aren't both translations new (I just realized a few lines below this one that it's not the case 😆 )? Why do we need the
shouldShowTranslation
check in this case?
Not sure if you already answered yourself 😅, but to be sure: no, only one of the translations is new:
Switch between all of our premium design themes.
-> NEWSwitch between a collection of premium design themes.
-> EXISTING
I'm also not sold on the wording,
premium design themes
sounds strange to me.
I agree, but it's my understanding that @Automattic/martech controls the language used in the plans grid, and any change in the copy should be handled by them since the impact of such change should be carefully measured.
So that's why I chose to stick with the existing "premium design themes" terminology.
…n ETK and wpcom-block-editor
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.
Works as expected.
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/11095809 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 @mmtr for including a screenshot in the description! This is really helpful for our translators. |
This commit adjusts the "Premium themes" feature in the Plans grid so it can surface that the Starter plans has access to some premium themes (once we launch the Theme Tiers project, see paYJgx-4kX-p2). The approach taken here relies on the proposal that @niranjan-uma-shankar made in paYJgx-4vW-p2#comment-4600, in which the Starter plan has access to "Dozens of premium themes" and the Explorer plan and above has access to "Unlimited premium themes". It's my understanding that @Automattic/martech wants to be very careful about the features we display in the Plans grid, so this commit doesn't change the Starter plan in the general view. It only changes the table that's visible after clicking on the "Compare plans" button. It also slightly changes the feature name for the Explorer plan from "Premium themes" to "Unlimited premium themes" to better reflect that all premium themes can be accessed with that plan. From a technical standpoint, this PR also consolidates all the different constants used for the premium themes feature so it's easier to maintain them: - `WPCOM_FEATURES_PREMIUM_THEMES` has been renamed to `WPCOM_FEATURES_PREMIUM_THEMES_UNLIMITED` so it's clear that this feature has access to all the premium themes. - `FEATURE_PERSONAL_THEMES` has been renamed to `WPCOM_FEATURES_PREMIUM_THEMES_LIMITED` so it's clear that this feature is synced with `class-wpcom-features.php` (hence the `WPCOM_FEATURES_` prefix) and it's the one that has access to some premium themes. - `FEATURE_PREMIUM_THEMES_V2` has been removed, and all its usages have been replaced with `WPCOM_FEATURES_PREMIUM_THEMES_UNLIMITED`. - `FEATURE_PREMIUM_THEMES` now indicates the general feature that it's used in the comparison table, so all previous usages have been replaced with `WPCOM_FEATURES_PREMIUM_THEMES_UNLIMITED`.
I added a slight improvement in #86985 - it's still across two lines but avoids an orphan. |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~11 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~51 bytes added 📈 [gzipped])
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 (~53 bytes added 📈 [gzipped])
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. Generated by performance advisor bot at iscalypsofastyet.com. |
Translation for this Pull Request has now been finished. |
This commit adjusts the "Premium themes" feature in the Plans grid so it can surface that the Starter plans has access to some premium themes (once we launch the Theme Tiers project, see paYJgx-4kX-p2). The approach taken here relies on the proposal that @niranjan-uma-shankar made in paYJgx-4vW-p2#comment-4600, in which the Starter plan has access to "Dozens of premium themes" and the Explorer plan and above has access to "Unlimited premium themes". It's my understanding that @Automattic/martech wants to be very careful about the features we display in the Plans grid, so this commit doesn't change the Starter plan in the general view. It only changes the table that's visible after clicking on the "Compare plans" button. It also slightly changes the feature name for the Explorer plan from "Premium themes" to "Unlimited premium themes" to better reflect that all premium themes can be accessed with that plan. From a technical standpoint, this PR also consolidates all the different constants used for the premium themes feature so it's easier to maintain them: - `WPCOM_FEATURES_PREMIUM_THEMES` has been renamed to `WPCOM_FEATURES_PREMIUM_THEMES_UNLIMITED` so it's clear that this feature has access to all the premium themes. - `FEATURE_PERSONAL_THEMES` has been renamed to `WPCOM_FEATURES_PREMIUM_THEMES_LIMITED` so it's clear that this feature is synced with `class-wpcom-features.php` (hence the `WPCOM_FEATURES_` prefix) and it's the one that has access to some premium themes. - `FEATURE_PREMIUM_THEMES_V2` has been removed, and all its usages have been replaced with `WPCOM_FEATURES_PREMIUM_THEMES_UNLIMITED`. - `FEATURE_PREMIUM_THEMES` now indicates the general feature that it's used in the comparison table, so all previous usages have been replaced with `WPCOM_FEATURES_PREMIUM_THEMES_UNLIMITED`.
Proposed Changes
This PR adjusts the "Premium themes" feature in the Plans grid so it can surface that the Starter plans has access to some premium themes (once we launch the Theme Tiers project, see paYJgx-4kX-p2).
The approach taken here relies on the proposal that @niranjan-uma-shankar made in paYJgx-4vW-p2#comment-4600, in which the Starter plan has access to "Dozens of premium themes" and the Explorer plan and above has access to "Unlimited premium themes".
It's my understanding that @Automattic/martech wants to be very careful about the features we display in the Plans grid, so this PR doesn't change the Starter plan in the general view. It only changes the table that's visible after clicking on the "Compare plans" button:
It also slightly changes the feature name for the Explorer plan to better reflect that all premium themes can be accessed with that plan:
From a technical standpoint, this PR also consolidates all the different constants used for the premium themes feature so it's easier to maintain them:
WPCOM_FEATURES_PREMIUM_THEMES
has been renamed toWPCOM_FEATURES_PREMIUM_THEMES_UNLIMITED
so it's clear that this feature has access to all the premium themes.FEATURE_PERSONAL_THEMES
has been renamed toWPCOM_FEATURES_PREMIUM_THEMES_LIMITED
so it's clear that this feature is synced withclass-wpcom-features.php
(hence theWPCOM_FEATURES_
prefix) and it's the one that has access to some premium themes.FEATURE_PREMIUM_THEMES_V2
has been removed, and all its usages have been replaced withWPCOM_FEATURES_PREMIUM_THEMES_UNLIMITED
.FEATURE_PREMIUM_THEMES
now indicates the general feature that it's used in the comparison table, so all previous usages have been replaced withWPCOM_FEATURES_PREMIUM_THEMES_UNLIMITED
.Testing Instructions
?flags=theme/tiers
query param to the URL and reload the page.?flags=-theme/tiers
and reload the page.