-
Notifications
You must be signed in to change notification settings - Fork 131
Add labels and mutations for default addresses #39
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
Add labels and mutations for default addresses #39
Conversation
we dont need it if theres no label
sometimes you dont want it, example: pagination
used this for testing line clamping - it worked
UI is not connected to pagination yet, testing via url params works though
this makes the focus ring fully visible for button groups
- take/skip pagination works now - add pagination label - refine zod validation schema - redirect to searchless url - filter active orders out - new error msg for when theres no more orders due to pagination
✅ Deploy Preview for inspiring-boba-355f4d ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for remix-vendure ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Just FYI before you continue, same conflicts here 😅 |
All good, thats to be expected, no problem 😄 |
Fixed it now but appearently Github doesnt reset the merged commits automatically so I'll try the solution from here. It says to change the base branch back and forth. Lets try it. |
That seems to have done the trick, GitHub doesn't complain to me about conflicts anymore |
I just found out that by dismissing the address deletion modal, you actually confirm it and delete the address, that made me laugh Screencast.from.21.04.2023.16.42.05.webmLet's fix this too as part of this PR 😆 |
@kyunal @michaelbromley we got customer address handling now :D Screencast.from.22.04.2023.16.17.51.webmI pretty much completely reworked the address page because it didnt even use Remix' patterns and forms. This video by Remix' maker Ryan Florence helped me implement this. The form pattern works really nicely. |
This mostly looks good to me. Just for my own understanding and for updating the README accordingly - this is only for setting the default shipping/billing address, but it isn't used in checkout, right? I'm still happy to merge without, just want it to be documented correctly. Separate billing addresses aren't supported by the checkout at all right now and if I select a default shipping address it isn't automatically selected. |
Correct. I implemented it at I guess having the whole CRUD actions covered is nice UX, then new customers dont have to switch routes i.e. Creating, updating, deleting and setting default addresses in the checkout covers what we need. Anything else? |
I think that's about it. Some of the actions might even be overkill for checkout, e.g. setting default addresses could make everything be very crowded. But that shouldn't mean we should rule that out entirely, just if it comes down to it it's fine to disregard certain features for checkout. And yes, it makes sense to separate them. Again, thanks for all the work you've recently put into this starter. I'll go ahead and merge this. |
This contains commits from #37 and #38 - merge those first. There are no other PRs open so I assumed we'll just merge them in a row so it all should work out. TODO: mutations for setting.