-
Notifications
You must be signed in to change notification settings - Fork 9
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: APP-366 account orders #2566
Conversation
✅ Deploy Preview for terrasos ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for regen-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@erikalogie @S4mmyb see testing instructions |
We have one: https://regennetwork.atlassian.net/browse/APP-441 |
@erikalogie you can have another look, I've also added support for displayed credit card info. |
@erikalogie how much should I increase the space between image and project name/id on mobile? there's a space of 10px as on figma |
on Figma it is 20px between the two sections, you can see here, and same on mobile and desktop: |
@erikalogie I'm asking about this: |
Oh maybe that part will look fine if the space below is changed. So leave it for now. |
6c2886b
to
5c14ced
Compare
@erikalogie you can have another look |
Everything looks good, just let me know when the server PR is merged and I can test the stripe receipt button |
3b29600
to
4243cae
Compare
@erikalogie you can test this now |
LGTM! Nice work here |
@r41ph I've added some new stuff since your initial approval, so let me know if you wanna have another look before I merge this, thanks! |
@blushi the code looks good to me. A couple of things:
|
I've fixed the Order tests and spacing issues. Could you have another look? @r41ph Else (like it's the case on your screen shot), they wrap and this prevents overflowing on even smaller screens, which would look bad: We would need to set a lower horizontal padding for the buttons which would look quite bad as well IMO: Thoughts @erikalogie ? |
I think the way you've done it looks good |
60528dc
to
e6c3ce3
Compare
Description
https://regennetwork.atlassian.net/browse/APP-366
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
How to test
From https://deploy-preview-2566--regen-marketplace.netlify.app/
NB: credit card info is missing for now, I've realized that we need some server update for that, which I'm working on, but the rest can still be tested in the mean time.DONEReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...