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
Private by default launch flow #30242
Private by default launch flow #30242
Conversation
e05130e
to
82e9470
Compare
Ooops... I did something while rebase that closed this PR. Re-opening. |
I made some changes to the cart which mean you no longer to supply the site Id - you can just use the site slug. That might simplify this code a bit... |
client/lib/signup/step-actions.js
Outdated
addToCartAndProceed(); | ||
} | ||
} | ||
export function addDomainToCart( |
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 think we have several different ways of adding things to the cart. There's one for domains and one for plans. It would be good if they both used the same code.
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.
Please check 829ce5d. I've consolidated the common parts of the code in processItemCart()
. I still had to retain addPlanToCart()
and addDomainToCart()
as separate functions because they provide different dependencies. Does it align with your suggestion or am I missing something?
This is looking like it's heading in the right direction, great job :) |
There are a few places where the destination is not respected.
At least some of them are due to the destination not being respected, and could get fixed with the work happening in #29136 |
Thanks for all this work @niranjan-uma-shankar and @scruffian. It tested well with a free plan, paid plan plus subdomain, and paid plan plus a domain. Here are a couple notes:
These items can most likely be addressed in separate PRs:
cc @danhauk |
I agree. It would be great to have something more celebratory there. I'll work on something for that. |
…he framework. In the launch flow, if the site has a domain or a paid plan, then the corresponding step is skipped.
…iables with undefined values since they are not needed to submit the signup step. Outcome: code is cleaner
…licable to all steps. It also combines the functionality of submitQueryDependencies()
…ing excluded even if just one dependency was getting fulfilled. Introduced a change in which we exclude a step only if all of the dependencies it provides have been fulfilled 2. Added the isSiteTopicFulfilled callback to the site topic step of the preview flow
…flow. This should hopefully fix the failing e2e test (though hacky)
…l the vertical query dependency
778ce08
to
e68bd75
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.
Lets merge this and deal with the remaining issues in another PR. @niranjan-uma-shankar are you ok to create a new issue which compiles the remaining unaddressed feedback?
Happy to do it @scruffian. |
Great 🚢 |
This reverts commit cf78231.
Changes proposed in this Pull Request
site-launch
flow.This flow will show the domain step and the plan step before taking them to the preview page with a
successful launch banner:
Testing instructions
Please apply D23389-code and follow the testing instructions given there.
Scenario-1 - Launch a site that does not have either a domain or a plan
Launch Site
should go through the launch flow, showing both the domain and plan steps.There are a few scenarios that can be tested here:
Scenario-2 - Launch a site that does not have a domain but has a plan
Launch Site
should go through the launch flow, showing only the domain step. Test both with purchasing a domain and skipping Purchase - in both cases, it should take you to the Preview page with Success banner.Scenario-3 - Launch a site that has a domain but does not have a plan
Launch Site
should go through the launch flow, showing only the plan step. Test both with purchasing a plan and skipping Purchase - in both cases, it should take you to the Preview page with Success banner.Scenario-4 - Launch a site that has both a domain and a plan
Other tests
/start/subdomain?vertical=a8c.8
should show the default recommendation on the domain step to be a *.food.blog.