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

ui: add padding to nav bar #5034

Closed
wants to merge 3 commits into from
Closed

Conversation

std-microblock
Copy link

@std-microblock std-microblock commented Jan 30, 2023

image

The current navigation bar has no margin to the top of the website, which has made it looks pretty weird and ugly to me. So I added a 0.5em top padding to it to make it looks comfortable and visually balanced to me.

image

This is what it looks like after the change.

@ovflowd
Copy link
Member

ovflowd commented Jan 30, 2023

Thanks! This is a nice contribution. But we actually fixed this on the #4991 PR, hence this code change would create merge-conflicts and ultimately be de-duplicated.

I super appreciate your contribution, regardless of it being merged or not :)

(And welcome you to do future contributions).

@ovflowd ovflowd closed this Jan 30, 2023
@std-microblock
Copy link
Author

Thanks! This is a nice contribution. But we actually fixed this on the #4991 PR, hence this code change would create merge-conflicts and ultimately be de-duplicated.

I super appreciate your contribution, regardless of it being merged or not :)

(And welcome you to do future contributions).

image
( yours )

image
( mine )

It still looks weird in your pr xD. These three elements should be on the same horizontal line to make sure it's visually balanced. I can make a pr later after your pr is merged, or you can change that in your pr anyway. You can take the code directly as I dont care if I'm mentioned as long as it looks nice.

@std-microblock
Copy link
Author

Also I suggest enlarging the height of the navigation bar to make it look looser ( I've also done that in my pr ), there's still lots of spare space, isn't it?

@ovflowd
Copy link
Member

ovflowd commented Jan 31, 2023

Just so you know, that layout is going to be discarded in favor of this so the items being aligned right now aren't really important 😅

@ovflowd
Copy link
Member

ovflowd commented Jan 31, 2023

I can make a pr later after your pr is merged, or you can change that in your pr anyway. You can take the code directly as I dont care if I'm mentioned as long as it looks nice.

It's not about caring or not or giving you the appropriate credits, it's more like, we're going to drop this layout soon 🙈

@std-microblock
Copy link
Author

Nice! Looking forward to new layout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants