-
Notifications
You must be signed in to change notification settings - Fork 10.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
docs: Add reference guide for processing payments with Square #20977
Conversation
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 awesome!
Did an editor's review. But want to do a technical walkthrough as well.
Co-Authored-By: LB <barth.laurie@gmail.com>
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.
A couple more comments. Almost done with checking the functionality :)
Hey, This will have a performance impact on gatsby because of the script tag. I propose to do something like the following so we only load the SDK when the component is used, even better is to move it into the The example of square isn't that great because the UI gets recreated when javascript loads with buttons and all of that, it introduces layouts shift and restyling operations. This can be taxing on low power devices where CPU is slow. I'm happy to hop on a call or have a chat about this in LA :) https://codesandbox.io/s/square-gatsby-qzitt
|
Thanks so much for weighing in, @wardpeet! Your example is enough to help me rewrite that part of the guide. (I would love to pair on this or anything else, regardless.) This will also help me address Laurie's most recent comment. 😄 |
@AishaBlake i have the starter almost complete, there's still a couple of items to be addressed and based on that i would like to ask you a couple of things. Feel free to provide feedback |
@jonniebigodes I'd make it as complete as you can and use hooks but it's totally up to you. This will be your starter to maintain, so those decisions are also yours. I'm grateful that you're taking this on and happy to provide feedback where I can. |
@AishaBlake probably i didn't explain myself properly. What i meant to say regarding the hooks approach was to aim for consistency in both the documentation and starter. If a single code base can be used for both aspects. Feel free to provide feedback. |
@jonniebigodes I think I understand. I think the hooks approach makes sense (and I can always adjust the guide laster if need be). I very intentionally left much of the code Square gives you, only adding labels for inputs and making it work. I would expect your starter to improve upon that base, not necessarily match it exactly. Feel free to link to your code in the relevant issue and tag me when you're ready for direct feedback! |
@laurieontech checking in on this. Were you able to have another look and did the update clarify things for you? |
I somehow lost track of this thinking it was the PR about the starter. Will look now! |
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 helps a lot, a couple things.
- Are the middle two examples both meant to have title paymentform.js? That confused me a bit.
- The second to last code snippet is quite long. Even though we're giving examples it verges into tutorial land potentially? What about having an example repo and linking to it through out instead? Just an idea.
Yes, they're both meant to have the same filename. I've broken the file into two pieces. I could include the whole file in the second code block but, as you said, it's already super long. Not all of that code is strictly necessary but I waffled back and forth between this and leaving things out which would potentially cause different confusion. It may be simpler to link to an example repo. I don't know if the one I sent you is sufficient... |
I think the one you sent me is likely exactly right! |
Added! |
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.
Made a recommendation to highlight a couple code snippet lines, let me know if you think those are accurate.
Co-Authored-By: LB <barth.laurie@gmail.com>
LGTM! |
@jonniebigodes: maybe also include somehow about this serverless to this doc so that others know about it? |
Description
Adding a reference guide for working with Square to help shore up the e-commerce documentation. I've gone back and forth on this a bit, trying to decide how far to take it. Having gone through the Square documentation, it seems to me like the existing plugin doesn't correctly place the required reference to Square's library.
I feel like it's probably worth creating a starter to make this easier for people but also that something like that goes beyond what I set out to do in this PR. I've opened an issue for creating a Square starter.
In the meantime, the goal of this PR is to give people a starting point. Folks should have an idea of the capabilities of Square, be able to hook up an app, and get a nonce (token) back when submitting the payment form.
I'd love feedback on where screenshots might be most helpful!
Related Issues
Fixes #20224
Addresses #19768