Skip to content
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

Fix padding on buy screen within modal #4253

Closed
markmhendrickson opened this issue Sep 21, 2023 · 7 comments · Fixed by #4470
Closed

Fix padding on buy screen within modal #4253

markmhendrickson opened this issue Sep 21, 2023 · 7 comments · Fixed by #4470
Assignees
Labels
effort:small Expected to take up to 1 day of integration work

Comments

@markmhendrickson
Copy link
Collaborator

We seem to have too much in a few places:

Screenshot 2023-09-21 at 14 21 06 Screenshot 2023-09-21 at 14 21 10
@pete-watters
Copy link
Contributor

I can fix this and also this issue: #4423

But we don't have visual designs for a lot of screens across all views so we have improvised here. It would be helpful to have pixel perfect designs for screens in at least full screen and extension view so we can set padding etc. correctly

@markmhendrickson
Copy link
Collaborator Author

@mica000 @fabric-8 either of you want to take on this design guidance?

@mica000
Copy link

mica000 commented Oct 27, 2023

Designs here: https://www.figma.com/file/mpfjDNidnBwUUoCz8r1Wkx/%232251---Fund-account?type=design&node-id=376-53889&mode=design&t=dxep2GBa6ZYT6OBm-4

settings of the wrapper around the cards
Screenshot 2023-10-27 at 15 40 24

@pete-watters
Copy link
Contributor

pete-watters commented Nov 1, 2023

I started looking at this but the Buy step is failing in the dev build. I'll investigate

=> I found a comment in the code saying the issue above is a known one so it can be ignored

Screenshot 2023-11-01 at 08 56 12

@pete-watters
Copy link
Contributor

Thanks for linking this design @mica000 . I've made some changes:

  • to improve the spacing
  • reduce heading size in extension view

When resizing the browser, when I move from full tab the changes don't kick in until we are in a narrow screen. Let me know if you want me to further adjust heading size / padding along the way

Kapture.2023-11-01.at.11.34.18.mp4

@pete-watters pete-watters linked a pull request Nov 1, 2023 that will close this issue
@pete-watters pete-watters added the effort:small Expected to take up to 1 day of integration work label Nov 1, 2023
@mica000
Copy link

mica000 commented Nov 1, 2023

@pete-watters Sorry, I see that you just closed the issue but didnt add the feedback yet. My bad!

Here is the issue I talked about:
Screenshot 2023-11-01 at 18 49 11

Wondering if we could:

  1. Change the header to heading 3 on the extension;
  2. Add the background color to fix the issue with the white bg when resizing to small sizes;
Screenshot 2023-11-01 at 18 48 33

https://www.figma.com/file/2MLHeIeL6XPVi3Tc2DfFCr/%E2%9D%96-Leather-%E2%80%93-Design-System?type=design&node-id=6421-55389&mode=design&t=LE3yc8UvRyVRPwK0-4

@pete-watters
Copy link
Contributor

@mica000 not your bad at all, I closed it as I fixed the other problems.

I can see the issue too and it looks bad in dark mode also:
https://github.com/leather-wallet/extension/assets/2938440/4b468843-289c-4190-9ce8-edc461eb288c

It seems:

  • it doesn't behave the same on the Fund page as the Home in the breakpoint just before extension
  • in dark mode the header gets set to black but it should be the page BG colour
  • the header has too much bottom padding so then the text heading doesn't get proper top padding

I'll fix it in the containers issue if that's OK?

I will be working on that next week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort:small Expected to take up to 1 day of integration work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants