Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/navlist-parent-flicker-fix.md
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 18 additions & 0 deletions packages/react/src/NavList/NavList.test.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -148,6 +149,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'})
Expand Down
48 changes: 36 additions & 12 deletions packages/react/src/NavList/NavList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,23 +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])')

if (currentItem) {
setContainsCurrentItem(true)
setIsOpen(true)
}
// Check if SubNav contains current item
// valid values: page, step, location, date, time, true and false
const currentItem = hasCurrentNavItem(subNav)
setContainsCurrentItem(Boolean(currentItem))

if (currentItem) {
setIsOpen(true)
}
}, [subNav, buttonId])

Expand Down