-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
Added border bottom to Sideabar of about page #6678
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Maybe use |
sure |
Lighthouse Results
|
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.
IMHO, it's good !
Thanks for this first contribution.
my pleasure, looking forward to do more!! |
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 genuinely looks weird and doesn't fit our design system. Sorry, I'm against this change (but appreciate the time you've put into this).
appreciate your input on this, but i still believe we need some sort of a visual distinction between them. one way I thought is darken out the title a little. if this still feels unnecessary , i will close this pr.. @ovflowd |
Let me mutter a bit about variants we could create, and Ill come back to you :) |
sure 😃 |
@ovflowd any thought on this ? |
Unfortunately haven't putten time into this yet, something I was cogitating/thinking was in line of this: OR this: |
@ovflowd , seeing the reaction of the team , i dont think the above design is feasible for now. for these pass days , i came up with a design that maybe suitable enough. (added a + sign and darken the header for better seperation). what do you think? :) |
maybe without the + it's can be ok |
sure will remove that !! |
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 think this is an improvement over what we currently have so +1 from me
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.
LGTM ! Lighthouse contrast ratio is passing 🎉
The headers being dark is definitely a nimble and good solution :) |
It is important to mention those are just random ideas Ive come up in 5 minutes. |
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.
LGTM. Thank you for your first contribution ;)
Fixes #6677
Description
Validation
Check List
npm run format
to ensure the code follows the style guide.npm run test
to check if all tests are passing.npx turbo build
to check if the website builds without errors.