-
Notifications
You must be signed in to change notification settings - Fork 1.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
update navbar styling, and fix broken links #1386
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks for these improvements, it visually improves the layout. I've requested a few changes.
docs/src/components/Nav.astro
Outdated
> | ||
<span class="has-text-weight-bold"> Playground</span> | ||
</a> | ||
<div class="navbar-end" style="gap:0.3rem;"> |
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.
better not to use inline style definitions like this. Please keep styling consistent with existing practices.
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.
For gap, I could find 2 options.
- Adding
gap:0.3rem
on.navbar-end
inmain.css
. - Adding
mr-1
(for desktop), andmb-1
(for <1024px) on each link in navbar.
I prefer the first one, so pushed that. Let me know if you can think of another approach to better handle this
docs/src/styles/main.scss
Outdated
@media screen and (max-width: 1024px) { | ||
.navbar-item { | ||
margin-top: 0.3em; | ||
display: flex !important; |
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.
using !important
declaratives is not a good practice and reflects misplaced CSS elsewhere. We should try to avoid this.
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.
fixed.
Also added a fix for broken link on apologies for mixing up things in this PR. I'll try to keep clean from next time. |
Styling Updates
Broken Links
DiceDB is now open source
blog, fix the team page redirect, onour
text.Final