Update theme tier badges in Theme Details and Theme Showcase (LiTS & LoTS) #98451
Conversation
abotic
added
[Status] Needs Review
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~149 bytes removed 📉 [gzipped]) Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~2622 bytes removed 📉 [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 (~1138 bytes removed 📉 [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. |
There was a problem hiding this comment.
I'm having a go at reviewing this based on spending some time implementing the new designs mentioned here.
These changes look great so far when comparing them to the above designs, including all the different tiers.
One potential change I noticed is that I believe any free themes should always show a "free" badge, rather than the "included with plan" badge, e.g. TT5 and LeanCV in this screenshot:
|
I'd also like to highlight this comment from @jeryj:
I'm wondering if that should be part of this PR, or maybe it could be done in a separate branch? |
|
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 |
Thanks for taking a look @mikachan ! I was thinking about this exact point - the "Free" vs "Included" badge choice for Free themes. The reasoning behind "Included" badge that I went with was just to help users better appreciate the whole package when they are on a Personal+ plan, even if some themes would be free anyway. But you raise a really good point about clarity. What's your take on which would be more intuitive for users? |
Thanks for bringing this up. It's a great point. I think it's worthwhile to delineate between the free themes and what's included in your plan. I support making that change! |
@mikachan Thanks for flagging this. I agree this should be handled in a separate PR to keep the current changes focused. Also, found a bug with community theme badges that needs fixing in some underlying code and state. I'll have it fixed in the next commit and we can do another round of testing/review if possible. |
|
Thanks for putting this PR together, it's looking great! This might still be in progress but just wanted to point out that when I look at partner themes on a business site, it's saying they're included in my plan: 
In these cases, can we show the monthly price: |
|
hey folks, just checking in to see how this is going and if there's anything I can do to unblock it if we're blocked? |
Hi @fditrapani , I was blocked on other task that took some time to resolve. Jumping on this one today and will try to get it ready as soon as possible. |
|
Awesome. Thanks for the update @abotic! |
- Fix partner theme price display logic for business plan - Add proper theme bundle/partner price handling - Refactor components with proper memoization and selector optimizations - Clean up unused imports and tooltip logic - Add proper equality checks to prevent unnecessary rerenders - Fix theme upgrade/included badge conditional rendering
- Fix partner theme price display logic for business plan - Add proper theme bundle/partner price handling - Refactor components with proper memoization and selector optimizations - Clean up unused imports and tooltip logic - Add proper equality checks to prevent unnecessary rerenders - Fix theme upgrade/included badge conditional rendering
|
Hi @fditrapani and @mikachan 👋 Thank you for your comments, I've made some changes and it is ready for review. Recent changes:
Updated testing instructions/table and added new video showing free and business plan showcase (Personal and Premium follow same pattern). Looking forward to your feedback. |
|
Thanks for making these updates @abotic! It's getting really close. I'll let you decide whether you want to merge this and work on the following tweaks as a follow up: Upgrade badge font 
here's how it should look: Remove partner badge 
and the theme details page: The same goes for the WooCommerce badge: We have a filter for that, so labelling it isn't as necessary. |
|
Is it expected that badges are not consistent between the logged-in Theme Showcase and the onboarding's Design Picker?
|
|
Good catch @mmtr. That looks like a regression. They should be the same. |
|
@abotic, popping this back out of Ready To merge for the inconsistency to be looked at |
|
@fditrapani @mmtr Same styling applied on onboarding as well. Thanks for feedback! Please note that there was one minor regression issue with plans that had pricing shown near them. We could change the font, apply some margins while being onboarded or something else, but changing the "plus" to "+" seemed to me like a best option to keep exactly same styling between onboarding and logged-in theme picker. If this is not okay, please let me know some other idea we could apply here. Example:
|
|
Thanks for the updates. We can save even more space, for this specific case, if we update it to "On Business + $10.00/month" |
|
Maybe a design nitpick, but I noted that the badges in the Design Picker look bigger now because the font-size has been increased to 14px: 
Currently, in production, the font-size is set to 12px which also matches the font-size used in the Figma design I can find in p9Jlb4-fea-p2. This makes me think that the font-size increase has not been intentional. |
|
Community themes also show a weird layout, is this expected? |
|
Another issue is that the theme name may be hidden by badges:
|
|
Thanks for bringing up these extra issue @mmtr!
Great catch. As you noted, this is impacting the space we have to display the theme name.
Given that we have a community filter. This badge is unclear. Can we remove it since we show it on the theme details page: 
Likewise with the Sensei and other badges. We're trying to do too much on this screen all at once. People can find these types of themes with a search or the filters if they're looking for them. Otherwise, they don't really mean much. |
|
@mmtr @fditrapani Thanks for great regression testing! That includes issue with a fonts, overlapping of elements and extra badges. We have now removed extra badges (community, bundle, WooCommerce) from theme showcase, but they are still available in theme details. Please let me know if you find anything else. |
|
Thanks for these updates @abotic. We're almost there! I did one final pass on everything across the 
Can we show it as regular text with the price like all the others: The same on the theme details, we only show the price in the dark pill: Can we add a star in there like all the other upgrades and add some text to not make it feel so transactional since we have the space :) |
|
@fditrapani Everything you proposed regarding pricing has been implemented and pushed. Let me know if there's anything else you would like to change/improve, and I can jump on it immediately :) |
There was a problem hiding this comment.
Thank you for all the updates @abotic! This looks great now.
| isThemePurchased( state, themeId, siteId ) | ||
| const themeType = useSelector( | ||
| ( state ) => getThemeType( state, themeId ), | ||
| ( prev, next ) => prev === next |
There was a problem hiding this comment.
Is this needed? I always thought this was the default behavior:
useSelector()only forces a re-render if the selector result appears to be different than the last result. The default comparison is a strict===reference comparison.
– https://react-redux.js.org/api/hooks#equality-comparisons-and-updates
There was a problem hiding this comment.
Yes, you're absolutely right! The custom equality function (prev, next) => prev === next is redundant since it's identical to useSelector's default behavior.
I'll remove it and simplify to just:
const themeType = useSelector(state => getThemeType(state, themeId));
Thanks for catching this!
| const getTooltipMessage = () => { | ||
| if ( ! isEcommerceTrialMonthly ) { | ||
| return createInterpolateElement( | ||
| translate( | ||
| 'This community theme can only be installed if you have the <Link>%(businessPlanName)s plan</Link> or higher on your site.', | ||
| { args: { businessPlanName: getPlan( PLAN_BUSINESS )?.getTitle() ?? '' } } | ||
| ), | ||
| { | ||
| Link: <ThemeTierBadgeCheckoutLink plan={ planTooltipName } />, | ||
| } | ||
| ); | ||
| } | ||
| if ( isEcommerceTrialMonthly ) { | ||
| return createInterpolateElement( | ||
| translate( | ||
| "This theme can't be installed on a trial site. Please upgrade to the <Link>%(ecommercePlanName)s plan</Link> to install this theme.", | ||
| { args: { ecommercePlanName: getPlan( PLAN_ECOMMERCE )?.getTitle() ?? '' } } | ||
| ), | ||
| { | ||
| Link: <ThemeTierBadgeCheckoutLink plan={ planTooltipName } />, | ||
| } | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
This causes a confusing badge now for trial plans.
For instance, on a site with an Commerce trial plan, we now say the theme is available on the Business plan, but they cannot really buy that plan (they can only buy the Commerce non-trial plan):
| Before | After |
|---|---|
 |
 |
On these cases, we should say "Available on the Commerce plan" instead of "Available on the Business plan".
| if ( isPartnerThemePurchased && ! isThemeAllowed && ! isEcommerceTrialMonthly ) { | ||
| return createInterpolateElement( | ||
| translate( | ||
| 'You have a subscription for this theme, but it will only be usable if you have the <link>%(businessPlanName)s plan</link> or higher on your site.', | ||
| { args: { businessPlanName: getPlan( PLAN_BUSINESS )?.getTitle() ?? '' } } | ||
| ), | ||
| { | ||
| Link: <ThemeTierBadgeCheckoutLink plan={ planTooltipName } />, | ||
| } | ||
| const priceBadge = useMemo( () => { | ||
| if ( ! isThemeAllowed ) { | ||
| return ( | ||
| <ThemeTierPlanUpgradeBadge | ||
| showPartnerPrice={ showPartnerPrice } | ||
| hideBackgroundOnUpgrade={ hideBackgroundOnUpgrade } | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| if ( isPartnerThemePurchased && ! isThemeAllowed && isEcommerceTrialMonthly ) { | ||
| return createInterpolateElement( | ||
| translate( | ||
| "You have a subscription for this theme, but it isn't usable on a trial plan site. Please upgrade to the <link>%(ecommercePlanName)s plan</link> to install this theme.", | ||
| { args: { ecommercePlanName: getPlan( PLAN_ECOMMERCE )?.getTitle() ?? '' } } | ||
| ), | ||
| { | ||
| Link: <ThemeTierBadgeCheckoutLink plan={ planTooltipName } />, | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Same for partner themes and Commerce trial sites. We cannot ask them to buy the Business plan, because their only option is the Commerce plan:
| Before | After |
|---|---|
 |
 |
| return fullLabel.length > MAX_LABEL_LENGTH | ||
| ? /* translators: This is a shorter version of the text "Available on %(planName)s plus %(price)s/month". */ |
There was a problem hiding this comment.
We should keep this comment since it's helpful for translators.
| <PremiumBadge | ||
| className="theme-tier-badge__content is-third-party" | ||
| focusOnShow={ false } | ||
| isClickable={ false } | ||
| labelText={ translate( 'Partner' ) } | ||
| shouldHideIcon | ||
| tooltipClassName="theme-tier-badge-tooltip" | ||
| tooltipContent={ partnerTooltipContent } | ||
| tooltipPosition="top" | ||
| /> |
There was a problem hiding this comment.
Is it expected that the "Partner" badge is now not displayed in the theme details view?
| Before | After |
|---|---|
 |
 |
Asking because this is inconsistent with the "Community" or "WooCommerce" badges:
| Before | After |
|---|---|
 |
 |
 |
 |
| @@ -472,6 +472,7 @@ const UnifiedDesignPickerStep: Step = ( { navigation, flow, stepName } ) => { | |||
| const getBadge = ( themeId: string, isLockedStyleVariation: boolean ) => ( | |||
| <ThemeTierBadge | |||
| canGoToCheckout={ false } | |||
| isThemeShowcase | |||
There was a problem hiding this comment.
Nitpick (non-blocking): This is a bit confusing, because the unified-design-picker.tsx file is part of the Design Picker, not the Theme Showcase.
What are you trying to achieve when isThemeShowcase is true? It seems that the intention is to support two different styles, one for when there is a list of themes and another one for when only one theme is visible.
Maybe you could rename the prop to something else like isSingleTheme or isThemeList, since that's generic enough for both the Design Picker (/setup/onboarding/designSetup) and the Theme Showcase (/themes).
Related to:
Proposed Changes
Freebadge for free themes when user is not logged inIncluded with planbadge for themes available in user's current planAvailable on {plan name}for themes requiring higher tier plansAvailable on Businessfor WooCommerce themesAvailable on Business (and price €X.XX/month where applicable)for partner themesBundledandCommunitythemesWhy are these changes being made?
Currently, the theme details page doesn't clearly indicate which plan is required for themes, leading to user confusion. This update improves clarity by:
Testing Instructions
before.mov
after.mov
Pre-merge Checklist