Skip to content

Commit f2f21ac

Browse files
authored
fix: navbar active link should match the most specific path (#8999)
fix: only highlight the most specific navbar item 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>
1 parent 4fd34e2 commit f2f21ac

5 files changed

Lines changed: 106 additions & 4 deletions

File tree

packages/ui-components/src/Common/BaseActiveLink/__tests__/index.test.jsx

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,42 @@ describe('ActiveLink', () => {
4949
'link active'
5050
);
5151
});
52+
53+
it('forces active class when active override is true regardless of pathname', async () => {
54+
render(
55+
<ActiveLink
56+
className="link"
57+
activeClassName="active"
58+
href="/link"
59+
pathname="/not-link"
60+
active
61+
>
62+
Link
63+
</ActiveLink>
64+
);
65+
66+
assert.equal(
67+
(await screen.findByText('Link')).getAttribute('class'),
68+
'link active'
69+
);
70+
});
71+
72+
it('forces inactive when active override is false even when pathname matches', async () => {
73+
render(
74+
<ActiveLink
75+
className="link"
76+
activeClassName="active"
77+
href="/link"
78+
pathname="/link"
79+
active={false}
80+
>
81+
Link
82+
</ActiveLink>
83+
);
84+
85+
assert.equal(
86+
(await screen.findByText('Link')).getAttribute('class'),
87+
'link'
88+
);
89+
});
5290
});

packages/ui-components/src/Common/BaseActiveLink/index.tsx

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export type ActiveLocalizedLinkProps = ComponentProps<LinkLike> & {
77
activeClassName?: string;
88
allowSubPath?: boolean;
99
pathname?: string;
10+
active?: boolean;
1011
as?: LinkLike;
1112
};
1213

@@ -16,17 +17,21 @@ const BaseActiveLink: FC<ActiveLocalizedLinkProps> = ({
1617
className,
1718
href = '',
1819
pathname = '/',
20+
active,
1921
as: Component = 'a',
2022
...props
2123
}) => {
22-
const finalClassName = classNames(className, {
23-
[activeClassName]: allowSubPath
24+
// NavBar sets `active` explicitly; if it's not passed we fall back to the path check.
25+
const isActive =
26+
active ??
27+
(allowSubPath
2428
? // When using allowSubPath we want only to check if
2529
// the current pathname starts with the utmost upper level
2630
// of an href (e.g. /docs/...)
2731
pathname.startsWith(`/${href.toString().split('/')[1]}`)
28-
: href.toString() === pathname,
29-
});
32+
: href.toString() === pathname);
33+
34+
const finalClassName = classNames(className, { [activeClassName]: isActive });
3035

3136
return <Component className={finalClassName} href={href} {...props} />;
3237
};

packages/ui-components/src/Containers/NavBar/NavItem/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ type NavItemProps = {
1717
target?: HTMLAttributeAnchorTarget | undefined;
1818

1919
pathname: string;
20+
active?: boolean;
2021
as: LinkLike;
2122
};
2223

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { describe, it } from 'node:test';
2+
import assert from 'node:assert/strict';
3+
4+
import { getActiveNavLink } from '..';
5+
6+
const navItems = [
7+
{ link: '/about' },
8+
{ link: '/about/sponsors' },
9+
{ link: '/docs' },
10+
{ link: 'https://nodejs.org/learn' },
11+
];
12+
13+
describe('getActiveNavLink', () => {
14+
it('picks the most specific sibling on its own pages', () => {
15+
assert.equal(
16+
getActiveNavLink(navItems, '/about/sponsors'),
17+
'/about/sponsors'
18+
);
19+
});
20+
21+
it('keeps the parent active on its other sub-pages', () => {
22+
assert.equal(
23+
getActiveNavLink(navItems, '/about/governance/charter'),
24+
'/about'
25+
);
26+
});
27+
28+
it('matches the parent on its own index page', () => {
29+
assert.equal(getActiveNavLink(navItems, '/about'), '/about');
30+
});
31+
32+
it('does not match across word boundaries', () => {
33+
assert.equal(getActiveNavLink(navItems, '/about-us'), '');
34+
});
35+
36+
it('never returns an external (non-internal) link', () => {
37+
assert.equal(getActiveNavLink(navItems, 'https://nodejs.org/learn'), '');
38+
});
39+
40+
it('returns empty string when nothing matches', () => {
41+
assert.equal(getActiveNavLink(navItems, '/unknown'), '');
42+
});
43+
});

packages/ui-components/src/Containers/NavBar/index.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,18 @@ const navInteractionIcons = {
2121
close: <XMark className={styles.navInteractionIcon} />,
2222
};
2323

24+
// Picks the longest nav link that prefixes the current path, so `/about` and `/about/sponsors` never highlight together.
25+
export const getActiveNavLink = (
26+
navItems: Array<{ link: string }>,
27+
pathname: string
28+
): string =>
29+
navItems.reduce((longest, { link }) => {
30+
const isMatch =
31+
link.startsWith('/') &&
32+
(pathname === link || pathname.startsWith(`${link}/`));
33+
return isMatch && link.length > longest.length ? link : longest;
34+
}, '');
35+
2436
type NavbarProps = {
2537
navItems?: Array<{
2638
text: FormattedMessage;
@@ -43,6 +55,8 @@ const NavBar: FC<PropsWithChildren<NavbarProps>> = ({
4355
}) => {
4456
const [isMenuOpen, setIsMenuOpen] = useState(false);
4557

58+
const activeLink = getActiveNavLink(navItems ?? [], pathname);
59+
4660
return (
4761
<nav className={styles.container}>
4862
<div className={styles.innerContainer}>
@@ -79,6 +93,7 @@ const NavBar: FC<PropsWithChildren<NavbarProps>> = ({
7993
{navItems.map(({ text, link, target }) => (
8094
<NavItem
8195
pathname={pathname}
96+
active={link === activeLink}
8297
as={Component}
8398
key={link}
8499
href={link}

0 commit comments

Comments
 (0)