Fix UnderlineNav not re-rendering when child tabs change

.changeset/underlinenav-rerender-on-children-change.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Fix `UnderlineNav` not re-rendering when child tabs are added, removed, or reordered.

packages/react/src/UnderlineNav/UnderlineNav.features.stories.tsx

@@ -13,6 +13,7 @@ import {
 } from '@primer/octicons-react'
 import type {Meta} from '@storybook/react-vite'
 import {UnderlineNav} from './index'
+import {Button} from '../Button'
 import {INITIAL_VIEWPORTS} from 'storybook/viewport'
 
 const meta = {
@@ -154,3 +155,29 @@ export const VariantFlush = () => {
     </UnderlineNav>
   )
 }
+
+export const DynamicChildren = () => {
+  const [showItem, setShowItem] = React.useState(false)
+
+  return (
+    <div>
+      <Button
+        onClick={() => {
+          setShowItem(prev => !prev)
+        }}
+      >
+        {showItem ? 'Hide item' : 'Show item'}
+      </Button>
+      <UnderlineNav aria-label="Repository">
+        <UnderlineNav.Item href="#" aria-current="page">
+          Code
+        </UnderlineNav.Item>
+        <UnderlineNav.Item href="#">Pull requests</UnderlineNav.Item>
+        <UnderlineNav.Item href="#">Actions</UnderlineNav.Item>
+        <UnderlineNav.Item href="#">Projects</UnderlineNav.Item>
+        <UnderlineNav.Item href="#">Wiki</UnderlineNav.Item>
+        {showItem && <UnderlineNav.Item href="#">Another</UnderlineNav.Item>}
+      </UnderlineNav>
+    </div>
+  )
+}

packages/react/src/UnderlineNav/UnderlineNav.test.tsx

@@ -1,5 +1,5 @@
 import {describe, expect, it, vi} from 'vitest'
-import type React from 'react'
+import React from 'react'
 import {render, screen} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import {
@@ -240,6 +240,50 @@ describe('UnderlineNav', () => {
     const textSpan = item.querySelector('[data-component="text"]')
     expect(textSpan).toHaveAttribute('data-content', 'Simple Text')
   })
+
+  // Regression test for https://github.com/github/primer/issues/6389
+  it('re-renders when a conditionally rendered child tab is toggled', async () => {
+    const Example = () => {
+      const [showItem, setShowItem] = React.useState(false)
+      return (
+        <div>
+          <button type="button" onClick={() => setShowItem(prev => !prev)}>
+            {showItem ? 'hide item' : 'show item'}
+          </button>
+          <UnderlineNav aria-label="Repository">
+            <UnderlineNav.Item href="#" aria-current="page">
+              Code
+            </UnderlineNav.Item>
+            <UnderlineNav.Item href="#">Pull requests</UnderlineNav.Item>
+            <UnderlineNav.Item href="#">Actions</UnderlineNav.Item>
+            <UnderlineNav.Item href="#">Projects</UnderlineNav.Item>
+            <UnderlineNav.Item href="#">Wiki</UnderlineNav.Item>
+            {showItem && <UnderlineNav.Item href="#">Another</UnderlineNav.Item>}
+          </UnderlineNav>
+        </div>
+      )
+    }
+
+    render(<Example />)
+    const toggle = screen.getByRole('button', {name: 'show item'})
+    const user = userEvent.setup()
+
+    // Initially the conditional tab is not rendered
+    expect(screen.queryByText('Another')).not.toBeInTheDocument()
+
+    // Toggle on: the new tab should appear in the DOM (either in the visible list
+    // or the overflow menu) without the nav being remounted.
+    await user.click(toggle)
+    expect(screen.getByText('Another')).toBeInTheDocument()
+    // Existing tabs are still present
+    expect(screen.getByText('Code')).toBeInTheDocument()
+    expect(screen.getByText('Pull requests')).toBeInTheDocument()
+
+    // Toggle off: the tab should be removed
+    await user.click(screen.getByRole('button', {name: 'hide item'}))
+    expect(screen.queryByText('Another')).not.toBeInTheDocument()
+    expect(screen.getByText('Code')).toBeInTheDocument()
+  })
 })
 
 describe('Keyboard Navigation', () => {

packages/react/src/UnderlineNav/UnderlineNav.tsx

@@ -175,17 +175,36 @@ export const UnderlineNav = forwardRef(
       menuItems: [],
     })
 
+    // The ordered signature drives `useResizeObserver` deps below so it re-fires whenever
+    // children are added, removed, or reordered.
+    const childrenSignature = validChildren.map(child => String(child.key ?? '')).join('|')
+
+    // `childrenChanged` only tracks membership (set of keys), not order. This is intentional
+    // because `swapMenuItemWithListItem` legitimately reorders `responsiveProps.items` (to keep
+    // the selected item visible), and we don't want that to be treated as a stale-children
+    // situation. We only need to fall back to rendering `validChildren` directly when a child
+    // was actually added or removed (so a new item isn't missed and a removed item isn't kept).
+    const responsiveKeys = new Set<string>()
+    for (const item of responsiveProps.items) responsiveKeys.add(String(item.key ?? ''))
+    for (const item of responsiveProps.menuItems) responsiveKeys.add(String(item.key ?? ''))
+    const validKeys = validChildren.map(child => String(child.key ?? ''))
+    const childrenChanged = validKeys.length !== responsiveKeys.size || validKeys.some(key => !responsiveKeys.has(key))
+
     // Make sure to have the fresh props data for list items when children are changed (keeping aria-current up-to-date)
-    const listItems = responsiveProps.items.map(item => {
-      return validChildren.find(child => child.key === item.key) ?? item
-    })
+    const listItems = childrenChanged
+      ? validChildren
+      : responsiveProps.items.map(item => {
+          return validChildren.find(child => child.key === item.key) ?? item
+        })
 
     // Make sure to have the fresh props data for menu items when children are changed (keeping aria-current up-to-date)
-    const menuItems = responsiveProps.menuItems.map(menuItem => {
-      return validChildren.find(child => child.key === menuItem.key) ?? menuItem
-    })
+    const menuItems = childrenChanged
+      ? []
+      : responsiveProps.menuItems.map(menuItem => {
+          return validChildren.find(child => child.key === menuItem.key) ?? menuItem
+        })
     // This is the case where the viewport is too narrow to show any list item with the more menu. In this case, we only show the dropdown
-    const onlyMenuVisible = responsiveProps.items.length === 0
+    const onlyMenuVisible = !childrenChanged && responsiveProps.items.length === 0
 
     useDevOnlyEffect(() => {
       // Address illegal state where there are multiple items that have `aria-current='page'` attribute
@@ -294,19 +313,27 @@ export const UnderlineNav = forwardRef(
 
     useOnOutsideClick({onClickOutside: closeOverlay, containerRef, ignoreClickRefs: [moreMenuBtnRef]})
 
-    useResizeObserver((resizeObserverEntries: ResizeObserverEntry[]) => {
-      const navWidth = resizeObserverEntries[0].contentRect.width
-      const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0
-      navWidth !== 0 &&
-        overflowEffect(
-          navWidth,
-          moreMenuWidth,
-          validChildren,
-          childWidthArray,
-          noIconChildWidthArray,
-          updateListAndMenu,
-        )
-    }, navRef as RefObject<HTMLElement>)
+    // The resize observer fires when the nav element resizes, and re-observes (firing the
+    // callback again with current size) whenever its deps change. Passing `childrenSignature`
+    // ensures the overflow calculation re-runs when child tabs are added, removed, or
+    // reordered. See https://github.com/github/primer/issues/6389.
+    useResizeObserver(
+      (resizeObserverEntries: ResizeObserverEntry[]) => {
+        const navWidth = resizeObserverEntries[0].contentRect.width
+        const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0
+        navWidth !== 0 &&
+          overflowEffect(
+            navWidth,
+            moreMenuWidth,
+            validChildren,
+            childWidthArray,
+            noIconChildWidthArray,
+            updateListAndMenu,
+          )
+      },
+      navRef as RefObject<HTMLElement>,
+      [childrenSignature],
+    )
 
     // Compute menuInlineStyles if needed
     let menuInlineStyles: React.CSSProperties = {...baseMenuInlineStyles}