Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add an address form and have the Javascript send that data when availabl... #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Nitron
Copy link

@Nitron Nitron commented Nov 3, 2012

...e.

This form extends the basic Stripe payment form, adding address fields.

@skoczen
Copy link
Member

skoczen commented Dec 2, 2012

Sorry for being slow on this. I generally like the spirit of this pull, have a request and a question:

  • Request: can you add docs for this? A little section in the readme would suffice, but if we're going to provide this functionality, we should tell people.
  • Question: magically detecting the fields via visible feels wrong. I understand why it's done that way, but wonder if there's a better way to be explicit about which form you're using and have the JS detect that. The current implementation starts to get a bit worrisome - the visibility of address_1 doesn't actually imply that the form's really there. It's a quick check, but I'd love to see something more explicit.

Thanks for the pull and contribution!

@Nitron
Copy link
Author

Nitron commented Dec 14, 2012

No worries, I'm slow too! Hoping to get to this over the weekend.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants