Fix nav highlighting and wiki link#11
Conversation
- Refactored `isActive` logic in `Layout.vue` to ensure the "Community" navbar item is only highlighted (gold) on the home page. - Ensured "Open Source", "Links", and "Interviews" sections correctly highlight their parent navigation items when their respective pages or sub-pages are active. - Updated all MUME Wiki links to use the absolute path `https://docs.mume.org/wiki/`.
- Restricted 'Community' nav highlight to the home page only. - Ensured 'Open Source', 'Links', and 'Interviews' parents highlight when active. - Updated Wiki link to absolute path 'https://docs.mume.org/wiki/'. - Added 'target="_self"' and 'rel="external"' to Wiki links to ensure they open in the same tab and prevent VitePress router interference.
Reviewer's GuideRefactors the navigation highlighting logic in the custom VitePress layout into a reusable helper and updates all Wiki links to point to docs.mume.org with consistent link attributes. Sequence diagram for updated nav highlighting on route changesequenceDiagram
actor User
participant Browser
participant VitePressRouter
participant LayoutComponent
participant isActiveHelper
User->>Browser: Click nav link (e.g. /opensource)
Browser->>VitePressRouter: Navigate to new path
VitePressRouter-->>Browser: Update current route path
Browser->>LayoutComponent: Re-render Layout.vue
LayoutComponent->>isActiveHelper: isActive('/', true)
isActiveHelper-->>LayoutComponent: true or false (Community highlighted)
LayoutComponent->>isActiveHelper: isActive('/opensource')
isActiveHelper-->>LayoutComponent: true for /opensource path and children
LayoutComponent-->>Browser: Render nav with current class on matching items
Browser-->>User: Display updated highlighted navigation
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
isActive('/', true)on the Community top-level link no longer marks Community as active when viewing/opensource,/links, or/interviews, which changes behavior from the previous implementation and may be an unintended UX regression; consider keeping the parent active when a child route is active. - Since
isActiveencapsulates the route matching logic used in several places, you might consider also using it for the Community parent (e.g.,isActive('/', /*include children*/)with an option) instead of hardcodingexact = true, to make the API more flexible and self-consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `isActive('/', true)` on the Community top-level link no longer marks Community as active when viewing `/opensource`, `/links`, or `/interviews`, which changes behavior from the previous implementation and may be an unintended UX regression; consider keeping the parent active when a child route is active.
- Since `isActive` encapsulates the route matching logic used in several places, you might consider also using it for the Community parent (e.g., `isActive('/', /*include children*/)` with an option) instead of hardcoding `exact = true`, to make the API more flexible and self-consistent.
## Individual Comments
### Comment 1
<location path="docs/.vitepress/theme/Layout.vue" line_range="89" />
<code_context>
</li>
<li class="has_children">
- <a :href="withBase('/')" :class="{ current: vpRoute.path === '/' || vpRoute.path === '/index.html' || vpRoute.path.startsWith(withBase('/opensource')) || vpRoute.path.startsWith(withBase('/links')) || vpRoute.path.startsWith(withBase('/interviews')) }">Community<span class="caret down"></span></a>
+ <a :href="withBase('/')" :class="{ current: isActive('/', true) }">Community<span class="caret down"></span></a>
<ul>
<li><a href="https://discord.gg/XkZN55am9a" target="_blank" rel="noopener">Discord <i class="fa fa-external-link" aria-hidden="true"></i></a></li>
</code_context>
<issue_to_address>
**question (bug_risk):** The `Community` menu no longer stays active when viewing its child pages, which changes prior UX behavior.
Previously, `Community` was marked `current` not only on `/` but also on routes starting with `/opensource`, `/links`, and `/interviews`. With `isActive('/', true)`, it now highlights only on the exact home route.
If that’s not intended, you could either:
- Use `isActive('/', false)` and treat `/` as a parent for those sections, or
- Extend `isActive` (or add a helper) so the parent item is active when any of its child paths are active.
If the change is intentional, just confirm it matches the desired navigation behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| </li> | ||
| <li class="has_children"> | ||
| <a :href="withBase('/')" :class="{ current: vpRoute.path === '/' || vpRoute.path === '/index.html' || vpRoute.path.startsWith(withBase('/opensource')) || vpRoute.path.startsWith(withBase('/links')) || vpRoute.path.startsWith(withBase('/interviews')) }">Community<span class="caret down"></span></a> | ||
| <a :href="withBase('/')" :class="{ current: isActive('/', true) }">Community<span class="caret down"></span></a> |
There was a problem hiding this comment.
question (bug_risk): The Community menu no longer stays active when viewing its child pages, which changes prior UX behavior.
Previously, Community was marked current not only on / but also on routes starting with /opensource, /links, and /interviews. With isActive('/', true), it now highlights only on the exact home route.
If that’s not intended, you could either:
- Use
isActive('/', false)and treat/as a parent for those sections, or - Extend
isActive(or add a helper) so the parent item is active when any of its child paths are active.
If the change is intentional, just confirm it matches the desired navigation behavior.
Summary by Sourcery
Improve docs navigation highlighting and update wiki links to the new docs host.
Bug Fixes:
Enhancements:
Documentation: