-
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
Domains: Create 100-Year Domain flow #95081
Conversation
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 (~278 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~6674 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 (~523 bytes added 📈 [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. |
3d6b3fa
to
b647a40
Compare
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 |
b647a40
to
58a5532
Compare
84fdc35
to
3fe77e0
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.
Read through the whole thing and left a few small comments. Didn't see any issues. Going to build and test locally now :)
client/landing/stepper/declarative-flow/internals/steps-repository/domains/index.tsx
Outdated
Show resolved
Hide resolved
client/landing/stepper/declarative-flow/internals/steps-repository/domains/style.scss
Outdated
Show resolved
Hide resolved
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.
The css of the domain purchase flow changed recently, if possible, I'd suggest matching that layout for consistency.
Other than that, I tested through this fully and it's working great, a very clean update:
- I did have some weirdness pressing the back button from in the cart, going back to the "Setting up your world" screen and couldn't escape it
- I left a few other comments above, mostly just nits :) Great work, I'm super excited to see the kinds of websites that sign up for 100 years!
36d77ee
to
97af23f
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.
Left one comment regarding a small css issue, but otherwise everything I tested and the code both look really great :)
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.
I'm not certain where best to comment, I think it's this file :) Everything I tested and looked at is working perfectly, it looks really great on all screen sizes. I just found one small inconsistency, a margin of 20px on the left at the 600px breakpoint, classes [.domains .step-container]
100-year-domain_left-margin.mov
That's a great job @leonardost, thanks for this PR! I have only a couple of comments:
QyzGiQ.mp4 |
@@ -1514,6 +1524,7 @@ class RegisterDomainStep extends Component { | |||
this.props.onAddDomain( suggestion, position, previousState ); | |||
} | |||
|
|||
this.setState( { pendingCheckSuggestion: suggestion } ); |
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 was removed in #84171 but I'm not sure why. Without this, the "Select" domain button doesn't have a "busy" state when it's clicked.
@StevenPartridge @gius80 thanks for your reviews! I've fixed the CSS and (re)added the busy state for the "Select" button. I think I'll leave the |
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.
Thanks for the update @leonardost! Looks great now!
I found another potential issue but it's for the next phase of the project (checkout). I'll post it here just to as a note. We should prevent adding more multiple domains in the cart for this flow.
* Submit the domains step on checkout back button click. * Use a cookie to retrieve dependencies. * Use session storage to correctly persist domains data. * Limit persisted data to onboarding flow. * Clean up code. * Clean up code. * Fix the paid domain showing up again. * Rename DomainsDependencies and clear storage in all flows.
…0-Year domain flow
57c541b
to
d4e0192
Compare
This reverts commit 0949664.
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/16923542 Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday. Thank you @leonardost for including a screenshot in the description! This is really helpful for our translators. |
Translation for this Pull Request has now been finished. |
Proposed Changes
This diff creates the 100-Year Domain flow as described in p9Jlb4-e6E-p2.
In this flow, users are able to search and choose a .com/net/org domain.
Notes:
Screenshots
Why are these changes being made?
We want to offer 100-year domain registrations as a separate offer from the 100-year plan. More details in p9Jlb4-e6E-p2.
Testing Instructions
USE_STORE_SANDBOX
mode in your backend to prevent actual purchases/setup/hundred-year-domain
Try to test the flow as thoroughly as possible:
Pre-merge Checklist