-
Notifications
You must be signed in to change notification settings - Fork 304
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
Add icons to navbar #965
Add icons to navbar #965
Conversation
looks quite nice! |
@graeme-a-stewart - what's your take on this? The code change itself is trivial |
Looks good to me too 👍 . |
Hi! When we were discussing this in the coordination meeting one of the recommendations was to have a bit more space between icon and text. We didn't exactly take a vote, but I got the feeling that this PR was a bit more controversial (?) |
Hi - thanks @klieret, I do like the idea. As @eduardo-rodrigues said I think it looks better with a little more space between the icon and the text, which I added on the last commit (I don't know if there's a better way than |
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.
Still need to fix misalignment
I can take a look later this week again. Perhaps I can figure out the alignment issue |
The issue of the misalignment is due to conflicting padding settings in /* The logo */
.navbar-brand {
height: 40px;
padding: 12.5px 15px;
font-size: 15px;
}
/* This is the rest of the items */
.navbar-nav > li > a {
padding-top: 9.5px;
padding-bottom: 9.5px;
} let me fix that |
Thanks @klieret - the fix is perfect 👍 |
Not sure about this, but I was playing around with having icons in the navbar, e.g.
might be a bit cluttered