-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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
Entrepreneur Signup: Simplify final step in CYS when entering from Entrepreneur signup flow. #47578
Entrepreneur Signup: Simplify final step in CYS when entering from Entrepreneur signup flow. #47578
Conversation
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
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.
Overall the changes look good. Nice! 👍🏼 There are two regressions though - but easy to fix (please see the inline comments). 🙂
plugins/woocommerce-admin/client/customize-store/transitional/index.tsx
Outdated
Show resolved
Hide resolved
plugins/woocommerce-admin/client/customize-store/transitional/index.tsx
Outdated
Show resolved
Hide resolved
baf746b
to
f49240d
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.
Nice! The changes look good and work as exppected! Perfect! One tiny thing I noticed - related to translations.
? __( | ||
"Congratuations! You've successfully designed your store. Now you can go back to the Home screen to complete your store setup and start selling.", | ||
'woocommerce' | ||
) |
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.
Looking at the translations, they don't exist for this new string yet. I think we could consider using the existing one until the new one is ready.
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 is fine because Woo 9.0 will only be shipped 3-4 weeks from now. Just need to make it to the code freeze first.
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 is fine because Woo 9.0 will only be shipped 3-4 weeks from now. Just need to make it to the code freeze first.
Oh, I see. Makes sense! With knowing that, I think the PR is good to go. 🚀
…index.tsx Co-authored-by: Ivan Ottinger <ivan.ottinger@automattic.com>
a513d08
to
d461aae
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.
LGTM! 🚀
Submission Review Guidelines:
Changes proposed in this Pull Request:
Simplify final step in CYS when entering from Entrepreneur signup flow.
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
your_site_here.wpcomstaging.com/wp-admin/admin.php?page=wc-admin&path=%2Fcustomize-store%2Fdesign-with-ai&ref=entrepreneur-signup
.Changelog entry
Significance
Type
Message
Comment