From 09c02443c77f90c6daff280fabcf3045986b0008 Mon Sep 17 00:00:00 2001 From: Niall O'Brien Date: Fri, 29 May 2026 12:10:06 +0100 Subject: [PATCH 1/6] feat: normalize menu path matching and expand logic - Add normalizePath utility to compare paths without trailing slashes - Update menu util functions to use normalized paths - Refactor Sidenav to use new menu matching helpers - Expand menu sections based on active path or section type - Add tests for path normalization and menu matching logic --- src/components/Sidenav/Sidenav.module.css | 23 +++- src/components/Sidenav/Sidenav.tsx | 146 ++++++++++++++-------- src/lib/menu/util.test.ts | 72 ++++++++++- src/lib/menu/util.ts | 48 ++++++- src/util/url.js | 12 ++ src/util/url.test.ts | 12 ++ 6 files changed, 251 insertions(+), 62 deletions(-) create mode 100644 src/util/url.test.ts diff --git a/src/components/Sidenav/Sidenav.module.css b/src/components/Sidenav/Sidenav.module.css index ab7bdaeb..6c15b5b0 100644 --- a/src/components/Sidenav/Sidenav.module.css +++ b/src/components/Sidenav/Sidenav.module.css @@ -42,11 +42,7 @@ /* Lists */ .listWrapper { - overflow-y: hidden; - - &:first-child { - padding-block: var(--space-l); - } + overflow: hidden; } /* Overwrite animation from JS to always display on mobile */ @@ -76,13 +72,20 @@ } } +.listRoot { + padding-block: var(--space-l); +} + .section + .listWrapper > .list { padding-inline-start: 0; } /* Links */ .linkButton { + display: flex; flex-shrink: 0; + align-items: center; + line-height: 0; cursor: pointer; } @@ -117,6 +120,16 @@ --svg-path-fill: var(--brand-color-blue-7); } +.linkMain { + display: flex; + flex: 1; + align-items: center; + min-width: 0; + line-height: 0; + text-decoration: none; + color: inherit; +} + .linkTitle { padding-inline-end: var(--space-xs); } diff --git a/src/components/Sidenav/Sidenav.tsx b/src/components/Sidenav/Sidenav.tsx index a620e49b..3693960d 100644 --- a/src/components/Sidenav/Sidenav.tsx +++ b/src/components/Sidenav/Sidenav.tsx @@ -1,6 +1,6 @@ 'use client'; -import React, { useEffect, useState } from 'react'; +import { type MouseEvent, useEffect, useState } from 'react'; import { cx } from 'class-variance-authority'; import { Transition, Variants } from 'motion/react'; @@ -12,7 +12,12 @@ import { useNavigation } from '@/app/navigation'; import { Icon } from '@/icons'; import { ChevronSmallIcon } from '@/icons/ChevronSmall'; import { MenuItem } from '@/lib/menu/types'; -import { getActiveAncestors } from '@/lib/menu/util'; +import { + getActiveAncestors, + isActiveMenuLeaf, + isExactMenuPath, + shouldExpandMenuItem, +} from '@/lib/menu/util'; import { last } from '@/lib/util'; import { Tag } from '../Tag'; @@ -25,6 +30,12 @@ const openCloseVariants: Variants = { collapsed: { opacity: 0, height: 0 }, }; +/** Stable React key for sidenav items (paths are not unique across siblings). */ +const buildMenuItemKey = (parentKey: string, item: MenuItem, index: number): string => { + const segment = `${index}:${item.path ?? item.title}`; + return parentKey ? `${parentKey}/${segment}` : segment; +}; + export const Sidenav = ({ items }: SidenavProps) => { const { navigationState, toggleNavigation } = useNavigation(); const pathname = usePathname(); @@ -62,115 +73,146 @@ export const Sidenav = ({ items }: SidenavProps) => { opacity: isOpen ? 1 : 0, }} transition={togglerTransition}> -
{items ? : null}
+
+ {items ? ( + + ) : null} +
); }; -const List = ({ items, isExpanded }: ListProps) => { +const MenuItemRows = ({ items, parentKey = '', listClassName }: MenuItemRowsProps) => { + return ( + + ); +}; + +const List = ({ items, isExpanded, parentKey }: ListProps) => { return ( - + ); }; -const Item = ({ item }: ItemProps) => { +const Item = ({ item, itemKey }: ItemProps) => { const pathname = usePathname(); const { setNavigationState } = useNavigation(); - const isCurrentPageActive = item.path === pathname && !item.children; - const shouldBeExpanded = isExpandedByDefault(item, pathname); - const [isExpanded, setIsExpanded] = useState(shouldBeExpanded); + const isActiveLeaf = isActiveMenuLeaf(pathname, item); + const isExactPathMatch = isExactMenuPath(pathname, item); + const defaultExpanded = shouldExpandMenuItem(pathname, item); + const [isExpanded, setIsExpanded] = useState(defaultExpanded); useEffect(() => { - setIsExpanded(shouldBeExpanded); - }, [shouldBeExpanded]); + setIsExpanded(defaultExpanded); + }, [defaultExpanded]); - function toggleExpand(event: React.MouseEvent) { + const handleLinkClick = (event: MouseEvent) => { const isMobileViewport = window.matchMedia('(max-width: 767px)').matches; - // Mobile will always link to the clicked item - if (isCurrentPageActive && isMobileViewport) { + if (item.children && isExactPathMatch && isExpanded) { + event.preventDefault(); + setIsExpanded(false); + return; + } + + if (isActiveLeaf && isMobileViewport) { event.preventDefault(); } - if (isMobileViewport && !isCurrentPageActive) { + if (isMobileViewport && !isActiveLeaf) { event.currentTarget.blur(); setNavigationState('closed'); } - setIsExpanded((isExpanded) => !isExpanded); - } + if (item.children && !isExpanded) { + setIsExpanded(true); + } + }; + + const handleChevronClick = (event: MouseEvent) => { + event.preventDefault(); + event.stopPropagation(); + setIsExpanded((expanded) => !expanded); + }; return (
  • {item.path ? ( - +
    {item.children ? ( - + ) : (
    )} -
    {item.title}
    + + {item.title} + - {item.method ? ( + {item.method && ( - {item.method} - - ) : null} - + active={isActiveLeaf} + mobileDarkMode + /> + )} +
    ) : ( {item.title} )} - {item.children ? : null} + {item.children ? : null}
    • ); }; -const isExpandedByDefault = (item: MenuItem, pathname: string) => { - if (!item.path) { - return true; - } - - return getActiveAncestors(pathname, [item]).length > 0; -}; - interface SidenavProps { items: MenuItem[]; } +interface MenuItemRowsProps { + items: MenuItem[]; + /** Omitted at the root; nested lists pass the parent item's key. */ + parentKey?: string; + listClassName: string; +} + interface ListProps { items: MenuItem[]; - isExpanded?: boolean; + isExpanded: boolean; + parentKey: string; } interface ItemProps { item: MenuItem; + itemKey: string; } diff --git a/src/lib/menu/util.test.ts b/src/lib/menu/util.test.ts index 77a72a2d..e4c8d842 100644 --- a/src/lib/menu/util.test.ts +++ b/src/lib/menu/util.test.ts @@ -1,9 +1,18 @@ -import { getActiveAncestors, getActiveMenuItem, getMenuItem } from './util'; +import type { MenuItem } from './types'; +import { + getActiveAncestors, + getActiveMenuItem, + getMenuItem, + isActiveMenuLeaf, + isExactMenuPath, + isMenuItemOnActiveBranch, + shouldExpandMenuItem, +} from './util'; describe('lib', () => { describe('menu', () => { describe('getActiveMenuItem()', () => { - test('finds active top-level item', async () => { + test('finds active top-level item', () => { const active1 = getActiveMenuItem('/api/something'); expect(active1.title).toEqual('API Reference'); const active2 = getActiveMenuItem('/guides/something'); @@ -14,8 +23,9 @@ describe('lib', () => { expect(active4.title).toEqual('Documentation'); }); }); + describe('getActiveAncestors()', () => { - test('finds active menu item with ancestors', async () => { + test('finds active menu item with ancestors', () => { const documentationItem = getMenuItem('documentation'); const ancestors = getActiveAncestors('/migrating-to-cloudsmith/export-from-nexus-sonatype', [ documentationItem, @@ -25,6 +35,62 @@ describe('lib', () => { expect(ancestors[1].title).toEqual('Migrating to Cloudsmith'); expect(ancestors[2].title).toEqual('Migrating from Nexus Sonatype'); }); + + test('matches paths regardless of trailing slash', () => { + const workspacesMenu: MenuItem = { + title: 'Workspaces', + path: '/workspaces', + children: [ + { + title: 'Authentication', + path: '/authentication/', + children: [{ title: 'SAML', path: '/authentication/single-sign-on' }], + }, + ], + }; + + const ancestors = getActiveAncestors('/authentication', [workspacesMenu]); + expect(ancestors.map((item) => item.title)).toEqual(['Workspaces', 'Authentication']); + expect(isMenuItemOnActiveBranch('/authentication', workspacesMenu)).toBe(true); + }); + }); + + describe('isExactMenuPath()', () => { + test('matches paths regardless of trailing slash', () => { + const item: MenuItem = { title: 'Authentication', path: '/authentication/' }; + expect(isExactMenuPath('/authentication', item)).toBe(true); + }); + }); + + describe('isActiveMenuLeaf()', () => { + test('is true only for a matching leaf item', () => { + const leaf: MenuItem = { title: 'SAML', path: '/authentication/single-sign-on' }; + const branch: MenuItem = { + title: 'Authentication', + path: '/authentication', + children: [leaf], + }; + + expect(isActiveMenuLeaf('/authentication/single-sign-on', leaf)).toBe(true); + expect(isActiveMenuLeaf('/authentication/single-sign-on', branch)).toBe(false); + }); + }); + + describe('shouldExpandMenuItem()', () => { + test('expands section headers without a path', () => { + const section: MenuItem = { title: 'Overview' }; + expect(shouldExpandMenuItem('/any-page', section)).toBe(true); + }); + + test('expands branches on the active path', () => { + const workspacesMenu: MenuItem = { + title: 'Workspaces', + path: '/workspaces', + children: [{ title: 'Authentication', path: '/authentication' }], + }; + + expect(shouldExpandMenuItem('/authentication', workspacesMenu)).toBe(true); + }); }); }); }); diff --git a/src/lib/menu/util.ts b/src/lib/menu/util.ts index 704705ac..79b5dbb0 100644 --- a/src/lib/menu/util.ts +++ b/src/lib/menu/util.ts @@ -1,5 +1,7 @@ import type { Menu, MenuItem } from './types'; +import { normalizePath } from '@/util/url'; + import json from '../../content/menu.json'; const menu: Menu = json as Menu; @@ -26,9 +28,11 @@ export const getMenuItems = (keys: string[]): MenuItem[] => { * Finds the active top-level menu item based on the pathname */ export const getActiveMenuItem = (pathname: string): MenuItem => { + const normalizedPathname = normalizePath(pathname); + for (const key in menu) { const item = menu[key]; - if (item.path && pathname.startsWith(item.path)) { + if (item.path && normalizedPathname.startsWith(normalizePath(item.path))) { return item; } } @@ -47,9 +51,13 @@ export const getActiveAncestors = ( items: MenuItem[], ancestors: MenuItem[] = [], ): MenuItem[] => { + const normalizedPathname = normalizePath(pathname); + for (const item of items) { + const normalizedItemPath = item.path ? normalizePath(item.path) : undefined; + // If this item has the exact pathname - if (item.path === pathname) { + if (normalizedItemPath === normalizedPathname) { return ancestors.concat([item]); } @@ -64,3 +72,39 @@ export const getActiveAncestors = ( return []; }; + +/** + * Whether the pathname exactly matches this menu item's path. + */ +export const isExactMenuPath = (pathname: string, item: MenuItem): boolean => { + if (!item.path) { + return false; + } + + return normalizePath(item.path) === normalizePath(pathname); +}; + +/** + * Whether this leaf menu item is the current page. + */ +export const isActiveMenuLeaf = (pathname: string, item: MenuItem): boolean => { + return isExactMenuPath(pathname, item) && !item.children; +}; + +/** + * Whether the current page is this item or one of its descendants. + */ +export const isMenuItemOnActiveBranch = (pathname: string, item: MenuItem): boolean => { + return getActiveAncestors(pathname, [item]).length > 0; +}; + +/** + * Default expanded state for a sidenav branch (active path or section without a path). + */ +export const shouldExpandMenuItem = (pathname: string, item: MenuItem): boolean => { + if (!item.path) { + return true; + } + + return isMenuItemOnActiveBranch(pathname, item); +}; diff --git a/src/util/url.js b/src/util/url.js index fecb6589..6f406655 100644 --- a/src/util/url.js +++ b/src/util/url.js @@ -1,3 +1,15 @@ +/** + * Normalizes a URL path for comparison by stripping trailing slashes. + * Root path `/` is returned unchanged. + */ +export const normalizePath = (path) => { + if (path === '/') { + return path; + } + + return path.replace(/\/+$/, ''); +}; + /** * Joins fragments of a path together, ensuring there is a single slash between each fragment. * Optionally also ensures there is a trailing slash. diff --git a/src/util/url.test.ts b/src/util/url.test.ts new file mode 100644 index 00000000..6475b9da --- /dev/null +++ b/src/util/url.test.ts @@ -0,0 +1,12 @@ +import { normalizePath } from './url'; + +describe('normalizePath', () => { + test('strips trailing slashes', () => { + expect(normalizePath('/authentication/')).toEqual('/authentication'); + expect(normalizePath('/workspaces')).toEqual('/workspaces'); + }); + + test('leaves root path unchanged', () => { + expect(normalizePath('/')).toEqual('/'); + }); +}); From f066253af1c61076210264896feaee0cb6dcf190 Mon Sep 17 00:00:00 2001 From: Niall O'Brien Date: Fri, 29 May 2026 13:57:35 +0100 Subject: [PATCH 2/6] feat: add robust hash-based scroll and sidenav interaction - Implement hash-based scrolling with retries for layout transitions - Add context and hooks for managing sidenav hash state - Support sibling branch collapse and expand logic in sidenav - Refactor sidenav item actions for hash anchors and sibling branches - Add tests for hash scroll and sidenav interaction logic --- src/components/QuickNav/QuickNav.tsx | 33 --- src/components/QuickNav/hashScroll.test.ts | 52 +++++ src/components/QuickNav/hashScroll.ts | 111 +++++++++++ src/components/QuickNav/useHashScroll.ts | 22 ++ src/components/Sidenav/Sidenav.tsx | 91 +++------ src/components/Sidenav/SidenavHashContext.tsx | 62 ++++++ .../Sidenav/SidenavSiblingBranches.tsx | 58 ++++++ src/components/Sidenav/sidenavNavigation.ts | 30 +++ .../Sidenav/useSidenavItemActions.ts | 188 ++++++++++++++++++ src/components/WithQuickNav/WithQuicknav.tsx | 5 + src/lib/menu/sidenavInteractions.test.ts | 169 ++++++++++++++++ src/lib/menu/sidenavInteractions.ts | 136 +++++++++++++ src/lib/menu/util.test.ts | 36 ++++ src/lib/menu/util.ts | 106 +++++++++- src/lib/windowLocationHash.ts | 20 ++ 15 files changed, 1020 insertions(+), 99 deletions(-) create mode 100644 src/components/QuickNav/hashScroll.test.ts create mode 100644 src/components/QuickNav/hashScroll.ts create mode 100644 src/components/QuickNav/useHashScroll.ts create mode 100644 src/components/Sidenav/SidenavHashContext.tsx create mode 100644 src/components/Sidenav/SidenavSiblingBranches.tsx create mode 100644 src/components/Sidenav/sidenavNavigation.ts create mode 100644 src/components/Sidenav/useSidenavItemActions.ts create mode 100644 src/lib/menu/sidenavInteractions.test.ts create mode 100644 src/lib/menu/sidenavInteractions.ts create mode 100644 src/lib/windowLocationHash.ts diff --git a/src/components/QuickNav/QuickNav.tsx b/src/components/QuickNav/QuickNav.tsx index d1702a55..3891b768 100644 --- a/src/components/QuickNav/QuickNav.tsx +++ b/src/components/QuickNav/QuickNav.tsx @@ -3,7 +3,6 @@ import { useEffect, useState } from 'react'; import { cx } from 'class-variance-authority'; -import { usePathname } from 'next/navigation'; import { quickNavContentSelector } from '@/lib/constants/quickNav'; @@ -12,26 +11,7 @@ import { useHeadingsObserver } from './useHeadingsObserver'; const headingsToObserve = ':scope > :is(h2, h3, h4, h5, h6):not([data-quick-nav-ignore])'; -export const scrollToHashTarget = (hash = window.location.hash) => { - const normalizedHash = hash.replace(/^#/, ''); - if (!normalizedHash) return false; - - let targetId = normalizedHash; - try { - targetId = decodeURIComponent(normalizedHash); - } catch { - targetId = normalizedHash; - } - - const target = document.getElementById(targetId); - if (!target) return false; - - target.scrollIntoView({ block: 'start' }); - return true; -}; - export const QuickNav = () => { - const pathname = usePathname(); const [headings, setHeadings] = useState>([]); const activeHeadline = useHeadingsObserver( quickNavContentSelector, @@ -40,19 +20,6 @@ export const QuickNav = () => { 1, ); - useEffect(() => { - if (!headings.length) return; - - const queueHashScroll = () => window.requestAnimationFrame(() => scrollToHashTarget()); - - queueHashScroll(); - window.addEventListener('hashchange', queueHashScroll); - - return () => { - window.removeEventListener('hashchange', queueHashScroll); - }; - }, [headings.length, pathname]); - useEffect(() => { const contentArea = document.querySelector(quickNavContentSelector); if (!(contentArea instanceof HTMLElement)) return; diff --git a/src/components/QuickNav/hashScroll.test.ts b/src/components/QuickNav/hashScroll.test.ts new file mode 100644 index 00000000..9228bd4d --- /dev/null +++ b/src/components/QuickNav/hashScroll.test.ts @@ -0,0 +1,52 @@ +/** @jest-environment jsdom */ + +import { scheduleHashScroll } from './hashScroll'; + +describe('scheduleHashScroll', () => { + beforeEach(() => { + jest.useFakeTimers(); + document.body.innerHTML = ''; + jest.spyOn(window, 'requestAnimationFrame').mockImplementation((callback) => { + callback(0); + return 0; + }); + }); + + afterEach(() => { + jest.restoreAllMocks(); + jest.useRealTimers(); + }); + + test('skips post-layout retry when the target is missing', () => { + const scrollIntoView = jest.fn(); + const target = document.createElement('h2'); + target.id = 'developer-tools'; + target.scrollIntoView = scrollIntoView; + document.body.appendChild(target); + + scheduleHashScroll('#missing'); + jest.runAllTicks(); + jest.advanceTimersByTime(350); + + expect(scrollIntoView).not.toHaveBeenCalled(); + }); + + test('cancels a scheduled post-layout retry', () => { + const scrollIntoView = jest.fn(); + const target = document.createElement('h2'); + target.id = 'developer-tools'; + target.scrollIntoView = scrollIntoView; + document.body.appendChild(target); + + const cancel = scheduleHashScroll('#developer-tools'); + jest.runAllTicks(); + + const callsAfterFrames = scrollIntoView.mock.calls.length; + expect(callsAfterFrames).toBeGreaterThanOrEqual(1); + + cancel(); + jest.advanceTimersByTime(350); + + expect(scrollIntoView).toHaveBeenCalledTimes(callsAfterFrames); + }); +}); diff --git a/src/components/QuickNav/hashScroll.ts b/src/components/QuickNav/hashScroll.ts new file mode 100644 index 00000000..390ecb35 --- /dev/null +++ b/src/components/QuickNav/hashScroll.ts @@ -0,0 +1,111 @@ +const FRAME_RETRIES = 5; +/** Match Sidenav `openCloseTransition.duration` (0.35s). */ +const POST_LAYOUT_MS = 350; + +const withInstantScroll = (scroll: () => void) => { + const { documentElement } = document; + const previousScrollBehavior = documentElement.style.scrollBehavior; + documentElement.style.scrollBehavior = 'auto'; + + scroll(); + + documentElement.style.scrollBehavior = previousScrollBehavior; +}; + +export const scrollToHashTarget = (hash = window.location.hash) => { + const normalizedHash = hash.replace(/^#/, ''); + if (!normalizedHash) return false; + + let targetId = normalizedHash; + try { + targetId = decodeURIComponent(normalizedHash); + } catch { + targetId = normalizedHash; + } + + const target = document.getElementById(targetId); + if (!target) return false; + + withInstantScroll(() => { + target.scrollIntoView({ block: 'start' }); + }); + + return true; +}; + +export type CancelHashScroll = () => void; + +/** + * Scrolls to the hash target, retrying across frames and after sidenav layout animations. + * Sidenav expand/collapse can reflow the page after the first scroll attempt. + * + * A post-layout retry runs only when a frame retry finds the target (to correct scroll + * position after the branch animation). If the target is never found, no delayed retry runs. + * + * Returns a cancel function; call it when the hash or route changes before pending work runs. + */ +export const scheduleHashScroll = (hash = window.location.hash): CancelHashScroll => { + if (!hash) { + return () => {}; + } + + let cancelled = false; + let rafId = 0; + let timeoutId = 0; + + const cancel = () => { + cancelled = true; + if (rafId) { + cancelAnimationFrame(rafId); + rafId = 0; + } + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = 0; + } + }; + + const attempt = (): boolean => { + if (cancelled) { + return false; + } + + return scrollToHashTarget(hash); + }; + + const schedulePostLayoutRetry = () => { + if (cancelled || timeoutId) { + return; + } + + timeoutId = window.setTimeout(() => { + timeoutId = 0; + attempt(); + }, POST_LAYOUT_MS); + }; + + queueMicrotask(() => { + if (cancelled) { + return; + } + + const retryFrames = (remaining: number) => { + if (cancelled) { + return; + } + + if (attempt()) { + schedulePostLayoutRetry(); + return; + } + + if (remaining > 0) { + rafId = requestAnimationFrame(() => retryFrames(remaining - 1)); + } + }; + + retryFrames(FRAME_RETRIES); + }); + + return cancel; +}; diff --git a/src/components/QuickNav/useHashScroll.ts b/src/components/QuickNav/useHashScroll.ts new file mode 100644 index 00000000..60eec06f --- /dev/null +++ b/src/components/QuickNav/useHashScroll.ts @@ -0,0 +1,22 @@ +'use client'; + +import { useLayoutEffect } from 'react'; + +import { usePathname } from 'next/navigation'; + +import { useWindowHash } from '@/lib/windowLocationHash'; + +import { scheduleHashScroll } from './hashScroll'; + +/** + * Scrolls to the in-page heading for the current URL hash. + * Runs after React commit (useLayoutEffect) and retries after sidenav layout animations. + */ +export const useHashScroll = () => { + const pathname = usePathname(); + const hash = useWindowHash(); + + useLayoutEffect(() => { + return scheduleHashScroll(hash); + }, [pathname, hash]); +}; diff --git a/src/components/Sidenav/Sidenav.tsx b/src/components/Sidenav/Sidenav.tsx index 5247c799..b551412c 100644 --- a/src/components/Sidenav/Sidenav.tsx +++ b/src/components/Sidenav/Sidenav.tsx @@ -1,6 +1,6 @@ 'use client'; -import { type MouseEvent, useEffect, useState } from 'react'; +import { useEffect } from 'react'; import { cx } from 'class-variance-authority'; import { Transition, Variants } from 'motion/react'; @@ -12,15 +12,13 @@ import { useNavigation } from '@/app/navigation'; import { Icon } from '@/icons'; import { ChevronSmallIcon } from '@/icons/ChevronSmall'; import { MenuItem } from '@/lib/menu/types'; -import { - getActiveAncestors, - isActiveMenuLeaf, - isExactMenuPath, - shouldExpandMenuItem, -} from '@/lib/menu/util'; +import { getActiveAncestors } from '@/lib/menu/util'; import { last } from '@/lib/util'; import { Tag } from '../Tag'; +import { SidenavHashProvider } from './SidenavHashContext'; +import { SidenavSiblingBranchProvider } from './SidenavSiblingBranches'; +import { useSidenavItemActions } from './useSidenavItemActions'; import styles from './Sidenav.module.css'; const togglerTransition: Transition = { duration: 0.2, ease: 'easeInOut' }; @@ -74,9 +72,11 @@ export const Sidenav = ({ items }: SidenavProps) => { }} transition={togglerTransition}>
    - {items ? ( - - ) : null} + + {items ? ( + + ) : null} +
    @@ -85,13 +85,15 @@ export const Sidenav = ({ items }: SidenavProps) => { const MenuItemRows = ({ items, parentKey = '', listClassName }: MenuItemRowsProps) => { return ( -
      - {items.map((item, index) => { - const itemKey = buildMenuItemKey(parentKey, item, index); - - return ; - })} -
    + +
      + {items.map((item, index) => { + const itemKey = buildMenuItemKey(parentKey, item, index); + + return ; + })} +
    +     ); }; @@ -108,54 +110,12 @@ const List = ({ items, isExpanded, parentKey }: ListProps) => { ); }; -const Item = ({ item, itemKey }: ItemProps) => { - const pathname = usePathname(); - const { setNavigationState } = useNavigation(); - const isActiveLeaf = isActiveMenuLeaf(pathname, item); - const isExactPathMatch = isExactMenuPath(pathname, item); - const isHashAnchorParent = Boolean(item.children && item.path?.includes('#')); - const defaultExpanded = shouldExpandMenuItem(pathname, item); - const [isExpanded, setIsExpanded] = useState(defaultExpanded); - - useEffect(() => { - setIsExpanded(defaultExpanded); - }, [defaultExpanded]); - - const handleLinkClick = (event: MouseEvent) => { - const isMobileViewport = window.matchMedia('(max-width: 767px)').matches; - - // Hash-anchor parents should only expand/collapse in the sidenav. - if (isHashAnchorParent) { - event.preventDefault(); - setIsExpanded((expanded) => !expanded); - return; - } - - if (item.children && isExactPathMatch && isExpanded) { - event.preventDefault(); - setIsExpanded(false); - return; - } - - if (isActiveLeaf && isMobileViewport) { - event.preventDefault(); - } - - if (isMobileViewport && !isActiveLeaf) { - event.currentTarget.blur(); - setNavigationState('closed'); - } - - if (item.children && !isExpanded) { - setIsExpanded(true); - } - }; - - const handleChevronClick = (event: MouseEvent) => { - event.preventDefault(); - event.stopPropagation(); - setIsExpanded((expanded) => !expanded); - }; +const Item = ({ item, itemKey, siblingItems }: ItemProps) => { + const { isExpanded, isActiveLeaf, handleLinkClick, handleChevronClick } = useSidenavItemActions({ + item, + itemKey, + siblingItems, + }); return (
  • @@ -224,4 +184,5 @@ interface ListProps { interface ItemProps { item: MenuItem; itemKey: string; + siblingItems: MenuItem[]; } diff --git a/src/components/Sidenav/SidenavHashContext.tsx b/src/components/Sidenav/SidenavHashContext.tsx new file mode 100644 index 00000000..797dc070 --- /dev/null +++ b/src/components/Sidenav/SidenavHashContext.tsx @@ -0,0 +1,62 @@ +'use client'; + +import { + createContext, + type ReactNode, + useCallback, + useContext, + useEffect, + useMemo, + useState, +} from 'react'; + +import { usePathname } from 'next/navigation'; + +import { useWindowHash } from '@/lib/windowLocationHash'; + +type SidenavHashContextValue = { + hash: string; + setHash: (hash: string) => void; +}; + +const SidenavHashContext = createContext(null); + +/** + * Tracks the URL hash for sidenav expand/collapse logic. + * Context `hash` is the source of truth for interaction resolvers; it is seeded from + * `window.location.hash` on route changes and updated imperatively when the user + * opens or clears hash-anchor sections (see `useSidenavItemActions`). + */ +export const SidenavHashProvider = ({ children }: { children: ReactNode }) => { + const pathname = usePathname(); + const windowHash = useWindowHash(); + const [hashOverride, setHashOverride] = useState(null); + const hash = hashOverride ?? windowHash; + + const setHash = useCallback((nextHash: string) => { + setHashOverride(nextHash); + }, []); + + useEffect(() => { + setHashOverride(null); + }, [pathname]); + + useEffect(() => { + if (hashOverride !== null && hashOverride === windowHash) { + setHashOverride(null); + } + }, [hashOverride, windowHash]); + + const value = useMemo(() => ({ hash, setHash }), [hash, setHash]); + + return {children}; +}; + +export const useSidenavHash = () => { + const context = useContext(SidenavHashContext); + if (!context) { + throw new Error('useSidenavHash must be used within SidenavHashProvider'); + } + + return context; +}; diff --git a/src/components/Sidenav/SidenavSiblingBranches.tsx b/src/components/Sidenav/SidenavSiblingBranches.tsx new file mode 100644 index 00000000..0f7df7a6 --- /dev/null +++ b/src/components/Sidenav/SidenavSiblingBranches.tsx @@ -0,0 +1,58 @@ +'use client'; + +import { + createContext, + type ReactNode, + useCallback, + useContext, + useMemo, + useRef, +} from 'react'; + +type SiblingBranchContextValue = { + collapseSiblings: (exceptKey: string) => void; + registerBranch: (key: string, setExpanded: (expanded: boolean) => void) => () => void; +}; + +const SiblingBranchContext = createContext(null); + +export const useSiblingBranchContext = () => { + const context = useContext(SiblingBranchContext); + if (!context) { + throw new Error('useSiblingBranchContext must be used within SidenavSiblingBranchProvider'); + } + + return context; +}; + +type SidenavSiblingBranchProviderProps = { + children: ReactNode; +}; + +/** Registers expand/collapse state for sibling branches at one list level. */ +export const SidenavSiblingBranchProvider = ({ children }: SidenavSiblingBranchProviderProps) => { + const branchSettersRef = useRef(new Map void>()); + + const registerBranch = useCallback((key: string, setExpanded: (expanded: boolean) => void) => { + branchSettersRef.current.set(key, setExpanded); + + return () => { + branchSettersRef.current.delete(key); + }; + }, []); + + const collapseSiblings = useCallback((exceptKey: string) => { + branchSettersRef.current.forEach((setExpanded, key) => { + if (key !== exceptKey) { + setExpanded(false); + } + }); + }, []); + + const value = useMemo( + () => ({ collapseSiblings, registerBranch }), + [collapseSiblings, registerBranch], + ); + + return {children}; +}; diff --git a/src/components/Sidenav/sidenavNavigation.ts b/src/components/Sidenav/sidenavNavigation.ts new file mode 100644 index 00000000..0b141fab --- /dev/null +++ b/src/components/Sidenav/sidenavNavigation.ts @@ -0,0 +1,30 @@ +import { normalizePath } from '@/util/url'; + +type SidenavRouter = { + push: (href: string) => void; +}; + +const scrollToDocumentTop = () => { + window.requestAnimationFrame(() => { + const { documentElement } = document; + const previousScrollBehavior = documentElement.style.scrollBehavior; + documentElement.style.scrollBehavior = 'auto'; + + window.scrollTo(0, 0); + + documentElement.style.scrollBehavior = previousScrollBehavior; + }); +}; + +/** Navigates to a path and scrolls to top (e.g. when clearing a hash fragment). */ +export const navigateToSidenavPath = (router: SidenavRouter, path: string) => { + router.push(path); + scrollToDocumentTop(); +}; + +/** Replaces the current URL with the base path when already on it, clearing any hash. */ +export const clearUrlHashOnPage = (router: SidenavRouter, pathname: string, basePath: string) => { + if (normalizePath(pathname) === basePath) { + navigateToSidenavPath(router, basePath); + } +}; diff --git a/src/components/Sidenav/useSidenavItemActions.ts b/src/components/Sidenav/useSidenavItemActions.ts new file mode 100644 index 00000000..686db57b --- /dev/null +++ b/src/components/Sidenav/useSidenavItemActions.ts @@ -0,0 +1,188 @@ +'use client'; + +import { type MouseEvent, useCallback, useEffect, useState } from 'react'; + +import { usePathname, useRouter } from 'next/navigation'; + +import { useNavigation } from '@/app/navigation'; +import { MenuItem } from '@/lib/menu/types'; +import { + resolveSidenavChevronClick, + resolveSidenavLinkClick, + type SidenavChevronClickAction, + type SidenavLinkClickAction, +} from '@/lib/menu/sidenavInteractions'; +import { + getMenuItemBasePath, + getMenuItemHash, + isActiveHashAnchorSibling, + isActiveMenuLeaf, + shouldExpandMenuItem, +} from '@/lib/menu/util'; + +import { clearUrlHashOnPage, navigateToSidenavPath } from './sidenavNavigation'; +import { useSidenavHash } from './SidenavHashContext'; +import { useSiblingBranchContext } from './SidenavSiblingBranches'; + +const MOBILE_MEDIA_QUERY = '(max-width: 767px)'; + +type UseSidenavItemActionsParams = { + item: MenuItem; + itemKey: string; + siblingItems: MenuItem[]; +}; + +type SidenavItemAction = SidenavLinkClickAction | SidenavChevronClickAction; + +export const useSidenavItemActions = ({ item, itemKey, siblingItems }: UseSidenavItemActionsParams) => { + const pathname = usePathname(); + const router = useRouter(); + const { hash, setHash } = useSidenavHash(); + const siblingBranch = useSiblingBranchContext(); + const { setNavigationState } = useNavigation(); + const isActiveLeaf = isActiveMenuLeaf(pathname, item); + const defaultExpanded = shouldExpandMenuItem(pathname, item, hash, siblingItems); + const [isExpanded, setIsExpanded] = useState(defaultExpanded); + + useEffect(() => { + return siblingBranch.registerBranch(itemKey, setIsExpanded); + }, [itemKey, siblingBranch]); + + useEffect(() => { + setIsExpanded(defaultExpanded); + }, [defaultExpanded, pathname, hash]); + + const openHashAnchorSection = useCallback(() => { + if (!item.path) { + return; + } + + const itemHash = getMenuItemHash(item.path); + if (itemHash) { + setHash(`#${itemHash}`); + } + + siblingBranch.collapseSiblings(itemKey); + setIsExpanded(true); + router.push(item.path); + }, [item.path, itemKey, router, setHash, siblingBranch]); + + const openBranchSection = useCallback( + (clearHash: boolean) => { + if (clearHash && item.path) { + setHash(''); + clearUrlHashOnPage(router, pathname, getMenuItemBasePath(item.path)); + } + + siblingBranch.collapseSiblings(itemKey); + setIsExpanded(true); + }, + [item.path, itemKey, pathname, router, setHash, siblingBranch], + ); + + const collapseClearHash = useCallback( + (collapseSiblingsOnClose: boolean) => { + const shouldClearUrlHash = Boolean( + item.path && isActiveHashAnchorSibling(pathname, item, hash), + ); + + setHash(''); + + if (shouldClearUrlHash && item.path) { + navigateToSidenavPath(router, getMenuItemBasePath(item.path)); + } + + if (collapseSiblingsOnClose) { + siblingBranch.collapseSiblings(itemKey); + } + + setIsExpanded(false); + }, + [hash, item, itemKey, pathname, router, setHash, siblingBranch], + ); + + const getInteractionContext = useCallback( + (isMobileViewport: boolean) => ({ + pathname, + hash, + item, + siblingItems, + isExpanded, + isMobileViewport, + }), + [hash, isExpanded, item, pathname, siblingItems], + ); + + const applyAction = useCallback( + (action: SidenavItemAction) => { + switch (action.kind) { + case 'collapse': + setIsExpanded(false); + break; + case 'collapse-clear-hash': + collapseClearHash(action.collapseSiblings); + break; + case 'open-hash-anchor': + openHashAnchorSection(); + break; + case 'open-sibling-branch': + openBranchSection(action.clearHash); + break; + case 'expand-branch': + setIsExpanded(true); + break; + case 'toggle': + setIsExpanded((expanded) => !expanded); + break; + case 'navigate': + break; + } + }, + [collapseClearHash, openBranchSection, openHashAnchorSection], + ); + + const handleLinkClick = useCallback( + (event: MouseEvent) => { + const isMobileViewport = window.matchMedia(MOBILE_MEDIA_QUERY).matches; + const { action, preventDefault, closeMobileNav, blurLink } = resolveSidenavLinkClick( + getInteractionContext(isMobileViewport), + ); + + if (preventDefault) { + event.preventDefault(); + } + + if (blurLink) { + event.currentTarget.blur(); + } + + if (closeMobileNav) { + setNavigationState('closed'); + } + + applyAction(action); + }, + [applyAction, getInteractionContext, setNavigationState], + ); + + const handleChevronClick = useCallback( + (event: MouseEvent) => { + event.preventDefault(); + event.stopPropagation(); + + const action = resolveSidenavChevronClick( + getInteractionContext(window.matchMedia(MOBILE_MEDIA_QUERY).matches), + ); + + applyAction(action); + }, + [applyAction, getInteractionContext], + ); + + return { + isExpanded, + isActiveLeaf, + handleLinkClick, + handleChevronClick, + }; +}; diff --git a/src/components/WithQuickNav/WithQuicknav.tsx b/src/components/WithQuickNav/WithQuicknav.tsx index 39fbc07c..10e8c13c 100644 --- a/src/components/WithQuickNav/WithQuicknav.tsx +++ b/src/components/WithQuickNav/WithQuicknav.tsx @@ -1,5 +1,8 @@ +'use client'; + import { Icon } from '@/icons'; +import { useHashScroll } from '../QuickNav/useHashScroll'; import { PageInfo } from '../PageInfo'; import { QuickNav } from '../QuickNav'; import styles from './WithQuicknav.module.css'; @@ -10,6 +13,8 @@ import styles from './WithQuicknav.module.css'; */ const WithQuicknav = ({ children, showPageInfo = false, path = '', lastUpdated }: WithQuicknavProps) => { + useHashScroll(); + return (
    diff --git a/src/lib/menu/sidenavInteractions.test.ts b/src/lib/menu/sidenavInteractions.test.ts new file mode 100644 index 00000000..e6180413 --- /dev/null +++ b/src/lib/menu/sidenavInteractions.test.ts @@ -0,0 +1,169 @@ +import type { MenuItem } from './types'; +import type { SidenavItemInteractionContext, SidenavLinkClickResult } from './sidenavInteractions'; +import { resolveSidenavChevronClick, resolveSidenavLinkClick } from './sidenavInteractions'; + +const aboutCloudsmith: MenuItem = { + title: 'About Cloudsmith', + path: '/about-cloudsmith', + children: [{ title: 'Key Concepts', path: '/about-cloudsmith/key-concepts' }], +}; + +const developerTools: MenuItem = { + title: 'Developer Tools', + path: '/about-cloudsmith#developer-tools', + children: [{ title: 'Cloudsmith CLI', path: '/developer-tools/cli' }], +}; + +const hashAnchorSiblings = [aboutCloudsmith, developerTools]; + +describe('resolveSidenavLinkClick', () => { + test.each<[string, SidenavItemInteractionContext, SidenavLinkClickResult]>([ + [ + 'collapse active hash-anchor', + { + pathname: '/about-cloudsmith', + hash: '#developer-tools', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: true, + isMobileViewport: false, + }, + { + action: { kind: 'collapse-clear-hash', collapseSiblings: false }, + preventDefault: true, + closeMobileNav: false, + blurLink: false, + }, + ], + [ + 'open hash-anchor', + { + pathname: '/about-cloudsmith', + hash: '', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + { + action: { kind: 'open-hash-anchor' }, + preventDefault: true, + closeMobileNav: false, + blurLink: false, + }, + ], + [ + 'open hash-anchor from child route', + { + pathname: '/developer-tools/terraform-provider', + hash: '', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: true, + isMobileViewport: false, + }, + { + action: { kind: 'open-hash-anchor' }, + preventDefault: true, + closeMobileNav: false, + blurLink: false, + }, + ], + [ + 'collapse non-hash sibling', + { + pathname: '/about-cloudsmith', + hash: '', + item: aboutCloudsmith, + siblingItems: hashAnchorSiblings, + isExpanded: true, + isMobileViewport: false, + }, + { + action: { kind: 'collapse' }, + preventDefault: true, + closeMobileNav: false, + blurLink: false, + }, + ], + [ + 'open sibling and clear hash', + { + pathname: '/about-cloudsmith', + hash: '#developer-tools', + item: aboutCloudsmith, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + { + action: { kind: 'open-sibling-branch', clearHash: true }, + preventDefault: true, + closeMobileNav: false, + blurLink: false, + }, + ], + [ + 'open sibling without clearing hash', + { + pathname: '/about-cloudsmith', + hash: '', + item: aboutCloudsmith, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + { + action: { kind: 'open-sibling-branch', clearHash: false }, + preventDefault: true, + closeMobileNav: false, + blurLink: false, + }, + ], + ])('%s', (_name, context, expected) => { + expect(resolveSidenavLinkClick(context)).toEqual(expected); + }); +}); + +describe('resolveSidenavChevronClick', () => { + test.each([ + [ + 'open hash-anchor', + { + pathname: '/about-cloudsmith', + hash: '', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + { kind: 'open-hash-anchor' }, + ], + [ + 'collapse hash-anchor and siblings', + { + pathname: '/developer-tools/cli', + hash: '', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: true, + isMobileViewport: false, + }, + { kind: 'collapse-clear-hash', collapseSiblings: true }, + ], + [ + 'open sibling and clear hash', + { + pathname: '/about-cloudsmith', + hash: '#developer-tools', + item: aboutCloudsmith, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + { kind: 'open-sibling-branch', clearHash: true }, + ], + ] as const)('%s', (_name, context, expected) => { + expect(resolveSidenavChevronClick(context)).toEqual(expected); + }); +}); diff --git a/src/lib/menu/sidenavInteractions.ts b/src/lib/menu/sidenavInteractions.ts new file mode 100644 index 00000000..5634fdae --- /dev/null +++ b/src/lib/menu/sidenavInteractions.ts @@ -0,0 +1,136 @@ +import type { MenuItem } from './types'; +import { + hasHashAnchorSiblingOnSameBase, + isActiveHashAnchorSibling, + isActiveMenuLeaf, + isAnyHashAnchorSiblingActive, + isExactMenuPath, + isHashAnchorParent, + isMenuItemOnActiveBranch, +} from './util'; + +export type SidenavItemInteractionContext = { + pathname: string; + hash: string; + item: MenuItem; + siblingItems: MenuItem[]; + isExpanded: boolean; + isMobileViewport: boolean; +}; + +export type SidenavLinkClickAction = + | { kind: 'navigate' } + | { kind: 'collapse' } + | { kind: 'collapse-clear-hash'; collapseSiblings: boolean } + | { kind: 'open-hash-anchor' } + | { kind: 'open-sibling-branch'; clearHash: boolean } + | { kind: 'expand-branch' }; + +export type SidenavLinkClickResult = { + action: SidenavLinkClickAction; + preventDefault: boolean; + closeMobileNav: boolean; + blurLink: boolean; +}; + +export type SidenavChevronClickAction = + | { kind: 'open-hash-anchor' } + | { kind: 'collapse-clear-hash'; collapseSiblings: boolean } + | { kind: 'open-sibling-branch'; clearHash: boolean } + | { kind: 'toggle' }; + +/** + * Resolves how a sidenav item link click should behave. + */ +export const resolveSidenavLinkClick = ( + ctx: SidenavItemInteractionContext, +): SidenavLinkClickResult => { + const { pathname, hash, item, siblingItems, isExpanded, isMobileViewport } = ctx; + const isActiveLeaf = isActiveMenuLeaf(pathname, item); + const isExactPathMatch = isExactMenuPath(pathname, item); + const isHashAnchor = isHashAnchorParent(item); + const isOnActiveBranch = isMenuItemOnActiveBranch(pathname, item); + const sharesBaseWithHashAnchorSibling = hasHashAnchorSiblingOnSameBase(item, siblingItems); + const hashSiblingIsActive = isAnyHashAnchorSiblingActive(pathname, hash, siblingItems); + + const idle = (action: SidenavLinkClickAction): SidenavLinkClickResult => ({ + action, + preventDefault: false, + closeMobileNav: false, + blurLink: false, + }); + + if (isHashAnchor) { + if (isExpanded && isActiveHashAnchorSibling(pathname, item, hash)) { + return { + ...idle({ kind: 'collapse-clear-hash', collapseSiblings: false }), + preventDefault: true, + }; + } + + return { + action: { kind: 'open-hash-anchor' }, + preventDefault: true, + closeMobileNav: isMobileViewport && !isOnActiveBranch, + blurLink: isMobileViewport && !isOnActiveBranch, + }; + } + + if (sharesBaseWithHashAnchorSibling && item.children) { + if (isExactPathMatch && isExpanded && !hashSiblingIsActive) { + return { ...idle({ kind: 'collapse' }), preventDefault: true }; + } + + if (hashSiblingIsActive || !isExpanded) { + return { + action: { kind: 'open-sibling-branch', clearHash: hashSiblingIsActive }, + preventDefault: hashSiblingIsActive || isExactPathMatch, + closeMobileNav: isMobileViewport && !isActiveLeaf, + blurLink: isMobileViewport && !isActiveLeaf, + }; + } + } + + if (item.children && isExactPathMatch && isExpanded) { + return { ...idle({ kind: 'collapse' }), preventDefault: true }; + } + + if (isActiveLeaf && isMobileViewport) { + return { ...idle({ kind: 'navigate' }), preventDefault: true }; + } + + return { + action: item.children && !isExpanded ? { kind: 'expand-branch' } : { kind: 'navigate' }, + preventDefault: false, + closeMobileNav: isMobileViewport && !isActiveLeaf, + blurLink: isMobileViewport && !isActiveLeaf, + }; +}; + +/** + * Resolves how a sidenav item chevron click should behave. + * Chevron collapse-clear-hash collapses same-level siblings; link clicks do not. + */ +export const resolveSidenavChevronClick = ( + ctx: SidenavItemInteractionContext, +): SidenavChevronClickAction => { + const { pathname, hash, item, siblingItems, isExpanded } = ctx; + const isOnActiveBranch = isMenuItemOnActiveBranch(pathname, item); + const isHashAnchor = isHashAnchorParent(item); + const sharesBaseWithHashAnchorSibling = hasHashAnchorSiblingOnSameBase(item, siblingItems); + const hashSiblingIsActive = isAnyHashAnchorSiblingActive(pathname, hash, siblingItems); + + if (isHashAnchor && item.path) { + if (!isOnActiveBranch || !isExpanded) { + return { kind: 'open-hash-anchor' }; + } + + return { kind: 'collapse-clear-hash', collapseSiblings: true }; + } + + if (sharesBaseWithHashAnchorSibling && !isExpanded) { + return { kind: 'open-sibling-branch', clearHash: hashSiblingIsActive }; + } + + return { kind: 'toggle' }; +}; diff --git a/src/lib/menu/util.test.ts b/src/lib/menu/util.test.ts index e4c8d842..b4c6f572 100644 --- a/src/lib/menu/util.test.ts +++ b/src/lib/menu/util.test.ts @@ -9,6 +9,20 @@ import { shouldExpandMenuItem, } from './util'; +const aboutCloudsmith: MenuItem = { + title: 'About Cloudsmith', + path: '/about-cloudsmith', + children: [{ title: 'Key Concepts', path: '/about-cloudsmith/key-concepts' }], +}; + +const developerTools: MenuItem = { + title: 'Developer Tools', + path: '/about-cloudsmith#developer-tools', + children: [{ title: 'Cloudsmith CLI', path: '/developer-tools/cli' }], +}; + +const hashAnchorSiblings = [aboutCloudsmith, developerTools]; + describe('lib', () => { describe('menu', () => { describe('getActiveMenuItem()', () => { @@ -60,6 +74,15 @@ describe('lib', () => { const item: MenuItem = { title: 'Authentication', path: '/authentication/' }; expect(isExactMenuPath('/authentication', item)).toBe(true); }); + + test('does not match bare pathname for hash-anchor menu paths', () => { + const item: MenuItem = { + title: 'Developer Tools', + path: '/about-cloudsmith#developer-tools', + children: [], + }; + expect(isExactMenuPath('/about-cloudsmith', item)).toBe(false); + }); }); describe('isActiveMenuLeaf()', () => { @@ -91,6 +114,19 @@ describe('lib', () => { expect(shouldExpandMenuItem('/authentication', workspacesMenu)).toBe(true); }); + + test('expands hash-anchor and sibling branches for the active hash', () => { + expect( + shouldExpandMenuItem('/about-cloudsmith', developerTools, '#developer-tools', hashAnchorSiblings), + ).toBe(true); + expect( + shouldExpandMenuItem('/about-cloudsmith', aboutCloudsmith, '#developer-tools', hashAnchorSiblings), + ).toBe(false); + expect(shouldExpandMenuItem('/about-cloudsmith', aboutCloudsmith, '', hashAnchorSiblings)).toBe( + true, + ); + }); }); + }); }); diff --git a/src/lib/menu/util.ts b/src/lib/menu/util.ts index 79b5dbb0..c1dac2b7 100644 --- a/src/lib/menu/util.ts +++ b/src/lib/menu/util.ts @@ -75,6 +75,9 @@ export const getActiveAncestors = ( /** * Whether the pathname exactly matches this menu item's path. + * Compares full paths as stored in the menu; hash fragments in `item.path` are not + * stripped, so hash-anchor parents (e.g. `/page#section`) never match a bare pathname. + * Use `getMenuItemBasePath` and `isActiveHashAnchorSibling` for hash-anchor behavior. */ export const isExactMenuPath = (pathname: string, item: MenuItem): boolean => { if (!item.path) { @@ -98,13 +101,114 @@ export const isMenuItemOnActiveBranch = (pathname: string, item: MenuItem): bool return getActiveAncestors(pathname, [item]).length > 0; }; +/** + * Parent menu item whose path is a same-page hash anchor (e.g. `/page#section`). + * Matches on `#` in the menu path only; not suitable if paths gain query strings. + */ +export const isHashAnchorParent = (item: MenuItem): boolean => { + return Boolean(item.children && item.path?.includes('#')); +}; + +/** Path portion of a menu path, without a hash fragment. */ +export const getMenuItemBasePath = (path: string): string => { + return normalizePath(path.split('#')[0] ?? path); +}; + +/** Hash fragment of a menu path, without the leading `#`. */ +export const getMenuItemHash = (path: string): string | undefined => { + const hashIndex = path.indexOf('#'); + if (hashIndex === -1) { + return undefined; + } + + return path.slice(hashIndex + 1); +}; + +/** Normalizes a hash value to a fragment without a leading `#`. */ +export const normalizeHashFragment = (hash: string): string => hash.replace(/^#/, ''); + +/** Whether this branch shares a base path with a hash-anchor sibling. */ +export const hasHashAnchorSiblingOnSameBase = ( + item: MenuItem, + siblingItems: MenuItem[], +): boolean => { + if (!item.path || !item.children || isHashAnchorParent(item)) { + return false; + } + + const basePath = getMenuItemBasePath(item.path); + + return siblingItems.some( + (sibling) => + isHashAnchorParent(sibling) && + sibling.path && + getMenuItemBasePath(sibling.path) === basePath, + ); +}; + +/** Whether any hash-anchor sibling is active for the current location. */ +export const isAnyHashAnchorSiblingActive = ( + pathname: string, + hash: string, + siblingItems: MenuItem[], +): boolean => { + return siblingItems.some((sibling) => isActiveHashAnchorSibling(pathname, sibling, hash)); +}; + +/** Whether a hash-anchor sibling is the active section for the current location. */ +export const isActiveHashAnchorSibling = ( + pathname: string, + item: MenuItem, + hash: string, +): boolean => { + if (!isHashAnchorParent(item) || !item.path) { + return false; + } + + const itemHash = getMenuItemHash(item.path); + const normalizedHash = normalizeHashFragment(hash); + + return Boolean( + itemHash && + normalizedHash === itemHash && + normalizePath(pathname) === getMenuItemBasePath(item.path), + ); +}; + /** * Default expanded state for a sidenav branch (active path or section without a path). */ -export const shouldExpandMenuItem = (pathname: string, item: MenuItem): boolean => { +export const shouldExpandMenuItem = ( + pathname: string, + item: MenuItem, + hash = '', + siblingItems: MenuItem[] = [], +): boolean => { if (!item.path) { return true; } + const normalizedHash = normalizeHashFragment(hash); + const activeHashSibling = siblingItems.find((sibling) => + isActiveHashAnchorSibling(pathname, sibling, normalizedHash), + ); + + if (isHashAnchorParent(item)) { + if (isMenuItemOnActiveBranch(pathname, item)) { + return true; + } + + return isActiveHashAnchorSibling(pathname, item, normalizedHash); + } + + const activeHashSiblingPath = activeHashSibling?.path; + if ( + activeHashSiblingPath && + normalizePath(pathname) === getMenuItemBasePath(item.path) && + normalizePath(pathname) === getMenuItemBasePath(activeHashSiblingPath) + ) { + return false; + } + return isMenuItemOnActiveBranch(pathname, item); }; diff --git a/src/lib/windowLocationHash.ts b/src/lib/windowLocationHash.ts new file mode 100644 index 00000000..9a9e03ef --- /dev/null +++ b/src/lib/windowLocationHash.ts @@ -0,0 +1,20 @@ +'use client'; + +import { useSyncExternalStore } from 'react'; + +/** Subscribes to URL hash changes (address bar, history, in-app navigation). */ +export const subscribeToWindowHash = (onStoreChange: () => void) => { + window.addEventListener('hashchange', onStoreChange); + window.addEventListener('popstate', onStoreChange); + + return () => { + window.removeEventListener('hashchange', onStoreChange); + window.removeEventListener('popstate', onStoreChange); + }; +}; + +export const getWindowHash = () => window.location.hash; + +/** Subscribes to `window.location.hash` (SSR snapshot is empty). */ +export const useWindowHash = () => + useSyncExternalStore(subscribeToWindowHash, getWindowHash, () => ''); From 152829f024d2ecce2283cf01e7851edf779e35f2 Mon Sep 17 00:00:00 2001 From: Niall O'Brien Date: Fri, 29 May 2026 14:41:35 +0100 Subject: [PATCH 3/6] refactor: improve active menu ancestor resolution and hash scroll - Add getCanonicalActiveAncestors to resolve a single active menu branch when multiple menu items share a path - Refactor Sidenav to use SidenavActiveMenuContext for ancestor tracking - Update hash scroll logic to handle top scroll and deferred scroll on navigation - Add resolveHashScrollEffect for explicit scroll behavior decisions - Improve sidenav navigation to clear hash without scrolling when appropriate - Update and add tests for new menu and scroll behaviors --- src/app/(api)/api/[...slug]/page.tsx | 4 +- src/app/(documentation)/[...slug]/page.tsx | 4 +- src/app/(guides)/guides/[...slug]/page.tsx | 4 +- src/app/navigation.tsx | 19 +- src/components/QuickNav/hashScroll.test.ts | 30 ++- src/components/QuickNav/hashScroll.ts | 89 +++++++-- .../QuickNav/resolveHashScrollEffect.test.ts | 27 +++ .../QuickNav/resolveHashScrollEffect.ts | 25 +++ src/components/QuickNav/useHashScroll.ts | 36 +++- src/components/Sidenav/Sidenav.tsx | 24 ++- .../Sidenav/SidenavActiveMenuContext.tsx | 41 ++++ src/components/Sidenav/SidenavHashContext.tsx | 3 +- src/components/Sidenav/sidenavNavigation.ts | 36 ++-- .../Sidenav/useSidenavItemActions.ts | 14 +- src/lib/menu/sidenavInteractions.test.ts | 180 +++++++++++------- src/lib/menu/sidenavInteractions.ts | 31 +-- src/lib/menu/util.test.ts | 102 +++++++++- src/lib/menu/util.ts | 170 ++++++++++++++++- 18 files changed, 660 insertions(+), 179 deletions(-) create mode 100644 src/components/QuickNav/resolveHashScrollEffect.test.ts create mode 100644 src/components/QuickNav/resolveHashScrollEffect.ts create mode 100644 src/components/Sidenav/SidenavActiveMenuContext.tsx diff --git a/src/app/(api)/api/[...slug]/page.tsx b/src/app/(api)/api/[...slug]/page.tsx index 66776885..ad1ff15c 100644 --- a/src/app/(api)/api/[...slug]/page.tsx +++ b/src/app/(api)/api/[...slug]/page.tsx @@ -7,7 +7,7 @@ import { ApiRequest, ApiResponses, Heading, Link, Note, Paragraph, Tag, TimeAgo import WithQuicknav from '@/components/WithQuickNav'; import { Icon } from '@/icons'; import { loadMdxInfo } from '@/lib/markdown/util'; -import { getActiveAncestors, getMenuItem } from '@/lib/menu/util'; +import { getCanonicalActiveAncestors, getMenuItem } from '@/lib/menu/util'; import { getLastUpdated, withDefaultMetadata, withMdxMetadata } from '@/lib/metadata/util'; import { parseSchemas, toOperations } from '@/lib/swagger/parse'; import { toRouteSegments, toSlug } from '@/lib/util'; @@ -78,7 +78,7 @@ const Page = async ({ params }: PageProps) => { const pathname = `${qualifiedSlug}`; const menuData = getMenuItem('api'); - const ancestors = getActiveAncestors(pathname, [menuData]); + const ancestors = getCanonicalActiveAncestors(pathname, [menuData]); const parentTitle = ancestors.length > 1 ? ancestors[ancestors.length - 2].title : null; if (mdxInfo) { diff --git a/src/app/(documentation)/[...slug]/page.tsx b/src/app/(documentation)/[...slug]/page.tsx index 348328f6..a010e99f 100644 --- a/src/app/(documentation)/[...slug]/page.tsx +++ b/src/app/(documentation)/[...slug]/page.tsx @@ -4,7 +4,7 @@ import { notFound } from 'next/navigation'; import type { Metadata } from 'next'; import { withMdxMetadata, withDefaultMetadata, getLastUpdated } from '@/lib/metadata/util'; import { TimeAgo } from '@/components'; -import { getMenuItem, getActiveAncestors } from '@/lib/menu/util'; +import { getMenuItem, getCanonicalActiveAncestors } from '@/lib/menu/util'; import WithQuicknav from '@/components/WithQuickNav'; import { cx } from 'class-variance-authority'; @@ -51,7 +51,7 @@ const Page = async ({ params }: PageProps) => { // 2. Original logic: Get parentTitle from the menu system. const pathname = `/${qualifiedSlug}`; const menuData = getMenuItem('documentation'); - const ancestors = getActiveAncestors(pathname, [menuData]); + const ancestors = getCanonicalActiveAncestors(pathname, [menuData]); const menuParentTitle = ancestors.length > 1 ? ancestors[ancestors.length - 2].title : null; // 3. Prioritize the title from the MDX file, then fall back to the menu. diff --git a/src/app/(guides)/guides/[...slug]/page.tsx b/src/app/(guides)/guides/[...slug]/page.tsx index f43e87cf..3437d149 100644 --- a/src/app/(guides)/guides/[...slug]/page.tsx +++ b/src/app/(guides)/guides/[...slug]/page.tsx @@ -4,7 +4,7 @@ import { notFound } from 'next/navigation'; import type { Metadata } from 'next'; import { withMdxMetadata, withDefaultMetadata, getLastUpdated } from '@/lib/metadata/util'; import { TimeAgo } from '@/components'; -import { getMenuItem, getActiveAncestors } from '@/lib/menu/util'; +import { getMenuItem, getCanonicalActiveAncestors } from '@/lib/menu/util'; import WithQuicknav from '@/components/WithQuickNav'; import { cx } from 'class-variance-authority'; @@ -50,7 +50,7 @@ const Page = async ({ params }: PageProps) => { const pathname = `/${qualifiedSlug}`; const menuData = getMenuItem('guides'); - const ancestors = getActiveAncestors(pathname, [menuData]); + const ancestors = getCanonicalActiveAncestors(pathname, [menuData]); const parentTitle = ancestors.length > 1 ? ancestors[ancestors.length - 2].title : null; return ( diff --git a/src/app/navigation.tsx b/src/app/navigation.tsx index 5dce8480..897f3006 100644 --- a/src/app/navigation.tsx +++ b/src/app/navigation.tsx @@ -1,6 +1,6 @@ 'use client'; -import { createContext, useContext, useEffect, useRef, useState } from 'react'; +import { createContext, useContext, useEffect, useState } from 'react'; import { usePathname } from 'next/navigation'; @@ -31,24 +31,11 @@ export const useNavigation = () => { export const NavigationEvents = () => { const pathname = usePathname(); const { setNavigationState } = useNavigation(); - const previousPathname = useRef(pathname); - // Close the navigation when the pathname changes + // Close the navigation when the pathname changes. + // Scroll-to-top and hash scrolling run in `useHashScroll` inside `WithQuicknav` only. useEffect(() => { setNavigationState('closed'); - - if (previousPathname.current !== pathname) { - // Disable smooth scroll so the top-reset is instant, then restore it - document.documentElement.style.scrollBehavior = 'auto'; - window.requestAnimationFrame(() => { - window.scrollTo(0, 0); - window.requestAnimationFrame(() => { - document.documentElement.style.scrollBehavior = ''; - }); - }); - } - - previousPathname.current = pathname; }, [pathname, setNavigationState]); return null; diff --git a/src/components/QuickNav/hashScroll.test.ts b/src/components/QuickNav/hashScroll.test.ts index 9228bd4d..720a6d47 100644 --- a/src/components/QuickNav/hashScroll.test.ts +++ b/src/components/QuickNav/hashScroll.test.ts @@ -1,6 +1,6 @@ /** @jest-environment jsdom */ -import { scheduleHashScroll } from './hashScroll'; +import { scheduleDeferredScrollToTop, scheduleHashScroll, scheduleScrollToTop } from './hashScroll'; describe('scheduleHashScroll', () => { beforeEach(() => { @@ -31,6 +31,34 @@ describe('scheduleHashScroll', () => { expect(scrollIntoView).not.toHaveBeenCalled(); }); + test('scheduleScrollToTop scrolls to the document top with a post-layout retry', () => { + const scrollTo = jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); + + scheduleScrollToTop(); + jest.runAllTicks(); + expect(scrollTo).toHaveBeenCalled(); + + const callsAfterFrames = scrollTo.mock.calls.length; + jest.advanceTimersByTime(350); + expect(scrollTo.mock.calls.length).toBeGreaterThan(callsAfterFrames); + + scrollTo.mockRestore(); + }); + + test('scheduleDeferredScrollToTop respects hash presence after the frame', () => { + const scrollTo = jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); + + scheduleDeferredScrollToTop(() => true); + jest.runAllTicks(); + expect(scrollTo).not.toHaveBeenCalled(); + + scheduleDeferredScrollToTop(() => false); + jest.runAllTicks(); + expect(scrollTo).toHaveBeenCalled(); + + scrollTo.mockRestore(); + }); + test('cancels a scheduled post-layout retry', () => { const scrollIntoView = jest.fn(); const target = document.createElement('h2'); diff --git a/src/components/QuickNav/hashScroll.ts b/src/components/QuickNav/hashScroll.ts index 390ecb35..9f427a24 100644 --- a/src/components/QuickNav/hashScroll.ts +++ b/src/components/QuickNav/hashScroll.ts @@ -12,6 +12,12 @@ const withInstantScroll = (scroll: () => void) => { documentElement.style.scrollBehavior = previousScrollBehavior; }; +export const scrollToDocumentTop = () => { + withInstantScroll(() => { + window.scrollTo(0, 0); + }); +}; + export const scrollToHashTarget = (hash = window.location.hash) => { const normalizedHash = hash.replace(/^#/, ''); if (!normalizedHash) return false; @@ -35,20 +41,16 @@ export const scrollToHashTarget = (hash = window.location.hash) => { export type CancelHashScroll = () => void; +type PostLayoutRetry = 'always' | 'onAttemptSuccess'; + /** - * Scrolls to the hash target, retrying across frames and after sidenav layout animations. - * Sidenav expand/collapse can reflow the page after the first scroll attempt. - * - * A post-layout retry runs only when a frame retry finds the target (to correct scroll - * position after the branch animation). If the target is never found, no delayed retry runs. - * + * Runs `attempt` across animation frames and optionally after sidenav layout animations. * Returns a cancel function; call it when the hash or route changes before pending work runs. */ -export const scheduleHashScroll = (hash = window.location.hash): CancelHashScroll => { - if (!hash) { - return () => {}; - } - +const scheduleScrollRetries = ( + attempt: () => boolean, + postLayout: PostLayoutRetry, +): CancelHashScroll => { let cancelled = false; let rafId = 0; let timeoutId = 0; @@ -65,12 +67,12 @@ export const scheduleHashScroll = (hash = window.location.hash): CancelHashScrol } }; - const attempt = (): boolean => { + const runAttempt = () => { if (cancelled) { return false; } - return scrollToHashTarget(hash); + return attempt(); }; const schedulePostLayoutRetry = () => { @@ -80,7 +82,7 @@ export const scheduleHashScroll = (hash = window.location.hash): CancelHashScrol timeoutId = window.setTimeout(() => { timeoutId = 0; - attempt(); + runAttempt(); }, POST_LAYOUT_MS); }; @@ -94,13 +96,20 @@ export const scheduleHashScroll = (hash = window.location.hash): CancelHashScrol return; } - if (attempt()) { + const succeeded = runAttempt(); + + if (postLayout === 'onAttemptSuccess' && succeeded) { schedulePostLayoutRetry(); return; } if (remaining > 0) { rafId = requestAnimationFrame(() => retryFrames(remaining - 1)); + return; + } + + if (postLayout === 'always') { + schedulePostLayoutRetry(); } }; @@ -109,3 +118,53 @@ export const scheduleHashScroll = (hash = window.location.hash): CancelHashScrol return cancel; }; + +/** + * Scrolls to the document top, retrying across frames and after sidenav layout animations. + * Use when the hash is cleared on the current page (e.g. switching hash-anchor menu sections). + */ +export const scheduleScrollToTop = (): CancelHashScroll => { + return scheduleScrollRetries(() => { + scrollToDocumentTop(); + return true; + }, 'always'); +}; + +/** + * Scrolls to the hash target, retrying across frames and after sidenav layout animations. + * Sidenav expand/collapse can reflow the page after the first scroll attempt. + * + * A post-layout retry runs only when a frame retry finds the target (to correct scroll + * position after the branch animation). If the target is never found, no delayed retry runs. + */ +export const scheduleHashScroll = (hash = window.location.hash): CancelHashScroll => { + if (!hash) { + return () => {}; + } + + return scheduleScrollRetries(() => scrollToHashTarget(hash), 'onAttemptSuccess'); +}; + +/** + * Defers scroll-to-top by one frame so a hash applied in the same navigation tick + * (e.g. `router.push('/page#section')`) is not overridden. + */ +export const scheduleDeferredScrollToTop = ( + hasWindowHash: () => boolean = () => Boolean(window.location.hash), +): CancelHashScroll => { + let cancelled = false; + let cancelScroll: CancelHashScroll | undefined; + const rafId = requestAnimationFrame(() => { + if (cancelled || hasWindowHash()) { + return; + } + + cancelScroll = scheduleScrollToTop(); + }); + + return () => { + cancelled = true; + cancelAnimationFrame(rafId); + cancelScroll?.(); + }; +}; diff --git a/src/components/QuickNav/resolveHashScrollEffect.test.ts b/src/components/QuickNav/resolveHashScrollEffect.test.ts new file mode 100644 index 00000000..d32c0317 --- /dev/null +++ b/src/components/QuickNav/resolveHashScrollEffect.test.ts @@ -0,0 +1,27 @@ +import { resolveHashScrollEffect } from './resolveHashScrollEffect'; + +describe('resolveHashScrollEffect', () => { + test('scrolls to hash when hash is present', () => { + expect( + resolveHashScrollEffect('/about-cloudsmith', '#developer-tools', '/other', ''), + ).toBe('hash'); + }); + + test('scrolls to top when hash is cleared on the same page', () => { + expect( + resolveHashScrollEffect('/about-cloudsmith', '', '/about-cloudsmith', '#developer-tools'), + ).toBe('top'); + }); + + test('does nothing when hash and pathname are unchanged', () => { + expect( + resolveHashScrollEffect('/about-cloudsmith', '', '/about-cloudsmith', ''), + ).toBe('none'); + }); + + test('defers top scroll when pathname changes without a hash', () => { + expect( + resolveHashScrollEffect('/about-cloudsmith', '', '/developer-tools/webhooks', ''), + ).toBe('deferred-top'); + }); +}); diff --git a/src/components/QuickNav/resolveHashScrollEffect.ts b/src/components/QuickNav/resolveHashScrollEffect.ts new file mode 100644 index 00000000..d081ef71 --- /dev/null +++ b/src/components/QuickNav/resolveHashScrollEffect.ts @@ -0,0 +1,25 @@ +export type HashScrollEffect = 'hash' | 'top' | 'deferred-top' | 'none'; + +/** + * Decides which scroll behavior `useHashScroll` should run for the current route/hash transition. + */ +export const resolveHashScrollEffect = ( + pathname: string, + hash: string, + previousPathname: string, + previousHash: string, +): HashScrollEffect => { + if (hash) { + return 'hash'; + } + + if (previousHash) { + return 'top'; + } + + if (previousPathname === pathname) { + return 'none'; + } + + return 'deferred-top'; +}; diff --git a/src/components/QuickNav/useHashScroll.ts b/src/components/QuickNav/useHashScroll.ts index 60eec06f..03466e77 100644 --- a/src/components/QuickNav/useHashScroll.ts +++ b/src/components/QuickNav/useHashScroll.ts @@ -1,22 +1,48 @@ 'use client'; -import { useLayoutEffect } from 'react'; +import { useLayoutEffect, useRef } from 'react'; import { usePathname } from 'next/navigation'; import { useWindowHash } from '@/lib/windowLocationHash'; -import { scheduleHashScroll } from './hashScroll'; +import { + scheduleDeferredScrollToTop, + scheduleHashScroll, + scheduleScrollToTop, +} from './hashScroll'; +import { resolveHashScrollEffect } from './resolveHashScrollEffect'; /** - * Scrolls to the in-page heading for the current URL hash. - * Runs after React commit (useLayoutEffect) and retries after sidenav layout animations. + * Scrolls to the in-page heading for the current URL hash, or to the document top when + * the hash is cleared. Runs after React commit and retries after sidenav layout animations. */ export const useHashScroll = () => { const pathname = usePathname(); const hash = useWindowHash(); + const previousHash = useRef(hash); + const previousPathname = useRef(pathname); useLayoutEffect(() => { - return scheduleHashScroll(hash); + const effect = resolveHashScrollEffect( + pathname, + hash, + previousPathname.current, + previousHash.current, + ); + + previousPathname.current = pathname; + previousHash.current = hash; + + switch (effect) { + case 'hash': + return scheduleHashScroll(hash); + case 'top': + return scheduleScrollToTop(); + case 'deferred-top': + return scheduleDeferredScrollToTop(); + default: + return () => {}; + } }, [pathname, hash]); }; diff --git a/src/components/Sidenav/Sidenav.tsx b/src/components/Sidenav/Sidenav.tsx index b551412c..5dffa68a 100644 --- a/src/components/Sidenav/Sidenav.tsx +++ b/src/components/Sidenav/Sidenav.tsx @@ -6,16 +6,15 @@ import { cx } from 'class-variance-authority'; import { Transition, Variants } from 'motion/react'; import * as motion from 'motion/react-client'; import Link from 'next/link'; -import { usePathname } from 'next/navigation'; import { useNavigation } from '@/app/navigation'; import { Icon } from '@/icons'; import { ChevronSmallIcon } from '@/icons/ChevronSmall'; import { MenuItem } from '@/lib/menu/types'; -import { getActiveAncestors } from '@/lib/menu/util'; import { last } from '@/lib/util'; import { Tag } from '../Tag'; +import { SidenavActiveMenuProvider, useSidenavActiveMenu } from './SidenavActiveMenuContext'; import { SidenavHashProvider } from './SidenavHashContext'; import { SidenavSiblingBranchProvider } from './SidenavSiblingBranches'; import { useSidenavItemActions } from './useSidenavItemActions'; @@ -34,11 +33,10 @@ const buildMenuItemKey = (parentKey: string, item: MenuItem, index: number): str return parentKey ? `${parentKey}/${segment}` : segment; }; -export const Sidenav = ({ items }: SidenavProps) => { +const SidenavContent = ({ items }: SidenavProps) => { const { navigationState, toggleNavigation } = useNavigation(); - const pathname = usePathname(); - const activeMenuItems = getActiveAncestors(pathname, items); - const activeLabel = last(activeMenuItems)?.title ?? 'Select page'; + const { activeAncestors } = useSidenavActiveMenu(); + const activeLabel = last(activeAncestors)?.title ?? 'Select page'; useEffect(() => { document.body.classList.add('has-sidenav-toggle'); @@ -73,9 +71,7 @@ export const Sidenav = ({ items }: SidenavProps) => { transition={togglerTransition}>
    - {items ? ( - - ) : null} + {items ? : null}
    @@ -83,6 +79,14 @@ export const Sidenav = ({ items }: SidenavProps) => { ); }; +export const Sidenav = ({ items }: SidenavProps) => { + return ( + + + + ); +}; + const MenuItemRows = ({ items, parentKey = '', listClassName }: MenuItemRowsProps) => { return ( @@ -141,7 +145,7 @@ const Item = ({ item, itemKey, siblingItems }: ItemProps) => {
    )} - + {item.title} diff --git a/src/components/Sidenav/SidenavActiveMenuContext.tsx b/src/components/Sidenav/SidenavActiveMenuContext.tsx new file mode 100644 index 00000000..fbb5881e --- /dev/null +++ b/src/components/Sidenav/SidenavActiveMenuContext.tsx @@ -0,0 +1,41 @@ +'use client'; + +import { createContext, type ReactNode, useContext, useMemo } from 'react'; + +import { usePathname } from 'next/navigation'; + +import type { MenuItem } from '@/lib/menu/types'; +import { getCanonicalActiveAncestors } from '@/lib/menu/util'; + +type SidenavActiveMenuContextValue = { + activeAncestors: MenuItem[]; +}; + +const SidenavActiveMenuContext = createContext(null); + +export const SidenavActiveMenuProvider = ({ + children, + items, +}: { + children: ReactNode; + items: MenuItem[]; +}) => { + const pathname = usePathname(); + const value = useMemo( + () => ({ activeAncestors: getCanonicalActiveAncestors(pathname, items) }), + [pathname, items], + ); + + return ( + {children} + ); +}; + +export const useSidenavActiveMenu = () => { + const context = useContext(SidenavActiveMenuContext); + if (!context) { + throw new Error('useSidenavActiveMenu must be used within SidenavActiveMenuProvider'); + } + + return context; +}; diff --git a/src/components/Sidenav/SidenavHashContext.tsx b/src/components/Sidenav/SidenavHashContext.tsx index 797dc070..036990ef 100644 --- a/src/components/Sidenav/SidenavHashContext.tsx +++ b/src/components/Sidenav/SidenavHashContext.tsx @@ -22,10 +22,11 @@ type SidenavHashContextValue = { const SidenavHashContext = createContext(null); /** - * Tracks the URL hash for sidenav expand/collapse logic. + * Tracks the URL hash for sidenav expand/collapse and click-resolution logic. * Context `hash` is the source of truth for interaction resolvers; it is seeded from * `window.location.hash` on route changes and updated imperatively when the user * opens or clears hash-anchor sections (see `useSidenavItemActions`). + * Page scroll position follows `window.location.hash` via `useHashScroll`, not this override. */ export const SidenavHashProvider = ({ children }: { children: ReactNode }) => { const pathname = usePathname(); diff --git a/src/components/Sidenav/sidenavNavigation.ts b/src/components/Sidenav/sidenavNavigation.ts index 0b141fab..7979ac02 100644 --- a/src/components/Sidenav/sidenavNavigation.ts +++ b/src/components/Sidenav/sidenavNavigation.ts @@ -1,30 +1,36 @@ import { normalizePath } from '@/util/url'; type SidenavRouter = { - push: (href: string) => void; + push: (href: string, options?: { scroll?: boolean }) => void; }; -const scrollToDocumentTop = () => { - window.requestAnimationFrame(() => { - const { documentElement } = document; - const previousScrollBehavior = documentElement.style.scrollBehavior; - documentElement.style.scrollBehavior = 'auto'; - - window.scrollTo(0, 0); - - documentElement.style.scrollBehavior = previousScrollBehavior; - }); +/** `history.replaceState` does not emit `hashchange`; subscribers need a synthetic event. */ +const replaceUrlClearingHash = (path: string) => { + window.history.replaceState(null, '', path); + window.dispatchEvent(new HashChangeEvent('hashchange')); }; -/** Navigates to a path and scrolls to top (e.g. when clearing a hash fragment). */ +/** + * Navigates to `path`, clearing any hash when already on that page. + * Scroll-to-top is handled by `useHashScroll` after the hash is removed from the URL. + */ export const navigateToSidenavPath = (router: SidenavRouter, path: string) => { - router.push(path); - scrollToDocumentTop(); + const normalizedPath = normalizePath(path); + + if (normalizePath(window.location.pathname) === normalizedPath) { + if (window.location.hash) { + replaceUrlClearingHash(normalizedPath); + } + + return; + } + + router.push(normalizedPath, { scroll: false }); }; /** Replaces the current URL with the base path when already on it, clearing any hash. */ export const clearUrlHashOnPage = (router: SidenavRouter, pathname: string, basePath: string) => { - if (normalizePath(pathname) === basePath) { + if (normalizePath(pathname) === normalizePath(basePath)) { navigateToSidenavPath(router, basePath); } }; diff --git a/src/components/Sidenav/useSidenavItemActions.ts b/src/components/Sidenav/useSidenavItemActions.ts index 686db57b..d3b2f1d6 100644 --- a/src/components/Sidenav/useSidenavItemActions.ts +++ b/src/components/Sidenav/useSidenavItemActions.ts @@ -16,11 +16,12 @@ import { getMenuItemBasePath, getMenuItemHash, isActiveHashAnchorSibling, - isActiveMenuLeaf, + isActiveMenuLeafForAncestors, shouldExpandMenuItem, } from '@/lib/menu/util'; import { clearUrlHashOnPage, navigateToSidenavPath } from './sidenavNavigation'; +import { useSidenavActiveMenu } from './SidenavActiveMenuContext'; import { useSidenavHash } from './SidenavHashContext'; import { useSiblingBranchContext } from './SidenavSiblingBranches'; @@ -38,10 +39,11 @@ export const useSidenavItemActions = ({ item, itemKey, siblingItems }: UseSidena const pathname = usePathname(); const router = useRouter(); const { hash, setHash } = useSidenavHash(); + const { activeAncestors } = useSidenavActiveMenu(); const siblingBranch = useSiblingBranchContext(); const { setNavigationState } = useNavigation(); - const isActiveLeaf = isActiveMenuLeaf(pathname, item); - const defaultExpanded = shouldExpandMenuItem(pathname, item, hash, siblingItems); + const isActiveLeaf = isActiveMenuLeafForAncestors(item, activeAncestors); + const defaultExpanded = shouldExpandMenuItem(pathname, item, hash, siblingItems, activeAncestors); const [isExpanded, setIsExpanded] = useState(defaultExpanded); useEffect(() => { @@ -59,12 +61,13 @@ export const useSidenavItemActions = ({ item, itemKey, siblingItems }: UseSidena const itemHash = getMenuItemHash(item.path); if (itemHash) { + // Optimistic hash for expand/collapse; scroll runs in `useHashScroll` from the URL after `router.push`. setHash(`#${itemHash}`); } siblingBranch.collapseSiblings(itemKey); setIsExpanded(true); - router.push(item.path); + router.push(item.path, { scroll: false }); }, [item.path, itemKey, router, setHash, siblingBranch]); const openBranchSection = useCallback( @@ -107,10 +110,11 @@ export const useSidenavItemActions = ({ item, itemKey, siblingItems }: UseSidena hash, item, siblingItems, + activeAncestors, isExpanded, isMobileViewport, }), - [hash, isExpanded, item, pathname, siblingItems], + [activeAncestors, hash, isExpanded, item, pathname, siblingItems], ); const applyAction = useCallback( diff --git a/src/lib/menu/sidenavInteractions.test.ts b/src/lib/menu/sidenavInteractions.test.ts index e6180413..45633a78 100644 --- a/src/lib/menu/sidenavInteractions.test.ts +++ b/src/lib/menu/sidenavInteractions.test.ts @@ -1,6 +1,15 @@ import type { MenuItem } from './types'; import type { SidenavItemInteractionContext, SidenavLinkClickResult } from './sidenavInteractions'; import { resolveSidenavChevronClick, resolveSidenavLinkClick } from './sidenavInteractions'; +import { getCanonicalActiveAncestors } from './util'; + +const withActiveAncestors = ( + context: Omit, + rootItems: MenuItem[], +): SidenavItemInteractionContext => ({ + ...context, + activeAncestors: getCanonicalActiveAncestors(context.pathname, rootItems), +}); const aboutCloudsmith: MenuItem = { title: 'About Cloudsmith', @@ -20,14 +29,17 @@ describe('resolveSidenavLinkClick', () => { test.each<[string, SidenavItemInteractionContext, SidenavLinkClickResult]>([ [ 'collapse active hash-anchor', - { - pathname: '/about-cloudsmith', - hash: '#developer-tools', - item: developerTools, - siblingItems: hashAnchorSiblings, - isExpanded: true, - isMobileViewport: false, - }, + withActiveAncestors( + { + pathname: '/about-cloudsmith', + hash: '#developer-tools', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: true, + isMobileViewport: false, + }, + hashAnchorSiblings, + ), { action: { kind: 'collapse-clear-hash', collapseSiblings: false }, preventDefault: true, @@ -37,14 +49,17 @@ describe('resolveSidenavLinkClick', () => { ], [ 'open hash-anchor', - { - pathname: '/about-cloudsmith', - hash: '', - item: developerTools, - siblingItems: hashAnchorSiblings, - isExpanded: false, - isMobileViewport: false, - }, + withActiveAncestors( + { + pathname: '/about-cloudsmith', + hash: '', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + hashAnchorSiblings, + ), { action: { kind: 'open-hash-anchor' }, preventDefault: true, @@ -54,14 +69,17 @@ describe('resolveSidenavLinkClick', () => { ], [ 'open hash-anchor from child route', - { - pathname: '/developer-tools/terraform-provider', - hash: '', - item: developerTools, - siblingItems: hashAnchorSiblings, - isExpanded: true, - isMobileViewport: false, - }, + withActiveAncestors( + { + pathname: '/developer-tools/terraform-provider', + hash: '', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: true, + isMobileViewport: false, + }, + hashAnchorSiblings, + ), { action: { kind: 'open-hash-anchor' }, preventDefault: true, @@ -71,14 +89,17 @@ describe('resolveSidenavLinkClick', () => { ], [ 'collapse non-hash sibling', - { - pathname: '/about-cloudsmith', - hash: '', - item: aboutCloudsmith, - siblingItems: hashAnchorSiblings, - isExpanded: true, - isMobileViewport: false, - }, + withActiveAncestors( + { + pathname: '/about-cloudsmith', + hash: '', + item: aboutCloudsmith, + siblingItems: hashAnchorSiblings, + isExpanded: true, + isMobileViewport: false, + }, + hashAnchorSiblings, + ), { action: { kind: 'collapse' }, preventDefault: true, @@ -88,14 +109,17 @@ describe('resolveSidenavLinkClick', () => { ], [ 'open sibling and clear hash', - { - pathname: '/about-cloudsmith', - hash: '#developer-tools', - item: aboutCloudsmith, - siblingItems: hashAnchorSiblings, - isExpanded: false, - isMobileViewport: false, - }, + withActiveAncestors( + { + pathname: '/about-cloudsmith', + hash: '#developer-tools', + item: aboutCloudsmith, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + hashAnchorSiblings, + ), { action: { kind: 'open-sibling-branch', clearHash: true }, preventDefault: true, @@ -105,14 +129,17 @@ describe('resolveSidenavLinkClick', () => { ], [ 'open sibling without clearing hash', - { - pathname: '/about-cloudsmith', - hash: '', - item: aboutCloudsmith, - siblingItems: hashAnchorSiblings, - isExpanded: false, - isMobileViewport: false, - }, + withActiveAncestors( + { + pathname: '/about-cloudsmith', + hash: '', + item: aboutCloudsmith, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + hashAnchorSiblings, + ), { action: { kind: 'open-sibling-branch', clearHash: false }, preventDefault: true, @@ -129,38 +156,47 @@ describe('resolveSidenavChevronClick', () => { test.each([ [ 'open hash-anchor', - { - pathname: '/about-cloudsmith', - hash: '', - item: developerTools, - siblingItems: hashAnchorSiblings, - isExpanded: false, - isMobileViewport: false, - }, + withActiveAncestors( + { + pathname: '/about-cloudsmith', + hash: '', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + hashAnchorSiblings, + ), { kind: 'open-hash-anchor' }, ], [ 'collapse hash-anchor and siblings', - { - pathname: '/developer-tools/cli', - hash: '', - item: developerTools, - siblingItems: hashAnchorSiblings, - isExpanded: true, - isMobileViewport: false, - }, + withActiveAncestors( + { + pathname: '/developer-tools/cli', + hash: '', + item: developerTools, + siblingItems: hashAnchorSiblings, + isExpanded: true, + isMobileViewport: false, + }, + hashAnchorSiblings, + ), { kind: 'collapse-clear-hash', collapseSiblings: true }, ], [ 'open sibling and clear hash', - { - pathname: '/about-cloudsmith', - hash: '#developer-tools', - item: aboutCloudsmith, - siblingItems: hashAnchorSiblings, - isExpanded: false, - isMobileViewport: false, - }, + withActiveAncestors( + { + pathname: '/about-cloudsmith', + hash: '#developer-tools', + item: aboutCloudsmith, + siblingItems: hashAnchorSiblings, + isExpanded: false, + isMobileViewport: false, + }, + hashAnchorSiblings, + ), { kind: 'open-sibling-branch', clearHash: true }, ], ] as const)('%s', (_name, context, expected) => { diff --git a/src/lib/menu/sidenavInteractions.ts b/src/lib/menu/sidenavInteractions.ts index 5634fdae..5dd0715c 100644 --- a/src/lib/menu/sidenavInteractions.ts +++ b/src/lib/menu/sidenavInteractions.ts @@ -2,11 +2,11 @@ import type { MenuItem } from './types'; import { hasHashAnchorSiblingOnSameBase, isActiveHashAnchorSibling, - isActiveMenuLeaf, + isActiveMenuLeafForAncestors, isAnyHashAnchorSiblingActive, isExactMenuPath, isHashAnchorParent, - isMenuItemOnActiveBranch, + isItemInActiveAncestors, } from './util'; export type SidenavItemInteractionContext = { @@ -14,6 +14,7 @@ export type SidenavItemInteractionContext = { hash: string; item: MenuItem; siblingItems: MenuItem[]; + activeAncestors: MenuItem[]; isExpanded: boolean; isMobileViewport: boolean; }; @@ -45,11 +46,11 @@ export type SidenavChevronClickAction = export const resolveSidenavLinkClick = ( ctx: SidenavItemInteractionContext, ): SidenavLinkClickResult => { - const { pathname, hash, item, siblingItems, isExpanded, isMobileViewport } = ctx; - const isActiveLeaf = isActiveMenuLeaf(pathname, item); + const { pathname, hash, item, siblingItems, activeAncestors, isExpanded, isMobileViewport } = ctx; + const isActiveLeafMatch = isActiveMenuLeafForAncestors(item, activeAncestors); const isExactPathMatch = isExactMenuPath(pathname, item); const isHashAnchor = isHashAnchorParent(item); - const isOnActiveBranch = isMenuItemOnActiveBranch(pathname, item); + const isOnActiveBranchMatch = isItemInActiveAncestors(item, activeAncestors); const sharesBaseWithHashAnchorSibling = hasHashAnchorSiblingOnSameBase(item, siblingItems); const hashSiblingIsActive = isAnyHashAnchorSiblingActive(pathname, hash, siblingItems); @@ -71,8 +72,8 @@ export const resolveSidenavLinkClick = ( return { action: { kind: 'open-hash-anchor' }, preventDefault: true, - closeMobileNav: isMobileViewport && !isOnActiveBranch, - blurLink: isMobileViewport && !isOnActiveBranch, + closeMobileNav: isMobileViewport && !isOnActiveBranchMatch, + blurLink: isMobileViewport && !isOnActiveBranchMatch, }; } @@ -85,8 +86,8 @@ export const resolveSidenavLinkClick = ( return { action: { kind: 'open-sibling-branch', clearHash: hashSiblingIsActive }, preventDefault: hashSiblingIsActive || isExactPathMatch, - closeMobileNav: isMobileViewport && !isActiveLeaf, - blurLink: isMobileViewport && !isActiveLeaf, + closeMobileNav: isMobileViewport && !isActiveLeafMatch, + blurLink: isMobileViewport && !isActiveLeafMatch, }; } } @@ -95,15 +96,15 @@ export const resolveSidenavLinkClick = ( return { ...idle({ kind: 'collapse' }), preventDefault: true }; } - if (isActiveLeaf && isMobileViewport) { + if (isActiveLeafMatch && isMobileViewport) { return { ...idle({ kind: 'navigate' }), preventDefault: true }; } return { action: item.children && !isExpanded ? { kind: 'expand-branch' } : { kind: 'navigate' }, preventDefault: false, - closeMobileNav: isMobileViewport && !isActiveLeaf, - blurLink: isMobileViewport && !isActiveLeaf, + closeMobileNav: isMobileViewport && !isActiveLeafMatch, + blurLink: isMobileViewport && !isActiveLeafMatch, }; }; @@ -114,14 +115,14 @@ export const resolveSidenavLinkClick = ( export const resolveSidenavChevronClick = ( ctx: SidenavItemInteractionContext, ): SidenavChevronClickAction => { - const { pathname, hash, item, siblingItems, isExpanded } = ctx; - const isOnActiveBranch = isMenuItemOnActiveBranch(pathname, item); + const { pathname, hash, item, siblingItems, activeAncestors, isExpanded } = ctx; + const isOnActiveBranchMatch = isItemInActiveAncestors(item, activeAncestors); const isHashAnchor = isHashAnchorParent(item); const sharesBaseWithHashAnchorSibling = hasHashAnchorSiblingOnSameBase(item, siblingItems); const hashSiblingIsActive = isAnyHashAnchorSiblingActive(pathname, hash, siblingItems); if (isHashAnchor && item.path) { - if (!isOnActiveBranch || !isExpanded) { + if (!isOnActiveBranchMatch || !isExpanded) { return { kind: 'open-hash-anchor' }; } diff --git a/src/lib/menu/util.test.ts b/src/lib/menu/util.test.ts index b4c6f572..24a76ce7 100644 --- a/src/lib/menu/util.test.ts +++ b/src/lib/menu/util.test.ts @@ -2,10 +2,12 @@ import type { MenuItem } from './types'; import { getActiveAncestors, getActiveMenuItem, + getCanonicalActiveAncestors, getMenuItem, isActiveMenuLeaf, + isActiveMenuLeafForAncestors, isExactMenuPath, - isMenuItemOnActiveBranch, + isItemInActiveAncestors, shouldExpandMenuItem, } from './util'; @@ -65,7 +67,7 @@ describe('lib', () => { const ancestors = getActiveAncestors('/authentication', [workspacesMenu]); expect(ancestors.map((item) => item.title)).toEqual(['Workspaces', 'Authentication']); - expect(isMenuItemOnActiveBranch('/authentication', workspacesMenu)).toBe(true); + expect(isItemInActiveAncestors(workspacesMenu, ancestors)).toBe(true); }); }); @@ -99,6 +101,75 @@ describe('lib', () => { }); }); + describe('getCanonicalActiveAncestors()', () => { + test('picks one branch when multiple menu items share the same path', () => { + const cursorIde: MenuItem = { title: 'Cursor IDE', path: '/developer-tools/vscode' }; + const theiaIde: MenuItem = { title: 'Theia IDE', path: '/developer-tools/vscode' }; + const vscodeInIntegrations: MenuItem = { + title: 'VS Code Extension', + path: '/developer-tools/vscode', + }; + const vscodeInDeveloperTools: MenuItem = { + title: 'VS Code Extension', + path: '/developer-tools/vscode', + }; + + const integrations: MenuItem = { + title: 'Integrations', + path: '/integrations', + children: [ + { title: 'Ansible', path: '/integrations/integrating-with-ansible' }, + cursorIde, + theiaIde, + vscodeInIntegrations, + ], + }; + + const developerToolsSection: MenuItem = { + title: 'Developer Tools', + path: '/about-cloudsmith#developer-tools', + children: [ + { title: 'Cloudsmith CLI', path: '/developer-tools/cli' }, + vscodeInDeveloperTools, + { title: 'Webhooks', path: '/developer-tools/webhooks' }, + ], + }; + + const menuItems = [integrations, developerToolsSection]; + const path = '/developer-tools/vscode'; + const ancestors = getCanonicalActiveAncestors(path, menuItems); + + expect(ancestors).toEqual([developerToolsSection, vscodeInDeveloperTools]); + expect(isActiveMenuLeafForAncestors(vscodeInDeveloperTools, ancestors)).toBe(true); + expect(isActiveMenuLeafForAncestors(cursorIde, ancestors)).toBe(false); + expect(isItemInActiveAncestors(integrations, ancestors)).toBe(false); + expect(isItemInActiveAncestors(developerToolsSection, ancestors)).toBe(true); + expect(shouldExpandMenuItem(path, integrations, '', [], ancestors)).toBe(false); + expect(shouldExpandMenuItem(path, developerToolsSection, '', [], ancestors)).toBe(true); + }); + + test('breaks affinity ties by keeping the earlier menu match', () => { + const firstMatch: MenuItem = { title: 'First', path: '/docs/page' }; + const secondMatch: MenuItem = { title: 'Second', path: '/docs/page' }; + + const sectionA: MenuItem = { + title: 'Section A', + path: '/docs', + children: [firstMatch, { title: 'Other A', path: '/docs/other-a' }], + }; + + const sectionB: MenuItem = { + title: 'Section B', + path: '/docs', + children: [secondMatch, { title: 'Other B', path: '/docs/other-b' }], + }; + + const ancestors = getCanonicalActiveAncestors('/docs/page', [sectionA, sectionB]); + + expect(ancestors).toEqual([sectionA, firstMatch]); + }); + }); + describe('shouldExpandMenuItem()', () => { test('expands section headers without a path', () => { const section: MenuItem = { title: 'Overview' }; @@ -111,20 +182,35 @@ describe('lib', () => { path: '/workspaces', children: [{ title: 'Authentication', path: '/authentication' }], }; + const ancestors = getActiveAncestors('/authentication', [workspacesMenu]); - expect(shouldExpandMenuItem('/authentication', workspacesMenu)).toBe(true); + expect(shouldExpandMenuItem('/authentication', workspacesMenu, '', [], ancestors)).toBe(true); }); test('expands hash-anchor and sibling branches for the active hash', () => { + const ancestors = getCanonicalActiveAncestors('/about-cloudsmith', hashAnchorSiblings); + expect( - shouldExpandMenuItem('/about-cloudsmith', developerTools, '#developer-tools', hashAnchorSiblings), + shouldExpandMenuItem( + '/about-cloudsmith', + developerTools, + '#developer-tools', + hashAnchorSiblings, + ancestors, + ), ).toBe(true); expect( - shouldExpandMenuItem('/about-cloudsmith', aboutCloudsmith, '#developer-tools', hashAnchorSiblings), + shouldExpandMenuItem( + '/about-cloudsmith', + aboutCloudsmith, + '#developer-tools', + hashAnchorSiblings, + ancestors, + ), ).toBe(false); - expect(shouldExpandMenuItem('/about-cloudsmith', aboutCloudsmith, '', hashAnchorSiblings)).toBe( - true, - ); + expect( + shouldExpandMenuItem('/about-cloudsmith', aboutCloudsmith, '', hashAnchorSiblings, ancestors), + ).toBe(true); }); }); diff --git a/src/lib/menu/util.ts b/src/lib/menu/util.ts index c1dac2b7..8edfcc2e 100644 --- a/src/lib/menu/util.ts +++ b/src/lib/menu/util.ts @@ -73,6 +73,147 @@ export const getActiveAncestors = ( return []; }; +/** Collects every menu item whose path exactly matches the pathname. */ +const findAllExactPathMatches = (pathname: string, items: MenuItem[]): MenuItem[] => { + const normalizedPathname = normalizePath(pathname); + const matches: MenuItem[] = []; + + const visit = (menuItems: MenuItem[]) => { + for (const item of menuItems) { + if (item.path && normalizePath(item.path) === normalizedPathname) { + matches.push(item); + } + + if (item.children) { + visit(item.children); + } + } + }; + + visit(items); + return matches; +}; + +/** Returns the ancestor chain from `items` down to `target`, or null when not found. */ +const findMenuItemAncestors = ( + items: MenuItem[], + target: MenuItem, + ancestors: MenuItem[] = [], +): MenuItem[] | null => { + for (const item of items) { + const nextAncestors = ancestors.concat(item); + + if (item === target) { + return nextAncestors; + } + + if (item.children) { + const found = findMenuItemAncestors(item.children, target, nextAncestors); + if (found) { + return found; + } + } + } + + return null; +}; + +/** + * Scores how well a menu item's siblings align with the current pathname. + * Used to pick one entry when multiple menu items share the same path. + */ +const scoreMenuSiblingAffinity = ( + pathname: string, + item: MenuItem, + rootItems: MenuItem[], +): { ratioScore: number; matchCount: number } => { + const ancestors = findMenuItemAncestors(rootItems, item); + if (!ancestors || ancestors.length < 2) { + return { ratioScore: 0, matchCount: 0 }; + } + + const parent = ancestors[ancestors.length - 2]; + const siblings = parent.children ?? []; + const pathSegments = normalizePath(pathname).split('/').filter(Boolean); + const primarySegment = pathSegments[0]; + + if (!primarySegment) { + return { ratioScore: 0, matchCount: 0 }; + } + + const segmentPrefix = `/${primarySegment}`; + const siblingsWithPath = siblings.filter((sibling) => sibling.path); + + if (siblingsWithPath.length === 0) { + return { ratioScore: 0, matchCount: 0 }; + } + + const matchingSiblings = siblingsWithPath.filter((sibling) => { + const siblingPath = getMenuItemBasePath(sibling.path!); + + return siblingPath === segmentPrefix || siblingPath.startsWith(`${segmentPrefix}/`); + }); + + const matchCount = matchingSiblings.length; + const ratioScore = Math.round((matchCount / siblingsWithPath.length) * 10000); + + return { ratioScore, matchCount }; +}; + +/** + * Picks the best leaf when several share a pathname. + * Tie-break order: sibling-path ratio, then absolute match count, then stable DFS order. + */ +const isSiblingAffinityBetter = ( + candidate: { ratioScore: number; matchCount: number }, + best: { ratioScore: number; matchCount: number }, +): boolean => { + if (candidate.ratioScore !== best.ratioScore) { + return candidate.ratioScore > best.ratioScore; + } + + if (candidate.matchCount !== best.matchCount) { + return candidate.matchCount > best.matchCount; + } + + return false; +}; + +const pickCanonicalLeaf = (pathname: string, leafMatches: MenuItem[], rootItems: MenuItem[]): MenuItem => { + return leafMatches.reduce((best, candidate) => { + const candidateMetrics = scoreMenuSiblingAffinity(pathname, candidate, rootItems); + const bestMetrics = scoreMenuSiblingAffinity(pathname, best, rootItems); + + return isSiblingAffinityBetter(candidateMetrics, bestMetrics) ? candidate : best; + }); +}; + +/** + * Resolves the single active menu branch for a pathname. + * When multiple menu entries intentionally share one path (e.g. Integrations aliases and + * Developer Tools both linking to `/developer-tools/vscode`), prefers the branch whose + * siblings best match the pathname prefix. + */ +export const getCanonicalActiveAncestors = ( + pathname: string, + items: MenuItem[], +): MenuItem[] => { + const exactMatches = findAllExactPathMatches(pathname, items); + const leafMatches = exactMatches.filter((item) => !item.children); + + if (leafMatches.length > 1) { + const canonicalLeaf = pickCanonicalLeaf(pathname, leafMatches, items); + + return findMenuItemAncestors(items, canonicalLeaf) ?? []; + } + + if (leafMatches.length === 1) { + return findMenuItemAncestors(items, leafMatches[0]) ?? []; + } + + return getActiveAncestors(pathname, items); +}; + /** * Whether the pathname exactly matches this menu item's path. * Compares full paths as stored in the menu; hash fragments in `item.path` are not @@ -87,18 +228,22 @@ export const isExactMenuPath = (pathname: string, item: MenuItem): boolean => { return normalizePath(item.path) === normalizePath(pathname); }; -/** - * Whether this leaf menu item is the current page. - */ -export const isActiveMenuLeaf = (pathname: string, item: MenuItem): boolean => { - return isExactMenuPath(pathname, item) && !item.children; +/** Whether `item` appears in the resolved active ancestor chain. */ +export const isItemInActiveAncestors = (item: MenuItem, activeAncestors: MenuItem[]): boolean => { + return activeAncestors.includes(item); +}; + +/** Whether `item` is the active leaf for the resolved ancestor chain. */ +export const isActiveMenuLeafForAncestors = (item: MenuItem, activeAncestors: MenuItem[]): boolean => { + return !item.children && activeAncestors[activeAncestors.length - 1] === item; }; /** - * Whether the current page is this item or one of its descendants. + * Whether this leaf menu item is the current page. + * Only valid when the menu tree has no duplicate paths for `pathname`. */ -export const isMenuItemOnActiveBranch = (pathname: string, item: MenuItem): boolean => { - return getActiveAncestors(pathname, [item]).length > 0; +export const isActiveMenuLeaf = (pathname: string, item: MenuItem): boolean => { + return !item.children && isExactMenuPath(pathname, item); }; /** @@ -183,6 +328,7 @@ export const shouldExpandMenuItem = ( item: MenuItem, hash = '', siblingItems: MenuItem[] = [], + activeAncestors?: MenuItem[], ): boolean => { if (!item.path) { return true; @@ -192,9 +338,13 @@ export const shouldExpandMenuItem = ( const activeHashSibling = siblingItems.find((sibling) => isActiveHashAnchorSibling(pathname, sibling, normalizedHash), ); + // Sidenav always passes `activeAncestors` (required when paths repeat). Subtree fallback is for unit tests. + const onActiveBranch = activeAncestors + ? isItemInActiveAncestors(item, activeAncestors) + : getActiveAncestors(pathname, [item]).length > 0; if (isHashAnchorParent(item)) { - if (isMenuItemOnActiveBranch(pathname, item)) { + if (onActiveBranch) { return true; } @@ -210,5 +360,5 @@ export const shouldExpandMenuItem = ( return false; } - return isMenuItemOnActiveBranch(pathname, item); + return onActiveBranch; }; From 39f85e0e84f5b50800f57689b6da22aa14b3f0b3 Mon Sep 17 00:00:00 2001 From: Niall O'Brien Date: Fri, 29 May 2026 15:13:41 +0100 Subject: [PATCH 4/6] chore: add copyright headers and clean up imports - Add Cloudsmith copyright headers to all relevant files - Simplify and reorder import statements for clarity - Use consistent import style for type-only imports - No functional changes to logic or behavior --- src/components/QuickNav/hashScroll.ts | 7 +++---- .../QuickNav/resolveHashScrollEffect.ts | 2 ++ src/components/QuickNav/useHashScroll.ts | 15 ++++---------- src/components/Sidenav/Sidenav.tsx | 2 +- .../Sidenav/SidenavActiveMenuContext.tsx | 12 ++++++----- src/components/Sidenav/SidenavHashContext.tsx | 14 +++++-------- .../Sidenav/SidenavSiblingBranches.tsx | 18 ++++++----------- src/components/Sidenav/sidenavNavigation.ts | 2 ++ .../Sidenav/useSidenavItemActions.ts | 20 +++++++++---------- src/lib/menu/sidenavInteractions.ts | 11 +++++----- src/lib/windowLocationHash.ts | 5 +++-- 11 files changed, 47 insertions(+), 61 deletions(-) diff --git a/src/components/QuickNav/hashScroll.ts b/src/components/QuickNav/hashScroll.ts index 9f427a24..fa6cde0d 100644 --- a/src/components/QuickNav/hashScroll.ts +++ b/src/components/QuickNav/hashScroll.ts @@ -1,3 +1,5 @@ +// Copyright 2026 Cloudsmith Ltd + const FRAME_RETRIES = 5; /** Match Sidenav `openCloseTransition.duration` (0.35s). */ const POST_LAYOUT_MS = 350; @@ -47,10 +49,7 @@ type PostLayoutRetry = 'always' | 'onAttemptSuccess'; * Runs `attempt` across animation frames and optionally after sidenav layout animations. * Returns a cancel function; call it when the hash or route changes before pending work runs. */ -const scheduleScrollRetries = ( - attempt: () => boolean, - postLayout: PostLayoutRetry, -): CancelHashScroll => { +const scheduleScrollRetries = (attempt: () => boolean, postLayout: PostLayoutRetry): CancelHashScroll => { let cancelled = false; let rafId = 0; let timeoutId = 0; diff --git a/src/components/QuickNav/resolveHashScrollEffect.ts b/src/components/QuickNav/resolveHashScrollEffect.ts index d081ef71..01951e1c 100644 --- a/src/components/QuickNav/resolveHashScrollEffect.ts +++ b/src/components/QuickNav/resolveHashScrollEffect.ts @@ -1,3 +1,5 @@ +// Copyright 2026 Cloudsmith Ltd + export type HashScrollEffect = 'hash' | 'top' | 'deferred-top' | 'none'; /** diff --git a/src/components/QuickNav/useHashScroll.ts b/src/components/QuickNav/useHashScroll.ts index 03466e77..f7c40a0d 100644 --- a/src/components/QuickNav/useHashScroll.ts +++ b/src/components/QuickNav/useHashScroll.ts @@ -1,3 +1,5 @@ +// Copyright 2026 Cloudsmith Ltd + 'use client'; import { useLayoutEffect, useRef } from 'react'; @@ -6,11 +8,7 @@ import { usePathname } from 'next/navigation'; import { useWindowHash } from '@/lib/windowLocationHash'; -import { - scheduleDeferredScrollToTop, - scheduleHashScroll, - scheduleScrollToTop, -} from './hashScroll'; +import { scheduleDeferredScrollToTop, scheduleHashScroll, scheduleScrollToTop } from './hashScroll'; import { resolveHashScrollEffect } from './resolveHashScrollEffect'; /** @@ -24,12 +22,7 @@ export const useHashScroll = () => { const previousPathname = useRef(pathname); useLayoutEffect(() => { - const effect = resolveHashScrollEffect( - pathname, - hash, - previousPathname.current, - previousHash.current, - ); + const effect = resolveHashScrollEffect(pathname, hash, previousPathname.current, previousHash.current); previousPathname.current = pathname; previousHash.current = hash; diff --git a/src/components/Sidenav/Sidenav.tsx b/src/components/Sidenav/Sidenav.tsx index 5dffa68a..f0dd76dc 100644 --- a/src/components/Sidenav/Sidenav.tsx +++ b/src/components/Sidenav/Sidenav.tsx @@ -131,7 +131,7 @@ const Item = ({ item, itemKey, siblingItems }: ItemProps) => { className={cx(styles.linkButton, styles.linkIcon)} onClick={handleChevronClick} aria-expanded={isExpanded} - aria-label={isExpanded ? 'Collapse section' : 'Expand section'}> + aria-label={isExpanded ? `Collapse ${item.title}` : `Expand ${item.title}`}> {children} - ); + return {children}; }; export const useSidenavActiveMenu = () => { diff --git a/src/components/Sidenav/SidenavHashContext.tsx b/src/components/Sidenav/SidenavHashContext.tsx index 036990ef..43b206f8 100644 --- a/src/components/Sidenav/SidenavHashContext.tsx +++ b/src/components/Sidenav/SidenavHashContext.tsx @@ -1,14 +1,10 @@ +// Copyright 2026 Cloudsmith Ltd + 'use client'; -import { - createContext, - type ReactNode, - useCallback, - useContext, - useEffect, - useMemo, - useState, -} from 'react'; +import { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react'; + +import type { ReactNode } from 'react'; import { usePathname } from 'next/navigation'; diff --git a/src/components/Sidenav/SidenavSiblingBranches.tsx b/src/components/Sidenav/SidenavSiblingBranches.tsx index 0f7df7a6..438dbfbf 100644 --- a/src/components/Sidenav/SidenavSiblingBranches.tsx +++ b/src/components/Sidenav/SidenavSiblingBranches.tsx @@ -1,13 +1,10 @@ +// Copyright 2026 Cloudsmith Ltd + 'use client'; -import { - createContext, - type ReactNode, - useCallback, - useContext, - useMemo, - useRef, -} from 'react'; +import { createContext, useCallback, useContext, useMemo, useRef } from 'react'; + +import type { ReactNode } from 'react'; type SiblingBranchContextValue = { collapseSiblings: (exceptKey: string) => void; @@ -49,10 +46,7 @@ export const SidenavSiblingBranchProvider = ({ children }: SidenavSiblingBranchP }); }, []); - const value = useMemo( - () => ({ collapseSiblings, registerBranch }), - [collapseSiblings, registerBranch], - ); + const value = useMemo(() => ({ collapseSiblings, registerBranch }), [collapseSiblings, registerBranch]); return {children}; }; diff --git a/src/components/Sidenav/sidenavNavigation.ts b/src/components/Sidenav/sidenavNavigation.ts index 7979ac02..0ad249eb 100644 --- a/src/components/Sidenav/sidenavNavigation.ts +++ b/src/components/Sidenav/sidenavNavigation.ts @@ -1,3 +1,5 @@ +// Copyright 2026 Cloudsmith Ltd + import { normalizePath } from '@/util/url'; type SidenavRouter = { diff --git a/src/components/Sidenav/useSidenavItemActions.ts b/src/components/Sidenav/useSidenavItemActions.ts index d3b2f1d6..06b0b75d 100644 --- a/src/components/Sidenav/useSidenavItemActions.ts +++ b/src/components/Sidenav/useSidenavItemActions.ts @@ -1,17 +1,17 @@ +// Copyright 2026 Cloudsmith Ltd + 'use client'; -import { type MouseEvent, useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; + +import type { SidenavChevronClickAction, SidenavLinkClickAction } from '@/lib/menu/sidenavInteractions'; +import type { MouseEvent } from 'react'; import { usePathname, useRouter } from 'next/navigation'; import { useNavigation } from '@/app/navigation'; +import { resolveSidenavChevronClick, resolveSidenavLinkClick } from '@/lib/menu/sidenavInteractions'; import { MenuItem } from '@/lib/menu/types'; -import { - resolveSidenavChevronClick, - resolveSidenavLinkClick, - type SidenavChevronClickAction, - type SidenavLinkClickAction, -} from '@/lib/menu/sidenavInteractions'; import { getMenuItemBasePath, getMenuItemHash, @@ -20,9 +20,9 @@ import { shouldExpandMenuItem, } from '@/lib/menu/util'; -import { clearUrlHashOnPage, navigateToSidenavPath } from './sidenavNavigation'; import { useSidenavActiveMenu } from './SidenavActiveMenuContext'; import { useSidenavHash } from './SidenavHashContext'; +import { clearUrlHashOnPage, navigateToSidenavPath } from './sidenavNavigation'; import { useSiblingBranchContext } from './SidenavSiblingBranches'; const MOBILE_MEDIA_QUERY = '(max-width: 767px)'; @@ -85,9 +85,7 @@ export const useSidenavItemActions = ({ item, itemKey, siblingItems }: UseSidena const collapseClearHash = useCallback( (collapseSiblingsOnClose: boolean) => { - const shouldClearUrlHash = Boolean( - item.path && isActiveHashAnchorSibling(pathname, item, hash), - ); + const shouldClearUrlHash = Boolean(item.path && isActiveHashAnchorSibling(pathname, item, hash)); setHash(''); diff --git a/src/lib/menu/sidenavInteractions.ts b/src/lib/menu/sidenavInteractions.ts index 5dd0715c..b35d5738 100644 --- a/src/lib/menu/sidenavInteractions.ts +++ b/src/lib/menu/sidenavInteractions.ts @@ -1,4 +1,7 @@ +// Copyright 2026 Cloudsmith Ltd + import type { MenuItem } from './types'; + import { hasHashAnchorSiblingOnSameBase, isActiveHashAnchorSibling, @@ -43,9 +46,7 @@ export type SidenavChevronClickAction = /** * Resolves how a sidenav item link click should behave. */ -export const resolveSidenavLinkClick = ( - ctx: SidenavItemInteractionContext, -): SidenavLinkClickResult => { +export const resolveSidenavLinkClick = (ctx: SidenavItemInteractionContext): SidenavLinkClickResult => { const { pathname, hash, item, siblingItems, activeAncestors, isExpanded, isMobileViewport } = ctx; const isActiveLeafMatch = isActiveMenuLeafForAncestors(item, activeAncestors); const isExactPathMatch = isExactMenuPath(pathname, item); @@ -112,9 +113,7 @@ export const resolveSidenavLinkClick = ( * Resolves how a sidenav item chevron click should behave. * Chevron collapse-clear-hash collapses same-level siblings; link clicks do not. */ -export const resolveSidenavChevronClick = ( - ctx: SidenavItemInteractionContext, -): SidenavChevronClickAction => { +export const resolveSidenavChevronClick = (ctx: SidenavItemInteractionContext): SidenavChevronClickAction => { const { pathname, hash, item, siblingItems, activeAncestors, isExpanded } = ctx; const isOnActiveBranchMatch = isItemInActiveAncestors(item, activeAncestors); const isHashAnchor = isHashAnchorParent(item); diff --git a/src/lib/windowLocationHash.ts b/src/lib/windowLocationHash.ts index 9a9e03ef..4f862c6f 100644 --- a/src/lib/windowLocationHash.ts +++ b/src/lib/windowLocationHash.ts @@ -1,3 +1,5 @@ +// Copyright 2026 Cloudsmith Ltd + 'use client'; import { useSyncExternalStore } from 'react'; @@ -16,5 +18,4 @@ export const subscribeToWindowHash = (onStoreChange: () => void) => { export const getWindowHash = () => window.location.hash; /** Subscribes to `window.location.hash` (SSR snapshot is empty). */ -export const useWindowHash = () => - useSyncExternalStore(subscribeToWindowHash, getWindowHash, () => ''); +export const useWindowHash = () => useSyncExternalStore(subscribeToWindowHash, getWindowHash, () => ''); From 73d51ffd1d3afd5f52ff180e6eb2f0147e251733 Mon Sep 17 00:00:00 2001 From: Niall O'Brien Date: Fri, 29 May 2026 15:27:44 +0100 Subject: [PATCH 5/6] fix: correct import order and adjust JSX formatting in Sidenav component --- src/components/Sidenav/Sidenav.tsx | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/components/Sidenav/Sidenav.tsx b/src/components/Sidenav/Sidenav.tsx index f0dd76dc..a0b9fc4e 100644 --- a/src/components/Sidenav/Sidenav.tsx +++ b/src/components/Sidenav/Sidenav.tsx @@ -14,11 +14,11 @@ import { MenuItem } from '@/lib/menu/types'; import { last } from '@/lib/util'; import { Tag } from '../Tag'; +import styles from './Sidenav.module.css'; import { SidenavActiveMenuProvider, useSidenavActiveMenu } from './SidenavActiveMenuContext'; import { SidenavHashProvider } from './SidenavHashContext'; import { SidenavSiblingBranchProvider } from './SidenavSiblingBranches'; import { useSidenavItemActions } from './useSidenavItemActions'; -import styles from './Sidenav.module.css'; const togglerTransition: Transition = { duration: 0.2, ease: 'easeInOut' }; const openCloseTransition: Transition = { duration: 0.35, ease: [0.55, 0, 0, 1] }; @@ -147,17 +147,17 @@ const Item = ({ item, itemKey, siblingItems }: ItemProps) => { {item.title} - - {item.method && ( - - )} + {item.method && ( + + )} +
    ) : ( {item.title} From 6397b2d6708033be83478a427981b7f9bd3cd6ad Mon Sep 17 00:00:00 2001 From: Niall O'Brien Date: Fri, 29 May 2026 15:29:34 +0100 Subject: [PATCH 6/6] fix: specify window object for animation frame and timeout functions in hashScroll --- src/components/QuickNav/hashScroll.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/QuickNav/hashScroll.ts b/src/components/QuickNav/hashScroll.ts index fa6cde0d..4e05aecb 100644 --- a/src/components/QuickNav/hashScroll.ts +++ b/src/components/QuickNav/hashScroll.ts @@ -57,11 +57,11 @@ const scheduleScrollRetries = (attempt: () => boolean, postLayout: PostLayoutRet const cancel = () => { cancelled = true; if (rafId) { - cancelAnimationFrame(rafId); + window.cancelAnimationFrame(rafId); rafId = 0; } if (timeoutId) { - clearTimeout(timeoutId); + window.clearTimeout(timeoutId); timeoutId = 0; } }; @@ -103,7 +103,7 @@ const scheduleScrollRetries = (attempt: () => boolean, postLayout: PostLayoutRet } if (remaining > 0) { - rafId = requestAnimationFrame(() => retryFrames(remaining - 1)); + rafId = window.requestAnimationFrame(() => retryFrames(remaining - 1)); return; } @@ -153,7 +153,7 @@ export const scheduleDeferredScrollToTop = ( ): CancelHashScroll => { let cancelled = false; let cancelScroll: CancelHashScroll | undefined; - const rafId = requestAnimationFrame(() => { + const rafId = window.requestAnimationFrame(() => { if (cancelled || hasWindowHash()) { return; } @@ -163,7 +163,7 @@ export const scheduleDeferredScrollToTop = ( return () => { cancelled = true; - cancelAnimationFrame(rafId); + window.cancelAnimationFrame(rafId); cancelScroll?.(); }; };