Skip to content

fix: navbar active link should match the most specific path#8999

Open
ryzrr wants to merge 1 commit into
nodejs:mainfrom
ryzrr:fix/navbar-active-link
Open

fix: navbar active link should match the most specific path#8999
ryzrr wants to merge 1 commit into
nodejs:mainfrom
ryzrr:fix/navbar-active-link

Conversation

@ryzrr

@ryzrr ryzrr commented Jul 5, 2026

Copy link
Copy Markdown

Description

fix navbar active-link matching that only compared the first path segment, so two items sharing a segment could highlight together. now the most specific link (longest matching prefix) wins.

Validation

run pnpm test in packages/ui-components for the new getActiveNavLink and active-override tests. node's own top nav is unchanged since its items are single-segment, this fixes the nested case for package consumers whose nav has items like /about and /about/sponsors.

Related Issues

none, raised with the maintainers beforehand.

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run pnpm format to ensure the code follows the style guide.
  • I have run pnpm test to check if all tests are passing.
  • I have run pnpm build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

nested nav items highlighted together because the active check only looked at the first path segment, so /about and /about/sponsors both matched on any /about page.

NavBar now picks the longest link that prefixes the current path and passes an explicit active flag to each item, so a parent stays active on its own sub-pages but hands off to a more specific child. this adds an optional active prop on BaseActiveLink that other callers like Sidebar and Footer don't set, so their behavior is unchanged.

Signed-off-by: Nikhil Kumar Rajak <ryzrr.official@gmail.com>
@ryzrr ryzrr requested a review from a team as a code owner July 5, 2026 18:41
Copilot AI review requested due to automatic review settings July 5, 2026 18:41
@vercel

vercel Bot commented Jul 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nodejs-org Ready Ready Preview Jul 5, 2026 6:42pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cursor

cursor Bot commented Jul 5, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Scoped UI navigation highlighting in ui-components with unit tests; default BaseActiveLink behavior is unchanged when active is omitted.

Overview
Fixes navbar active-state when multiple items share a path prefix (e.g. /about and /about/sponsors), where both could highlight under the old first-segment / per-item allowSubPath behavior.

Adds exported getActiveNavLink, which chooses the longest internal link that equals the pathname or is a prefix (pathname === link or pathname.startsWith(link + '/')), ignoring externals and false positives like /about-us. NavBar computes one winner and passes active={link === activeLink} into each NavItem.

BaseActiveLink gains an optional active boolean: when set, it overrides pathname/allowSubPath matching so the bar can drive highlighting centrally; when omitted, behavior stays as before.

Reviewed by Cursor Bugbot for commit 0079837. Bugbot is set up for automated code reviews on this repo. Configure here.

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.

3 participants