-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Fix the bug of "active" class not working when navbar links are accessed. #32874
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
Conversation
templates/base/head_navbar.tmpl
Outdated
@@ -3,15 +3,15 @@ | |||
{{$notificationUnreadCount = call .NotificationUnreadCount}} | |||
{{end}} | |||
|
|||
<nav id="navbar" aria-label="{{ctx.Locale.Tr "aria.navbar"}}"> | |||
<nav id="navbar" class="ui secondary menu" aria-label="{{ctx.Locale.Tr "aria.navbar"}}"> | |||
<div class="navbar-left"> |
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.
I think ui secondary menu
should be on class="navbar-left"
?
Ideally the layout should be:
<div "ui secondary menu">
<div "item active">...</>
<div "item">...</>
</>
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.
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.
The ui secondary menu
is from Fomantic UI: https://fomantic-ui.com/collections/menu.html#secondary-menu
I do not think we should abuse it by inserting unrelated elements between menu
and item
.
If you'd like to make it work for "nav-right", you could do this:
<div navbar>
<div navbar-left ui secondary menu> item ... </div>
<div navbar-right ui secondary menu> item ... </div>
</div>
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.
Actually there could be another choice: totally drop Fomantic UI, and add a .navbar-left > .item.active, .navbar-right > .item.active { ... }
style
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.
Actually there could be another choice: totally drop Fomantic UI, and add a
.navbar-left > .item.active, .navbar-right > .item.active { ... }
style
Hmm ...... I think we need to do this, because the styles are all done by us, not by Fomantic UI, and there is a plan to remove Fomantic UI step by step.
We need to make the code maintainable, otherwise the UI will break soon. So the proper way is to do it like this: Improve navbar: add "admin" tip, add "active" style #32927
|
before:
after: