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 monthly pricing option #48963
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~1661 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. 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. |
Update on the progress so far: Done
Progress proceeded more slowly than expected, as some parts of the UI had to be refactored to allow for different outcomes depending on the billing period (monthly vs annually) What's left
|
aria-label={ | ||
billingInterval === 'ANNUALLY' | ||
? __( 'Included with annual plans', __i18n_text_domain__ ) | ||
: __( 'Only included with annual plans', __i18n_text_domain__ ) |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 17 times:
translate( 'Included with annual plans' )
ES Score: 9
21d9c57
to
2f76ed0
Compare
Rebased on top of latest |
{ ! isFree && | ||
( billingInterval === 'ANNUALLY' | ||
? sprintf( | ||
__( 'per month, billed as %s annually', __i18n_text_domain__ ), |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 3 times:
translate( 'per month{{br/}}billed yearly' )
ES Score: 7
> | ||
{ sprintf( | ||
// Translators: "%s" is a number, and "%%" is the percent sign. Please keep the "%s%%" string unchanged when translating. | ||
__( 'Save %s%% by paying annually', __i18n_text_domain__ ), |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 17 times:
translate( 'Save %(discountRate)s%% by paying annually' )
ES Score: 8
Hey @ollierozdarz , we’ve been working on the UI changes to enable monthly plans in Gutenboarding (and Focused Launch and Step by Step Launch). So far we followed the “Onboarding” and “Launch flow” design specs on Figma, but in a few cases we had to go with our own judgment — mainly because of some features that are supported in the reskinned signup & launch flows. One thing to keep in mind is that the “Plans Grid” package is re-used across different flows (gutenboarding, step by step launch, and focused launch), and so the changes that we make are going to be applied across all these flows. We would like you to visit the staging link and give some early feedback on this version of the plans grid in Gutenboarding. In particular, these are some items that we’d like your feedback on:
As a caveat, please keep in mind that at the moment the data for monthly plans has not been integrated yet, so please ignore the figures (prices and discounts) about monthly plans during your review (you can actually see the complete list of TODO items at the top of this page). Here's also a screen capture, in case the staging link doesn't work for you: monthly-plans-grid.mp4 |
Hi @ciampo, thanks for the ping. First of all, great job on this! 👏 Rick has asked me to take a look at giving this specific page some more love in the near future (specifically the plans accordion), so I'm not keen to change much here now, and I think this is nearly at a state where it's at parity to ship. I did notice a few small things that could be improved and listed them below: For the note above, I never specified this in my refined reskin onboarding Figma files (my bad! 🙂), but I think when this link is active we should underline it to make it stand out more (i.e. the user is able to select a URL since they have Annual toggled and the correct plan). For the After screenshot, I've just aligned the checks and crosses with the feature label (for consistency) and added more bottom margin to the 'Included with annual plans', since they were sitting very close to the text below. While we're here, we may as well fix the border-radius for the primary container (it should be using the same [looks like 5px] on all four corners). |
Great work on this! I'd like to add some further feedback:
|
Thank you @ollierozdarz and @paulbonahora for your feedback! Here's some quick notes in response to @ollierozdarz 's feedback:
And here's some quick notes in response to @paulbonahora 's feedback:
|
a9f4253
to
451a00a
Compare
Update:
|
1d4e0e4
to
61f1d38
Compare
Update:
|
4dff4e3
to
d3d8961
Compare
Update:
|
Update:
|
…annually - refactor all calls to LaunchStore into one useSelect - get the plan product from the PlanStore, useful to get the slug and billingPeriod - show only the effective features that come with the selected plan - fix tick icon sizing (it would shrink on small viewports)
This prevents that text from overlapping with action buttons in step by step flow
It was deleted by mistake during a rebase
0ea59ec
to
f7487a8
Compare
Rename "doesFeatureRequireAnnuallyBilledPlan" to "featureRequiresAnnual" Fix typo: Monhtly => Monthly Refactor maxMonthlyDiscountPercentage truthy check (+ typo) Refactor maxMonthlyDiscountPercentage truthy check Typo
Fix: wrap { disabledLabel > inside a React fragment to avoid render errors Use strings as text nodes instead of react fragments
Fix translations: use same strings as /start for plans toggle Fix translations: use same strings as /start form "SAVE X% BY PAYING ANUALLY" Add "Domain Registration" string in step by step launch final step Fix translations: "per month, billed as %s annually" => "billed annually" This is because there are already translations matching what's in /start Fix translations: use double percent sign to avoid sprintf errors
… plans are selected Translations fix: remove "billed yearly" from bottom of details plan Translations fix: remove (billed yearly) from comparison table only for monthly plans
f7487a8
to
948c222
Compare
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.
Tested /new
in EN and FR and it looks good. I guess we'll re-test the ET before the next version release.
🚢
It looks like there's confusion with the wording on /new/plans page at the very bottom of the page when the yearly option is selected. See screenshot here. I'd suggest replacing the wording "Monthly Subscription (billed yearly)" to "Monthly Price (billed yearly)" for clarity. There's no change needed when the toggle is set to monthly. cc: @simison |
__i18n_text_domain__ | ||
), | ||
maxSavingsPerc | ||
{ maxDiscount: maxMonthlyDiscountPercentage ?? 0 } |
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.
Seems like we're checking for maxMonthlyDiscountPercentage
above, so it will never be null
/ undefined
, therefore there should be no need for the nullish coalescing I think?
Changes proposed in this Pull Request
<PlansIntervalToggle>
component to the Plans Grid UITesting instructions
Setup
Test in Gutenboarding
yarn && yarn start
in the root folder of this repocalyspo.localhost:3000/new
and navigate to the Plan stepTest in Focused Launch Flow
/start
to your sandboxyarn && yarn start
in the root foldercd apps/editing-toolkit && yarn dev --sync
calyspo.localhost:3000/page/UNLAUNCHED_SITE_CREATED_WITH_START.wordpress.com/home?flags=create/focused-launch-flow
Test in Step By Step Launch Flow
/new
and on a free plan to your sandboxyarn && yarn start
in the root foldercd apps/editing-toolkit && yarn dev --sync
calyspo.localhost:3000/page/UNLAUNCHED_SITE_CREATED_WITH_NEW .wordpress.com/home
Fixes #48760
Fixes #49027