-
Notifications
You must be signed in to change notification settings - Fork 12
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
Search: small updates on look & feel #497
base: main
Are you sure you want to change the base?
Conversation
Based on my feedback from #496
Those are some great updates! One thing to be careful of is AA, the text color looks like it doesn't have enough contract with the background. You can check this in Firefox with right clicking the element and "Inspect Accessibility properties". |
width: 60%; | ||
max-height: 1000px; | ||
max-width: 1500px; | ||
max-width: 700px; |
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.
What do these position changes look like on mobile?
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.
Very thin. That's a known issue that we are tracking in #423. I won't touch that in this PR.
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.
This might be best as a relative unit either way, em
or vw
instead of px
. A change in font size will make this look too spaced out (smaller font size) or too cramped (larger font size).
Cool, I reviewed and updated the colors to follow the standards. This PR is ready for re-review and merge. |
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.
This is a great change to me 💯
Based on my feedback from #496
This is the results. I think it looks a lot better, more compacted and easy to quickly scan.
Closes #496