-
Notifications
You must be signed in to change notification settings - Fork 130
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
feat: add country selection to address
component 🇨🇦
#434
Conversation
hi @tomas-salgado! I just wanted to give an update on what I've been working on. Let me know if I am heading in the right direction. Thank you! |
@jessherlitz yep, this looks like it's on the right track! It seems the CI test is failing right now because of a types issue, but at first glance it doesn't look like it should be too tricky to resolve. Let me know if you need any help! |
@tomas-salgado sorry for the delay on this.. I am almost done!! |
50eb7ed
to
79e88b8
Compare
Hi @tomas-salgado! I finished this issue. Could you please review it? Thank you! |
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.
Nice work @jessherlitz! Left a few comments-lmk when those are addressed and I'll take another look!
apps/member-profile/app/routes/_profile.home.claim-swag-pack._index.tsx
Outdated
Show resolved
Hide resolved
apps/member-profile/app/routes/_profile.home.claim-swag-pack._index.tsx
Outdated
Show resolved
Hide resolved
apps/member-profile/app/routes/_profile.home.claim-swag-pack._index.tsx
Outdated
Show resolved
Hide resolved
address
component 🇨🇦
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.
Just tested this out locally, really great work @jessherlitz!
One note for @ramiAbdou when he reviews:
Right now the dropdown for states will default to US States, but it changes to Canadian territories if you select your country as Canada. This functionality works fine. Do you think this is good or we should have the remaining fields (State, etc.) only appear AFTER selecting country?
I took a look at other address fields (ie: Stripe) and they typically have the country field appear first, so I moved the country field to the top, so I think that should suffice. Don't think we need to hide the other fields or anything like that since most people will fill out the form in order. |
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.
Back-to-back amazing PRs @jessherlitz! 🔥
A bit of unfortunate news that we just found out today though...so apparently SwagUp supports international shipments now, however there's specific items that can't be shipped internationally and we have one of those items in our swag pack 😞😞
That being said, this PR is still helpful b/c now we know if a member is trying to ship to Canada. I added some logic to notify our team if someone tries to claim their pack using a Canadian address so that we can manually send them a merch store gift card).
Keep up the momentum, Jess!
address
component 🇨🇦 address
component 🇨🇦
Description ✏️
Closes #342
Type of Change 🐞
Checklist ✅