-
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
New Checkout: split layout (take 2) #41373
Conversation
- Move CheckoutStepBody inside CheckoutSteps - Split real steps from not - Move `checkout-content` back to Checkout component - Update loading - Update demo
- Remove props from Checkout Summary so that component just renders children - Update demo and defaults - Fix summary height when split into columns
- adds the component and shifts the step wrapper styles to it - removes CheckoutSteps children splitting logic - moves CheckoutSteps (and CheckoutStep children) and CheckoutStepBody to new component in default Checkout
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~1135 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. 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. |
<CheckoutSummaryFeaturesList> | ||
<CheckoutSummaryFeaturesListItem> | ||
<WPCheckoutCheckIcon /> | ||
{ translate( 'Live chat and email support' ) } |
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 29 times:
translate( 'Email and live chat support' )
ES Score: 9
See 2 additional suggestions in the PR translation status page
</CheckoutSummaryFeaturesListItem> | ||
<CheckoutSummaryFeaturesListItem> | ||
<WPCheckoutCheckIcon /> | ||
{ translate( 'Dozens of free 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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 19 times:
translate( 'Dozens of free themes.' )
ES Score: 15
See 2 additional suggestions in the PR translation status page
</CheckoutSummaryFeaturesListItem> | ||
<CheckoutSummaryFeaturesListItem> | ||
<WPCheckoutCheckIcon /> | ||
{ translate( 'Money back guarantee' ) } |
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( '30-day Money Back Guarantee' )
ES Score: 9
This is looking great! the immediate thing that pops out is that the benefits in the right column are static and not reflecting the products in the cart. This is what I get for a domain only: and this is the business plan: If we want to start with static, that's ok but we'll need to make sure the benefits are generic enough for all our products. ==== I don't know if this is a regression but I'm upgrading my plan and it's defaulting to the two year length without giving me the choice of switching back to the one year. You can see it in the screenshot above. |
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! Tested and it appears to work correctly.
@fditrapani: I did this to get it out the door. The most generic are the support and money back points.
We talked about this in Slack, but it's the current Checkout behavior to default to the variation that you currently have selected. I agree that it'd be great to show the other available variations too. I'll look into what'd be required for that. |
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/3349298 Thank you @michaeldcain for including a screenshot in the description! This is really helpful for our translators. |
Translation for this Pull Request has now been finished. |
Originally attempted in #41063 and reverted in #41363
This does a few things with the ultimate goal of splitting out a summary into a separate column on desktop screens.
CheckoutSummary
component to composite-checkout (package, demo, documentation)CheckoutStepArea
component to handle styling ofCheckoutSteps
+CheckoutStepBody
CheckoutSummary
in WPcom's implementation of composite-checkoutCheckoutSummary
on mobile screens (collapse mobile toggle coming in future PR)Fixes: #41088
To test:
yarn run composite-checkout-demo
?flags=composite-checkout-testing
>960px
), the checkout summary is displayed, floated to the right, with a list of default features, and the tax (once calculated) and total