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

update navbar styling, and fix broken links #1386

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

RamGoel
Copy link

@RamGoel RamGoel commented Dec 19, 2024

Styling Updates

  • Add spacing in nav-items to prevent this ↑
  • Separate internal & external links, (personal suggestion, feel free to avoid, let me know I'll undo this change)

Broken Links

  • When on /blog, the blog link isn't active because of typo
  • In the DiceDB is now open source blog, fix the team page redirect, on our text.

Final

image

@RamGoel RamGoel changed the title feat: nav spacing, rearrange links, & blog active fix nav spacing, rearrange links, & blog active fix Dec 19, 2024
Copy link
Collaborator

@JyotinderSingh JyotinderSingh left a 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.

>
<span class="has-text-weight-bold"> Playground</span>
</a>
<div class="navbar-end" style="gap:0.3rem;">
Copy link
Collaborator

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.

Copy link
Author

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 in main.css.
  • Adding mr-1 (for desktop), and mb-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

@media screen and (max-width: 1024px) {
.navbar-item {
margin-top: 0.3em;
display: flex !important;
Copy link
Collaborator

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

@RamGoel RamGoel changed the title nav spacing, rearrange links, & blog active fix update navbar styling, and fix broken links Dec 21, 2024
@RamGoel
Copy link
Author

RamGoel commented Dec 21, 2024

Also added a fix for broken link on DiceDB is now open source blog in this PR.

apologies for mixing up things in this PR. I'll try to keep clean from next time.

cc @JyotinderSingh

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