Highlight the clicked sidebar item immediately (#62499)

Ebonsignori · web-flow · commit f28f18c93855 · 2026-07-29T23:21:19.000Z
diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts
@@ -52,6 +52,58 @@ test('use sidebar to go to Hello World page', async ({ page }) => {
   await expect(page).toHaveTitle(/Hello World - GitHub Docs/)
 })
 
+test('sidebar highlights the clicked item optimistically while navigation is pending', async ({
+  page,
+}) => {
+  // Article pages are getServerSideProps routes, so router.asPath (and thus the real
+  // aria-current) only updates after the destination loads. The sidebar marks the
+  // clicked link with a visual-only `data-pending` accent so the click is acknowledged
+  // immediately. Throttle the client-side data fetch so the navigation stays pending
+  // long enough to observe that intermediate state.
+  await page.goto('/get-started')
+  await page.getByTestId('product-sidebar').getByText('Start your journey').click()
+
+  const sidebar = page.getByTestId('product-sidebar')
+  const helloWorld = sidebar.getByRole('link', { name: 'Hello World' })
+  const linkRewriting = sidebar.getByRole('link', { name: 'Link rewriting' })
+
+  // Hold the next data request open until we release it, so navigation stays pending.
+  let releaseNavigation = () => {}
+  const navigationHeld = new Promise<void>((resolve) => {
+    releaseNavigation = resolve
+  })
+  await page.route('**/_next/data/**', async (route) => {
+    await navigationHeld
+    await route.continue()
+  })
+
+  await helloWorld.click()
+
+  // While pending: the clicked link carries the optimistic visual marker, but the URL
+  // and the semantic aria-current still reflect the (still-loaded) get-started page.
+  await expect(helloWorld).toHaveAttribute('data-pending', '')
+  await expect(helloWorld).not.toHaveAttribute('aria-current', 'page')
+  await expect(page).not.toHaveURL(/hello-world/)
+
+  // Let the navigation finish: the marker gives way to a real aria-current.
+  releaseNavigation()
+  await expect(page).toHaveURL(/\/en\/get-started\/start-your-journey\/hello-world/)
+  await expect(helloWorld).toHaveAttribute('aria-current', 'page')
+  await expect(helloWorld).not.toHaveAttribute('data-pending', '')
+
+  // A modifier-click (open in new tab) must NOT move the optimistic selection.
+  // handleNavClick bails on modifier clicks, so pendingHref is never set: the current
+  // page keeps its URL, its aria-current, and the clicked link gets no data-pending.
+  // Use ControlOrMeta so the real "open in new tab" modifier is sent per-platform
+  // (Ctrl on Linux/Windows CI, Meta on macOS). The click opens a background tab we
+  // don't need to assert on; catch any popup so it doesn't leak.
+  page.on('popup', (popup) => popup.close())
+  await linkRewriting.click({ modifiers: ['ControlOrMeta'] })
+  await expect(linkRewriting).not.toHaveAttribute('data-pending', '')
+  await expect(helloWorld).toHaveAttribute('aria-current', 'page')
+  await expect(page).toHaveURL(/\/en\/get-started\/start-your-journey\/hello-world/)
+})
+
 test('press "/" to open the search overlay', async ({ page }) => {
   await page.goto('/')
   await turnOffExperimentsInPage(page)
diff --git a/src/landings/components/SidebarProduct.module.scss b/src/landings/components/SidebarProduct.module.scss
@@ -7,10 +7,15 @@
   // so a top-level article like "Quickstart" gets only the subtle background pill
   // and no green bar. Restore the bar for active top-level leaves to match nested
   // items. Brand's classes are hashed, so match on the stable substrings.
+  //
+  // This also covers `[data-pending]` — the optimistic marker on a just-clicked item
+  // (see the pending rules below) — so a clicked top-level leaf gets the bar too.
   :global([class*="NavList__item--leaf"][class*="NavList__item--level-1"])
     :global([class*="NavList__link"])[aria-current]:not(
       [aria-current="false"]
-    )::before {
+    )::before,
+  :global([class*="NavList__item--leaf"][class*="NavList__item--level-1"])
+    :global([class*="NavList__link"])[data-pending]::before {
     content: "";
     position: absolute;
     // Level-1 links are shorter (~24px) than nested leaves, so use a small inset
@@ -25,4 +30,38 @@
     border-radius: var(--base-size-2);
     background-color: var(--brand-NavList-activeIndicator-color);
   }
+
+  // Optimistic "pending" highlight. Article pages are getServerSideProps routes and can
+  // be slow to load, so router.asPath — and thus aria-current — only updates once the
+  // page has loaded. We keep aria-current on the *loaded* page (assistive tech must not
+  // be told a still-loading destination is the current page), and instead mark the
+  // just-clicked link with `data-pending` for a VISUAL-ONLY accent. Brand couples its
+  // active styling to `[aria-current]`, so mirror that treatment here for `[data-pending]`.
+  // data-pending is only set while a *different* page is loading, so it never collides
+  // with the real aria-current item.
+  :global([class*="NavList__link"])[data-pending] {
+    color: var(--brand-color-text-default);
+    background-color: var(--brand-color-canvas-subtle);
+  }
+
+  // Nested leaves: brand moves the background pill onto the label and adds a leading
+  // accent bar (mirrors `.item--leaf:not(.item--level-1) .link[aria-current]`).
+  :global([class*="NavList__item--leaf"]:not([class*="NavList__item--level-1"]))
+    :global([class*="NavList__link"])[data-pending] {
+    background-color: transparent;
+
+    :global([class*="NavList__labelArea"]) {
+      background-color: var(--brand-color-canvas-subtle);
+    }
+
+    &::before {
+      content: "";
+      position: absolute;
+      inset-block: var(--base-size-8);
+      inset-inline-start: 0;
+      width: var(--base-size-4);
+      border-radius: var(--base-size-2);
+      background-color: var(--brand-NavList-activeIndicator-color);
+    }
+  }
 }
diff --git a/src/landings/components/SidebarProduct.tsx b/src/landings/components/SidebarProduct.tsx
@@ -27,7 +27,9 @@ type Router = ReturnType<typeof useRouter>
 // not next/link), so intercept clicks to restore next/link-style client-side
 // navigation. Modifier/middle clicks fall through to the browser so open-in-new-tab
 // still works, and the <a href> keeps links crawlable for SSR. Mirrors Breadcrumbs.tsx.
-function handleNavClick(router: Router, event: MouseEvent<HTMLElement>, href: string) {
+// Returns true when it performed a client-side navigation (so the caller can move the
+// optimistic selection), false when the click was left to the browser.
+function handleNavClick(router: Router, event: MouseEvent<HTMLElement>, href: string): boolean {
   if (
     event.defaultPrevented ||
     event.button !== 0 ||
@@ -37,20 +39,28 @@ function handleNavClick(router: Router, event: MouseEvent<HTMLElement>, href: st
     event.altKey ||
     !href.startsWith('/')
   ) {
-    return
+    return false
   }
   event.preventDefault()
   // hrefs already include the locale prefix (e.g. /en/...), so disable Next.js
   // locale handling to avoid double-prefixing.
   router.push(href, undefined, { locale: false })
+  return true
 }
 
 // The sidebar renders the full product tree (hundreds of nodes) and fully remounts
 // on every navigation (key={asPath} in SidebarNav). To keep per-item cost down we
 // subscribe to the router ONCE here and hand items a stable routePath plus stable
 // navigate/prefetch callbacks, instead of every item calling useRouter itself.
 type SidebarNavValue = {
+  // The real loaded route. Drives aria-current (the semantic "current page") and the
+  // auto-expanded active ancestor chain — both must reflect the page actually loaded.
   routePath: string
+  // The in-flight click target, or null. Drives a VISUAL-ONLY optimistic accent bar
+  // (via data-pending) so the click feels acknowledged before the slow
+  // getServerSideProps page loads — without lying to assistive tech about the current
+  // page. Once navigation completes, the keyed remount clears it and routePath catches up.
+  pendingHref: string | null
   navigate: (event: MouseEvent<HTMLElement>, href: string) => void
   prefetch: (href: string) => void
 }
@@ -64,6 +74,17 @@ function useSidebarNav(): SidebarNavValue {
   return value
 }
 
+// Props for a leaf link's <a>: aria-current tracks the loaded page (semantics), while
+// data-pending marks the in-flight click so CSS can move the accent bar optimistically
+// without changing what screen readers announce as current. data-pending is only set
+// while a *different* page is loading, so it never double-marks the already-current item.
+function leafLinkProps(nav: SidebarNavValue, href: string) {
+  return {
+    'aria-current': (nav.routePath === href ? 'page' : false) as 'page' | false,
+    'data-pending': nav.pendingHref === href && nav.routePath !== href ? '' : undefined,
+  }
+}
+
 // Separate context for the REST-only scroll-spy state (full asPath with query+hash,
 // and query). Kept out of SidebarNavValue so its per-navigation identity churn
 // doesn't invalidate the memoized common items — only RestNavListItem consumes it.
@@ -103,19 +124,43 @@ export const SidebarProduct = () => {
   const { asPath, locale, query } = router
   const routePath = `/${locale}${asPath.split('?')[0].split('#')[0]}`
 
+  // Optimistic selection: the href of an in-flight click. Used to move the accent bar
+  // visually (data-pending) the instant a link is clicked, even while the destination
+  // page is still loading. This SidebarProduct instance persists during the pending
+  // fetch (SidebarNav keys it on asPath, which only changes once navigation completes),
+  // so the state survives the wait and is discarded by the keyed remount when the new
+  // route lands. aria-current is NOT derived from this — it stays on the loaded route.
+  const [pendingHref, setPendingHref] = useState<string | null>(null)
+
   const prefetchHref = usePrefetchOnInteraction()
   // Stable callbacks so memoized items don't re-render on unrelated changes.
   const navigate = useCallback(
-    (event: MouseEvent<HTMLElement>, href: string) => handleNavClick(router, event, href),
+    (event: MouseEvent<HTMLElement>, href: string) => {
+      // Only move the optimistic highlight on a real client-side nav, not on a
+      // modifier/middle click that opens a new tab (the current page stays put).
+      if (handleNavClick(router, event, href)) setPendingHref(href)
+    },
     [router],
   )
   const prefetch = useCallback((href: string) => prefetchHref(router, href), [router, prefetchHref])
   const navValue = useMemo<SidebarNavValue>(
-    () => ({ routePath, navigate, prefetch }),
-    [routePath, navigate, prefetch],
+    () => ({ routePath, pendingHref, navigate, prefetch }),
+    [routePath, pendingHref, navigate, prefetch],
   )
   const restNavValue = useMemo<RestNavValue>(() => ({ asPath, query }), [asPath, query])
 
+  useEffect(() => {
+    // Clear the optimistic highlight if a navigation genuinely fails, so it doesn't
+    // stick on a page that never loaded. Skip cancellations (err.cancelled) — those
+    // fire when a second click supersedes the first, and pendingHref already points at
+    // that newer target, which we want to keep highlighted.
+    const clearPending = (err: { cancelled?: boolean }) => {
+      if (!err?.cancelled) setPendingHref(null)
+    }
+    router.events.on('routeChangeError', clearPending)
+    return () => router.events.off('routeChangeError', clearPending)
+  }, [router.events])
+
   useEffect(() => {
     // Brand NavList auto-expands the whole ancestor chain of the active item, so
     // scroll to the item marked aria-current="page" (the active article) rather
@@ -243,14 +288,14 @@ function navListLevelSentinel() {
 }
 
 const LeafLink = memo(function LeafLink({ node }: { node: ProductTreeNode }) {
-  const { routePath, navigate, prefetch } = useSidebarNav()
+  const nav = useSidebarNav()
   return (
     <NavList.Item
       as="a"
       href={node.href}
-      aria-current={routePath === node.href ? 'page' : false}
-      onClick={(event: MouseEvent<HTMLElement>) => navigate(event, node.href)}
-      {...prefetchHandlers(prefetch, node.href)}
+      {...leafLinkProps(nav, node.href)}
+      onClick={(event: MouseEvent<HTMLElement>) => nav.navigate(event, node.href)}
+      {...prefetchHandlers(nav.prefetch, node.href)}
     >
       {node.title}
     </NavList.Item>
@@ -264,9 +309,9 @@ const NavListItem = memo(function NavListItem({
   childPage: ProductTreeNode
   level?: number
 }) {
-  const { routePath, navigate, prefetch } = useSidebarNav()
+  const nav = useSidebarNav()
+  const { routePath, navigate, prefetch } = nav
   const locale = routePath.split('/')[1]
-  const isActive = routePath === childPage.href
   const hasChildren = childPage.childPages.length > 0
   const specialCategory = childPage.layout === 'category-landing'
   const canNest = level < MAX_NAVLIST_LEVEL
@@ -307,7 +352,7 @@ const NavListItem = memo(function NavListItem({
         <NavList.Item
           as="a"
           href={sidebarLinkHref}
-          aria-current={routePath === sidebarLinkHref ? 'page' : false}
+          {...leafLinkProps(nav, sidebarLinkHref)}
           onClick={(event: MouseEvent<HTMLElement>) => navigate(event, sidebarLinkHref)}
           {...prefetchHandlers(prefetch, sidebarLinkHref)}
         >
@@ -318,7 +363,7 @@ const NavListItem = memo(function NavListItem({
         <NavList.Item
           as="a"
           href={childPage.href}
-          aria-current={isActive ? 'page' : false}
+          {...leafLinkProps(nav, childPage.href)}
           onClick={(event: MouseEvent<HTMLElement>) => navigate(event, childPage.href)}
           {...prefetchHandlers(prefetch, childPage.href)}
         >
@@ -333,7 +378,8 @@ const NavListItem = memo(function NavListItem({
 })
 
 function RestNavListItem({ category }: { category: ProductTreeNode }) {
-  const { routePath, navigate, prefetch } = useSidebarNav()
+  const nav = useSidebarNav()
+  const { routePath, navigate, prefetch } = nav
   const { asPath, query } = useRestNav()
   const [visibleAnchor, setVisibleAnchor] = useState('')
   const miniTocItems =
@@ -378,7 +424,7 @@ function RestNavListItem({ category }: { category: ProductTreeNode }) {
       <NavList.Item
         as="a"
         href={category.href}
-        aria-current={routePath === category.href ? 'page' : false}
+        {...leafLinkProps(nav, category.href)}
         onClick={(event: MouseEvent<HTMLElement>) => navigate(event, category.href)}
         {...prefetchHandlers(prefetch, category.href)}
       >
@@ -429,7 +475,7 @@ function RestNavListItem({ category }: { category: ProductTreeNode }) {
             key={childPage.href}
             as="a"
             href={childPage.href}
-            aria-current={routePath === childPage.href ? 'page' : false}
+            {...leafLinkProps(nav, childPage.href)}
             onClick={(event: MouseEvent<HTMLElement>) => navigate(event, childPage.href)}
             {...prefetchHandlers(prefetch, childPage.href)}
           >
