-
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
Congrats: Update design for ecommerce plan #79525
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~168 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. |
7eeaa76
to
a810eb8
Compare
client/my-sites/checkout/checkout-thank-you/standard-checkout-thank-you.tsx
Outdated
Show resolved
Hide resolved
This PR modifies the release build for happy-blocks To test your changes on WordPress.com, run To deploy your changes after merging, see the documentation: PCYsg-r7r-p2 |
@@ -542,17 +539,31 @@ export class CheckoutThankYou extends Component< | |||
); | |||
} | |||
|
|||
// Keep the AtomicStoreThankYouCard for the Woo Mobile App | |||
if ( isWcMobileApp() ) { |
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.
This doesn't look safe to me, AtomicStoreThankYouCard
is doing email verification and sets the CTA url to siteWooCommerceWizardUrl
, can we wrap StandardCheckoutThankYou
inside AtomicStoreThankYouCard
and keep the same behavior?
We're also losing the "Create your store!" CTA text. "Lets work on the site" doesn't sound right for the ecommerce plan.
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.
This PR also sets the CTA to the same URL as siteWooCommerceWizardUrl
.
As for email verification, I tested, and with the current flow, the user will get redirected to another email verification page when clicking on the CTA siteWooCommerceWizardUrl
Once they verify their email and click on connect jetpack, they will go to the store setup page
Summarizing the flow for unverified emails:
How this PR currently changes the flow:
- Thank You page
- Click on CTA
- Verify Email
- Connect Jetpack (this might be the confusing step)
- Start setting up the store
The current production flow:
- Thank You card
- Verify Email
- Click on CTA
- Start setting up the store
Any ideas on how we could incorporate the old flow into the new design
cc: @JanaMW27
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.
How this PR currently changes the flow:
Thank You page
Click on CTA
Verify Email
Connect Jetpack (this might be the confusing step)
Start setting up the storeThe current production flow:
Thank You card
Verify Email
Click on CTA
Start setting up the store
How did we add the Jetpack connection step?
Any ideas on how we could incorporate the old flow into the new design
I'd need to dig in to really judge this but I think the new design needs to support the old design's functionality other wise the only option is to revert to the "new" thank you component approach and let them control the flow and we just control the basic design outline.
I guess you could change something higher up to but I don't know how that would work.
What are you thinking?
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.
Oh, also this option looked ok if I'm remembering it correctly:
can we wrap StandardCheckoutThankYou inside AtomicStoreThankYouCard and keep the same behavior?
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.
How did we add the Jetpack connection step?
I didn't add anything in particular. It was... just there 😅 when I tested the flow with the PR.
Maybe it would be helpful if we have more people to test the flow within this PR to confirm.
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.
What are you thinking?
I was only taking account of how things "look" right now. For example, I'm not sure where would be the sweet spot to add the resend email button or add the copy. Asked for more input here: p9Jlb4-7Hx-p2#comment-8670
I agree that code-wise it's not going to be too far from:
wrap StandardCheckoutThankYou inside AtomicStoreThankYouCard and keep the same behavior?
Even if we create a new component, it's going to copy that behavior.
Also related to Automattic/wc-calypso-bridge#1252 |
We can remove the part for |
Closing this PR as @Automattic/serenity has taken over. |
Related to https://github.com/Automattic/dotcom-forge/issues/2723
Proposed Changes
Testing Instructions
/plans
pagePre-merge Checklist