fix issues with navlist static to interactive subnav state (#7511)

Co-authored-by: primer[bot] <119360173+primer[bot]@users.noreply.github.com>
Co-authored-by: Hector Garcia <hectahertz@github.com>
Co-authored-by: LiuLiu <liuliu-dev@github.com>

Author: 4 people

.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.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'
@@ -147,6 +148,23 @@ 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', () => {
+    // intentionally suppress the expected React SSR useLayoutEffect warning
+    // this test focuses specifically on the initial SSR render
+    const container = document.createElement('div')
+    const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => null)
+    try {
+      container.innerHTML = renderToStaticMarkup(<NavListWithCurrentSubNav />)
+    } finally {
+      consoleSpy.mockRestore()
+    }
+
+    const item2Button = container.querySelector('button[aria-expanded]')
+    expect(item2Button).not.toBeNull()
+    expect(item2Button).toHaveAttribute('aria-expanded', 'true')
+    expect(item2Button?.textContent).toBe('Item 2')
+  })
+
   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

@@ -137,23 +137,49 @@ 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])')
-
-      if (currentItem) {
-        setContainsCurrentItem(true)
-        setIsOpen(true)
-      }
+    // The React tree check handles the initial render before refs exist. The DOM fallback handles custom link
+    // components that compute aria-current internally instead of receiving it as a prop.
+    // valid values: page, step, location, date, time, true and false
+    const currentItem =
+      hasCurrentNavItem(subNav) || Boolean(subNavRef.current?.querySelector('[aria-current]:not([aria-current=false])'))
+    setContainsCurrentItem(currentItem)
+
+    if (currentItem) {
+      setIsOpen(true)
     }
   }, [subNav, buttonId])