Replace dynamic overflow with horizontal scroll in UnderlineNav

.changeset/underline-nav-css-overflow.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": minor
+---
+
+Removes `UnderlineNav` dynamic overflow functionality, replacing with simple scroll container to simplify logic, improve performance, and remove layout shifts, especially when using server-side rendering.

e2e/components/UnderlineNav.test.ts

@@ -199,19 +199,19 @@ test.describe('UnderlineNav', () => {
           })
 
           // Default state
-          // expect(await page.screenshot()).toMatchSnapshot()
+          expect(await page.screenshot()).toMatchSnapshot()
 
           await page.setViewportSize({width: viewports['primer.breakpoint.sm'], height: 768})
-          await page.locator('button', {hasText: 'More Repository Items'}).waitFor()
+          await page.locator('button', {hasText: 'More items'}).waitFor()
 
           // Resize
-          // expect(await page.screenshot()).toMatchSnapshot()
+          expect(await page.screenshot()).toMatchSnapshot()
 
-          await page.getByRole('button', {name: 'More Repository Items'}).click()
-          // expect(await page.screenshot()).toMatchSnapshot()
+          await page.getByRole('button', {name: 'More items'}).click()
+          expect(await page.screenshot()).toMatchSnapshot()
 
-          await page.getByRole('link', {name: 'Settings (10)'}).click()
-          // expect(await page.screenshot()).toMatchSnapshot()
+          await page.getByRole('menuitem', {name: 'Settings (10)'}).click()
+          expect(await page.screenshot()).toMatchSnapshot()
         })
 
         test('Hide icons when there is not enough space to display all list items @vrt', async ({page}) => {
@@ -223,61 +223,13 @@ test.describe('UnderlineNav', () => {
           })
 
           // Default State
-          // expect(await page.screenshot()).toMatchSnapshot()
+          expect(await page.screenshot()).toMatchSnapshot()
 
           // Resize
           await page.setViewportSize({width: viewports['primer.breakpoint.md'], height: 768})
 
           // Icons should be hidden
-          // expect(await page.screenshot()).toMatchSnapshot()
-        })
-
-        test('Keep selected item visible @vrt', async ({page}) => {
-          await visit(page, {
-            id: 'components-underlinenav-features--overflow-template',
-            globals: {
-              colorScheme: theme,
-            },
-          })
-          await page.setViewportSize({width: viewports['primer.breakpoint.sm'], height: 768})
-
-          await page.locator('button', {hasText: 'More Repository Items'}).waitFor()
-          await page.getByRole('button', {name: 'More Repository Items'}).click()
-          await page.getByRole('link', {name: 'Settings (10)'}).click()
-
-          // State after selecting the second last item
-          // expect(await page.screenshot()).toMatchSnapshot()
-
-          // Resize
-          await page.setViewportSize({
-            width: 1100,
-            height: 480,
-          })
-          await page.locator('button', {hasText: 'More Repository Items'}).waitFor({
-            state: 'hidden',
-          })
-
-          // Current state
-          // expect(await page.screenshot()).toMatchSnapshot()
-
-          // Resize
-          await page.setViewportSize({
-            width: 800,
-            height: 480,
-          })
-          await page.locator('button', {hasText: 'More Repository Items'}).waitFor()
-
-          // Current state
-          // expect(await page.screenshot()).toMatchSnapshot()
-
-          // Resize
-          await page.setViewportSize({
-            width: 600,
-            height: 480,
-          })
-          await page.locator('button', {hasText: 'More Repository Items'}).waitFor()
-          // Current state
-          // expect(await page.screenshot()).toMatchSnapshot()
+          expect(await page.screenshot()).toMatchSnapshot()
         })
       })
     }

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

@@ -77,11 +77,7 @@ const items: {navigation: string; icon: React.ReactElement; counter?: number | s
 export const OverflowTemplate = ({initialSelectedIndex = 1}: {initialSelectedIndex?: number}) => {
   const [selectedIndex, setSelectedIndex] = React.useState<number | null>(initialSelectedIndex)
   return (
-    <UnderlineNav
-      aria-label="Repository"
-      // @ts-ignore UnderlineNav does not take selectionVariant prop, but we need to pass it to the underlying ActionList so it doesn't show Selections.
-      selectionVariant={undefined}
-    >
+    <UnderlineNav aria-label="Repository">
       {items.map((item, index) => (
         <UnderlineNav.Item
           key={item.navigation}
@@ -106,7 +102,7 @@ export const OverflowTemplate = ({initialSelectedIndex = 1}: {initialSelectedInd
 }
 
 export const OverflowOnNarrowScreen = () => {
-  return <OverflowTemplate initialSelectedIndex={1} />
+  return <OverflowTemplate initialSelectedIndex={items.length - 1} />
 }
 
 OverflowOnNarrowScreen.parameters = {

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

@@ -110,7 +110,7 @@ SelectAMenuItem.play = async ({canvasElement}: {canvasElement: HTMLElement}) =>
 
   await delay(1000)
 
-  const moreBtn = canvas.getByRole('button', {name: 'More Repository items'})
+  const moreBtn = canvas.getByRole('button', {name: 'More items'})
   userEvent.hover(moreBtn)
 
   await delay()
@@ -131,35 +131,4 @@ SelectAMenuItem.play = async ({canvasElement}: {canvasElement: HTMLElement}) =>
   expect(lastListItem).toEqual(menuListItem)
 }
 
-const KeepSelectedItemVisible = () => {
-  return <OverflowTemplate initialSelectedIndex={7} />
-}
-// eslint-disable-next-line @typescript-eslint/ban-ts-comment
-// @ts-ignore
-KeepSelectedItemVisible.play = async ({canvasElement}: {canvasElement: HTMLElement}) => {
-  const canvas = within(canvasElement)
-  // await delay(2000)
-  const selectedItem = canvas.getByRole('link', {name: 'Settings (10)'})
-  expect(selectedItem).toHaveAttribute('aria-current', 'page')
-  // change viewport
-  canvasElement.style.width = '900px'
-  await delay(1000)
-  expect(selectedItem).toHaveAttribute('aria-current', 'page')
-  canvasElement.style.width = '800px'
-  await delay(1000)
-  expect(selectedItem).toHaveAttribute('aria-current', 'page')
-  canvasElement.style.width = '700px'
-  await delay(1000)
-  expect(selectedItem).toHaveAttribute('aria-current', 'page')
-  canvasElement.style.width = '600px'
-  await delay(1000)
-  expect(selectedItem).toHaveAttribute('aria-current', 'page')
-  canvasElement.style.width = '500px'
-  await delay(1000)
-  const lastListItem = canvas.getByRole('list').children[2].children[0]
-  const menuListItem = canvas.getByRole('link', {name: 'Settings (10)'})
-  // expect Settings be the last element on the list.
-  expect(lastListItem).toEqual(menuListItem)
-}
-
-export {KeyboardNavigation, SelectAMenuItem, KeepSelectedItemVisible}
+export {KeyboardNavigation, SelectAMenuItem}

packages/react/src/UnderlineNav/UnderlineNav.module.css

@@ -1,22 +1,37 @@
-.MenuItemContent {
-  display: flex;
-  align-items: center;
-  justify-content: space-between;
-}
+.ScrollContainer {
+  overflow-x: auto;
+  overflow-y: hidden;
+  -webkit-overflow-scrolling: auto;
+  scrollbar-width: thin;
+  scrollbar-color: var(--borderColor-muted) transparent;
+  scrollbar-gutter: stable;
+
+  /* Progressive enhancement: Detect overflow using scroll-based animations.
+  This lets us calculate the icon visibility during SSR. */
+  animation: detect-overflow linear;
+  animation-timeline: scroll(self inline);
+
+  --UnderlineNav_icons-display: inline;
+
+  [data-component='icon'] {
+    display: var(--UnderlineNav_icons-display);
+  }
+
+  /* Even on browsers that support scroll-driven animations, we still need to
+  force icons to remain hidden with JS to avoid flickering as they cause/remove
+  overflow. Once they are hidden once, they remain hidden for the life of the
+  component. */
+  &[data-hide-icons='true'] {
+    animation: none;
+    animation-timeline: none;
 
-/* More button styles migrated from styles.ts (was moreBtnStyles) */
-.MoreButton {
-  margin: 0; /* reset Safari extra margin */
-  border: 0;
-  background: transparent;
-  font-weight: var(--base-text-weight-normal);
-  box-shadow: none;
-  padding-top: var(--base-size-4);
-  padding-bottom: var(--base-size-4);
-  padding-left: var(--base-size-8);
-  padding-right: var(--base-size-8);
+    --UnderlineNav_icons-display: none;
+  }
+}
 
-  & > [data-component='trailingVisual'] {
-    margin-left: 0;
+@keyframes detect-overflow {
+  0%,
+  100% {
+    --UnderlineNav_icons-display: none;
   }
 }

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

@@ -1,6 +1,6 @@
 import {describe, expect, it, vi} from 'vitest'
 import type React from 'react'
-import {render, screen} from '@testing-library/react'
+import {render, screen, within} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import {
   CodeIcon,
@@ -16,6 +16,7 @@ import {UnderlineNav} from '.'
 import {implementsClassName} from '../utils/testing'
 import classes from '../internal/components/UnderlineTabbedInterface.module.css'
 import {clsx} from 'clsx'
+import {page} from 'vitest/browser'
 
 const ResponsiveUnderlineNav = ({
   selectedItemText = 'Code',
@@ -78,7 +79,8 @@ describe('UnderlineNav', () => {
   it('renders icons correctly', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav />)
     const nav = getByRole('navigation')
-    expect(nav.getElementsByTagName('svg').length).toEqual(7)
+    const list = within(nav).getByRole('list')
+    expect(list.getElementsByTagName('svg').length).toEqual(7)
   })
 
   it('fires onSelect on click', async () => {
@@ -141,9 +143,10 @@ describe('UnderlineNav', () => {
     expect(counter.textContent).toBe('\u00A0(120)')
   })
 
-  it('respects loadingCounters prop', () => {
+  it('respects loadingCounters prop', async () => {
+    await page.viewport(1000, 500)
     const {getByRole} = render(<ResponsiveUnderlineNav loadingCounters={true} />)
-    const item = getByRole('link', {name: 'Actions'})
+    const item = getByRole('link', {name: 'Actions', hidden: true})
     const loadingCounter = item.getElementsByTagName('span')[2]
     expect(loadingCounter.className).toContain('LoadingCounter')
     expect(loadingCounter.textContent).toBe('')