fix issues with navlist static to interactive subnav state #7511

RSoeborg · 2026-02-07T22:46:49Z

Changelog

This PR fixes a visual flicker in NavList where a parent item with SubNav could briefly appear in the wrong state when navigating between sibling sub-items.

This can be seen here (example happening on the docs site `primer.style`):

example.from.docs.mov

Notice how Foundations flicker when I change submenu.

Here is a minimal example (amplified)

amplified.example.mov

I updated the dev storybook with an amplified example, that makes it easier to see whats actually happening.

Example after fix

postfix.mov

This is how it looks after the fix.

Rollout strategy

Patch release
Minor release
Major release; if selected, include a written rollout or migration plan
None; if selected, include a brief description as to why

Testing & Reviewing

Just replace these parts:

  const [isOpen, setIsOpen] = React.useState((defaultOpen || null) ?? hasCurrentItem)
  const subNavRef = React.useRef<HTMLUListElement>(null)
  const [containsCurrentItem, setContainsCurrentItem] = React.useState(hasCurrentItem)

With the "current" behaviour:

  const [isOpen, setIsOpen] = React.useState((defaultOpen || null) ?? false)
  const subNavRef = React.useRef<HTMLUListElement>(null)
  const [containsCurrentItem, setContainsCurrentItem] = React.useState(false)

and test in storybook: http://localhost:6006/?path=/story/components-navlist-dev--sub-nav-static-to-interactive-transition

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews (Storybook)
Changes are SSR compatible
Tested in Chrome
Tested in Firefox
Tested in Safari
Tested in Edge
(GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

…dant

…tion

changeset-bot · 2026-02-07T22:46:54Z

🦋 Changeset detected

Latest commit: b8994ab

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copilot

Pull request overview

Fixes a NavList flicker where a parent item with a SubNav briefly renders in the wrong expanded/current state when transitioning from static (SSR markup) to interactive (hydrated) state.

Changes:

Add a recursive pre-render check to detect whether a SubNav contains the current item and initialize the parent’s open state accordingly.
Add an SSR-focused unit test to verify initial expanded rendering when a current SubNav item exists.
Add a dev Storybook scenario that reproduces the static-to-interactive transition flicker.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
packages/react/src/NavList/NavList.tsx	Pre-computes “contains current item” before first render to avoid initial-state mismatch/flicker.
packages/react/src/NavList/NavList.test.tsx	Adds SSR test to validate initial expanded state when SubNav contains the current item.
packages/react/src/NavList/NavList.dev.stories.tsx	Adds an amplified Storybook repro by switching between static HTML and interactive render.

Copilot · 2026-02-07T22:48:04Z

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'


Importing renderToStaticMarkup from react-dom/server in a Storybook story (browser bundle) can break Vite/Storybook builds because react-dom/server is Node-oriented. Prefer switching this import to react-dom/server.browser (if available in your React version), or restructure the story to avoid bundling the server renderer into the client (e.g., generate the static markup outside the browser bundle / via a build-time step).

Suggested change

import {renderToStaticMarkup} from 'react-dom/server'

import {renderToStaticMarkup} from 'react-dom/server.browser'

We can remove the .dev.stories after a maintainer has tested the changes locally. It's just to showcase the bug

Copilot · 2026-02-07T22:48:04Z

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

+  const staticMarkup = React.useMemo(() => {
+    return renderToStaticMarkup(<StaticTransitionNav currentItem={currentItem} />)
+  }, [currentItem])


Importing renderToStaticMarkup from react-dom/server in a Storybook story (browser bundle) can break Vite/Storybook builds because react-dom/server is Node-oriented. Prefer switching this import to react-dom/server.browser (if available in your React version), or restructure the story to avoid bundling the server renderer into the client (e.g., generate the static markup outside the browser bundle / via a build-time step).

Same as above, we can remove the .dev.stories after a maintainer has tested the changes locally. It's just to showcase the bug

Copilot · 2026-02-07T22:48:05Z

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

+  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"')
+  })


This assertion isn’t scoped to the parent item that should be expanded (it will pass if any aria-expanded="true" exists anywhere in the markup). Consider parsing the markup into a DOM container and asserting aria-expanded on the specific control associated with “Item 2” (e.g., query the toggle button by accessible name / nearby text, then assert its aria-expanded value).

I was thinking about it, but the test works to test what we want atm. I don't want to overcomplicate the test too much. An additional thought about this, is that maybe we should supress this warning:

Warning: useLayoutEffect does nothing on the server, because its effect cannot be encoded into the server renderer's output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this, useLayoutEffect should only be used in components that render exclusively on the client. See https://reactjs.org/link/uselayouteffect-ssr for common fixes.%s at ItemWithSubNav (http://localhost:63315/src/NavList/NavList.tsx:210:13) at http://localhost:63315/src/NavList/NavList.tsx:75:13 at ul at UnwrappedList (http://localhost:63315/src/ActionList/List.tsx:15:9) at nav at http://localhost:63315/src/NavList/NavList.tsx:17:13 at NavListWithCurrentSubNav (http://localhost:63315/Users/rso/repos/personal/primer-react/packages/react/src/NavList/NavList.test.tsx?import&browserv=1770504860076:189:15)

Using a consoleSpy and just ignoring the output like vi.spyOn(console, 'error').mockImplementation(() => null) ? I didn't do it because I'm not entirely sure if its bad practise or not.

RSoeborg added 2 commits February 7, 2026 23:30

fix(navlist): initialize subnav parent open state from current descen…

3e306f3

…dant

chore(navlist): add dev story for static-to-interactive subnav transi…

e917a34

…tion

RSoeborg requested a review from a team as a code owner February 7, 2026 22:46

RSoeborg requested review from Copilot and hectahertz February 7, 2026 22:46

Copilot AI reviewed Feb 7, 2026

View reviewed changes

RSoeborg requested a deployment to github-pages February 7, 2026 22:56 — with GitHub Actions In progress

ci: add changeset file for pr

b8994ab

RSoeborg deployed to github-pages February 7, 2026 23:15 — with GitHub Actions View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix issues with navlist static to interactive subnav state #7511

fix issues with navlist static to interactive subnav state #7511

Uh oh!

RSoeborg commented Feb 7, 2026 •

edited

Loading

Uh oh!

changeset-bot bot commented Feb 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

RSoeborg Feb 7, 2026

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

RSoeborg Feb 7, 2026

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

RSoeborg Feb 7, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	import {renderToStaticMarkup} from 'react-dom/server'
	import {renderToStaticMarkup} from 'react-dom/server.browser'

fix issues with navlist static to interactive subnav state #7511

Are you sure you want to change the base?

fix issues with navlist static to interactive subnav state #7511

Uh oh!

Conversation

RSoeborg commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog

This can be seen here (example happening on the docs site primer.style):

Here is a minimal example (amplified)

Example after fix

Rollout strategy

Testing & Reviewing

Merge checklist

Uh oh!

changeset-bot bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

RSoeborg Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

RSoeborg Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

RSoeborg Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

RSoeborg commented Feb 7, 2026 •

edited

Loading

This can be seen here (example happening on the docs site `primer.style`):

changeset-bot bot commented Feb 7, 2026 •

edited

Loading

RSoeborg Feb 7, 2026 •

edited

Loading