-
Notifications
You must be signed in to change notification settings - Fork 12
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
nav: set the active item on the top navigation #197
nav: set the active item on the top navigation #197
Conversation
✅ Deploy Preview for bump-content-hub ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
src/_components/shared/nav/item.erb
Outdated
@@ -1,3 +1,3 @@ | |||
<nav-item> | |||
<nav-item<% if @active %> class="active"<% end %>> |
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.
What do you think about using the a11y attribute aria-current=true
instead?
Based on the three collections “help”, “guides” and “product-updates” this commit adds a CSS property `active` on the active category in the top navigation bar. Visually I've simply added an underlined text decoration (but fill free to suggest something else). Part of bump-sh#193
bd81812
to
1e5dd46
Compare
I changed the style of the current page link to match what we have in the guides page, is that alright with you @grossyoan ? |
That's perfect Thimy! |
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!
Based on the three collections “help”, “guides” and “product-updates”
this commit adds a CSS property
active
on the active category in thetop navigation bar.
Visually I've simply added an underlined text decoration (but fill
free to suggest something else).
before
after
Part of #193