fix issues with navlist static to interactive subnav state

.changeset/navlist-parent-flicker-fix.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Fix NavList parent item flicker during static-to-interactive transitions when navigating between current sub-items in a SubNav.

packages/react/src/NavList/NavList.dev.stories.tsx

@@ -1,4 +1,6 @@
 import type {Meta} from '@storybook/react-vite'
+import React, {useEffect} from 'react'
+import {renderToStaticMarkup} from 'react-dom/server'
 import {PageLayout} from '../PageLayout'
 import {NavList} from './NavList'
 import {ArrowRightIcon, ArrowLeftIcon, BookIcon, FileDirectoryIcon} from '@primer/octicons-react'
@@ -69,3 +71,87 @@ export const WithGroupTitleAndHeading = () => (
     <PageLayout.Content></PageLayout.Content>
   </PageLayout>
 )
+
+type NavigationItem = 'item1' | 'subitem1' | 'subitem2' | 'item3'
+
+const NAVIGATION_FALLBACK_ITEM: NavigationItem = 'subitem1'
+
+function getNavigationItemFromHash(): NavigationItem {
+  const hash = window.location.hash.replace('#', '')
+  if (hash === 'item1' || hash === 'subitem1' || hash === 'subitem2' || hash === 'item3') {
+    return hash
+  }
+  return NAVIGATION_FALLBACK_ITEM
+}
+
+function StaticTransitionNav({currentItem}: {currentItem: NavigationItem}) {
+  return (
+    <PageLayout>
+      <PageLayout.Pane position="start">
+        <NavList>
+          <NavList.Item href="#item1" aria-current={currentItem === 'item1' ? 'page' : 'false'}>
+            Item 1
+          </NavList.Item>
+          <NavList.Item>
+            Item 2
+            <NavList.SubNav>
+              <NavList.Item href="#subitem1" aria-current={currentItem === 'subitem1' ? 'page' : 'false'}>
+                Sub item 1
+              </NavList.Item>
+              <NavList.Item href="#subitem2" aria-current={currentItem === 'subitem2' ? 'page' : 'false'}>
+                Sub item 2
+              </NavList.Item>
+            </NavList.SubNav>
+          </NavList.Item>
+          <NavList.Item href="#item3" aria-current={currentItem === 'item3' ? 'page' : 'false'}>
+            Item 3
+          </NavList.Item>
+        </NavList>
+      </PageLayout.Pane>
+      <PageLayout.Content></PageLayout.Content>
+    </PageLayout>
+  )
+}
+
+export const SubNavStaticToInteractiveTransition = () => {
+  const [currentItem, setCurrentItem] = React.useState<NavigationItem>(() => getNavigationItemFromHash())
+  const [renderMode, setRenderMode] = React.useState<'static' | 'interactive'>('static')
+
+  const staticMarkup = React.useMemo(() => {
+    return renderToStaticMarkup(<StaticTransitionNav currentItem={currentItem} />)
+  }, [currentItem])
+
+  useEffect(() => {
+    const abortController = new AbortController()
+    let transitionTimer: number | null = null
+
+    const showStaticThenInteractive = () => {
+      setCurrentItem(getNavigationItemFromHash())
+      setRenderMode('static')
+
+      if (transitionTimer) {
+        window.clearTimeout(transitionTimer)
+      }
+
+      // Keep static markup visible long enough to make the collapsed frame obvious.
+      transitionTimer = window.setTimeout(() => {
+        setRenderMode('interactive')
+      }, 180)
+    }
+
+    showStaticThenInteractive()
+    window.addEventListener('hashchange', showStaticThenInteractive, {signal: abortController.signal})
+
+    return () => {
+      abortController.abort()
+      if (transitionTimer) {
+        window.clearTimeout(transitionTimer)
+      }
+    }
+  }, [])
+
+  if (renderMode === 'static') return <div dangerouslySetInnerHTML={{__html: staticMarkup}} />
+  return <StaticTransitionNav currentItem={currentItem} />
+}
+
+SubNavStaticToInteractiveTransition.storyName = 'SubNav static-to-interactive transition'

packages/react/src/NavList/NavList.test.tsx

@@ -1,6 +1,7 @@
 import {describe, it, expect, vi} from 'vitest'
 import {render, fireEvent, act} from '@testing-library/react'
 import React from 'react'
+import {renderToStaticMarkup} from 'react-dom/server'
 import {NavList} from './NavList'
 import {ReactRouterLikeLink} from '../Pagination/mocks/ReactRouterLink'
 import {implementsClassName} from '../utils/testing'
@@ -148,6 +149,11 @@ describe('NavList.Item with NavList.SubNav', () => {
     expect(queryByRole('list', {name: 'Item 2'})).toBeNull()
   })
 
+  it('renders parent item expanded on initial static render when SubNav contains the current item', () => {
+    const markup = renderToStaticMarkup(<NavListWithCurrentSubNav />)
+    expect(markup).toContain('aria-expanded="true"')
+  })
+
   it('hides SubNav by default if SubNav does not contain the current item', () => {
     const {queryByRole} = render(<NavListWithSubNav />)
     const subNav = queryByRole('list', {name: 'Item 2'})

packages/react/src/NavList/NavList.tsx

@@ -123,21 +123,47 @@ const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: s
   isOpen: false,
 })
 
+function hasCurrentNavItem(node: React.ReactNode): boolean {
+  if (
+    !isValidElement<{
+      children?: React.ReactNode
+      'aria-current'?: NavListItemProps['aria-current']
+    }>(node)
+  ) {
+    return false
+  }
+
+  const ariaCurrent = node.props['aria-current']
+  if (Boolean(ariaCurrent) && ariaCurrent !== 'false') {
+    return true
+  }
+
+  if (!node.props.children) {
+    return false
+  }
+
+  return React.Children.toArray(node.props.children).some(hasCurrentNavItem)
+}
+
 function ItemWithSubNav({children, subNav, depth: _depth, defaultOpen, style}: ItemWithSubNavProps) {
   const buttonId = useId()
   const subNavId = useId()
-  const [isOpen, setIsOpen] = React.useState((defaultOpen || null) ?? false)
-  const subNavRef = React.useRef<HTMLDivElement>(null)
-  const [containsCurrentItem, setContainsCurrentItem] = React.useState(false)
+
+  // We have to use recursion to check if the current nav item is part of the subnav before initial render
+  // which is why we can't use the querySelector on the ref as it will cause the parent item to blink during first render.
+  const hasCurrentItem = React.useMemo(() => hasCurrentNavItem(subNav), [subNav])
+  const [isOpen, setIsOpen] = React.useState((defaultOpen || null) ?? hasCurrentItem)
+  const subNavRef = React.useRef<HTMLUListElement>(null)
+  const [containsCurrentItem, setContainsCurrentItem] = React.useState(hasCurrentItem)
 
   useIsomorphicLayoutEffect(() => {
     if (subNavRef.current) {
       // Check if SubNav contains current item
       // valid values: page, step, location, date, time, true and false
       const currentItem = subNavRef.current.querySelector('[aria-current]:not([aria-current=false])')
+      setContainsCurrentItem(Boolean(currentItem))
 
       if (currentItem) {
-        setContainsCurrentItem(true)
         setIsOpen(true)
       }
     }