diff --git a/.changeset/navlist-parent-flicker-fix.md b/.changeset/navlist-parent-flicker-fix.md new file mode 100644 index 00000000000..8c0e46ee1f7 --- /dev/null +++ b/.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. diff --git a/packages/react/src/NavList/NavList.test.tsx b/packages/react/src/NavList/NavList.test.tsx index 81e79482cef..9ed86a1d005 100644 --- a/packages/react/src/NavList/NavList.test.tsx +++ b/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,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() + } 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() const subNav = queryByRole('list', {name: 'Item 2'}) diff --git a/packages/react/src/NavList/NavList.tsx b/packages/react/src/NavList/NavList.tsx index c00ef3813e0..de08115d082 100644 --- a/packages/react/src/NavList/NavList.tsx +++ b/packages/react/src/NavList/NavList.tsx @@ -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(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(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])