-
Notifications
You must be signed in to change notification settings - Fork 464
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: WC session list height #2642
Conversation
Branch preview✅ Deploy successful! https://wc_session_height--walletweb.review-wallet-web.5afe.dev |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
@@ -9,7 +9,7 @@ | |||
.sessionListItem { | |||
border: 1px solid var(--color-border-light); | |||
border-radius: 6px; | |||
height: 56px; | |||
min-height: 56px; |
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.
Setting a pixel height of anything is per se an anti-pattern. Why can't it just be the size of the content?
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.
It's to match the height of the input field/our "global" Accordion
min-height
:
vs.
Shall I remove it?
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.
I don't see any difference 🤷
can be merged |
What it solves
Resolves overflow of DApp names
How this PR fixes it
The
ListItem
s ofSessionList
now have amin-height
instead of fixedheight
.How to test it
Connect to CoW Swap via WC and observe no overflow of the name.
Screenshots
Checklist