-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Move checkout forms to provider #343
Conversation
Signed-off-by: Loan Laux <loan@outgrow.io>
Signed-off-by: Loan Laux <loan@outgrow.io>
@loan-laux is attempting to deploy a commit to the Vercel Solutions Team on Vercel. A member of the Team first needs to authorize it. |
@@ -19,6 +19,7 @@ | |||
"@utils/*": ["utils/*"], | |||
"@config/*": ["config/*"], | |||
"@assets/*": ["assets/*"], | |||
"@components/checkout/*": ["framework/shopify/components/checkout/*"], |
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 don't think this is required, the way we currently import hooks should also work for components.
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.
Hi @loan-laux👋
Before moving on, we'll me merging the nodejs agnostic PR tomorrow most likely: #252 - This would change the way we use the provider and integrate new stuff to it, like the checkout.
We also need to spec this first, I think the checkout should also work like a hook, meaning a useCheckout
hook that returns checkout info and that may also handle form submission, validation and other things unrelated to design, similar to the way https://react-hook-form.com/ works. Once that's done we can define a common Checkout component if needed, but we shouldn't be starting there.
What we currently have for the checkout is the design, but the implementation should follow the patterns of our commerce hooks
Thanks for taking a look, @lfades! Sure, I'll keep an eye on #252 today. I also agree on a As you said, we need to spec this first. Let's wait for the Node.js provider API to be merged and then I'll come back with a better-defined idea. |
This PR moves the checkout form components to the providers, so that each provider can provide its own logic and UI for payment and shipping. This way, providers can decide to use Stripe Elements or whatever works best with their backend.