UnderlineNav overflow behaviour implementation (#2297)

* initial impl for overflow behaviour

* scroll behaviour initial commit - wip

* overflow and scroll behaviour and styles

* introduce scroll arrow buttons

* scroll behaviour in dept

* revert back stories to original state

* Fix types and align/variant issues

* Use separate component for arrow buttons

* fade out affects on scroll ends

* type issues and some refactor

* style refactor & theming & scrollIntoView from primer behaviour

* Update documentation

* Update documentation

* add changeset

* remove scroll story until finding a way to simulate coarse pointer

* add single variant selection to ActionList

* take more button into account when calculating & code review feedback

* remove scroll behaviour - due to accessibility concerns

* hover state remove on mobile and improvments on more btn functionality

* remove prop controls and align story

* update tests and docs

* update docs

* overflow effect improvments

* Revert "remove scroll behaviour - due to accessibility concerns"

This reverts commit eade73d.

* scroll behaviour feedback

* update tests

* keyboard navigation tab through

* Update .changeset/sweet-eggs-complain.md

Co-authored-by: Cole Bemis <[email protected]>

* Update docs/content/drafts/UnderlineNav2.mdx

Co-authored-by: Pavithra Kodmad <[email protected]>
Co-authored-by: Cole Bemis <[email protected]>

Author: 3 people

.changeset/sweet-eggs-complain.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+UnderlineNav2: Introducing overflow behavior for fine and coarse pointer devices

docs/content/drafts/UnderlineNav2.mdx

@@ -1,15 +1,22 @@
 ---
 title: UnderlineNav v2
+componentId: underline_nav_2
 status: Draft
 description: Use an underlined nav to allow tab like navigation with overflow behaviour in your UI.
+source: https://github.com/primer/react/tree/main/src/UnderlineNav2
+storybook: https://primer.style/react/storybook/?path=/story/layout-underlinenav
 ---
 
+```js
+import {UnderlineNav} from '@primer/react/drafts'
+```
+
 ## Examples
 
 ### Simple
 
 ```jsx live drafts
-<UnderlineNav label="simple nav">
+<UnderlineNav>
   <UnderlineNav.Item selected>Item 1</UnderlineNav.Item>
   <UnderlineNav.Item>Item 2</UnderlineNav.Item>
   <UnderlineNav.Item>Item 3</UnderlineNav.Item>
@@ -19,20 +26,88 @@ description: Use an underlined nav to allow tab like navigation with overflow be
 ### With icons
 
 ```jsx live drafts
-<UnderlineNav label="simple nav with icons">
-  <UnderlineNav.Item selected leadingIcon={EyeIcon}>
-    Item 1
+<UnderlineNav>
+  <UnderlineNav.Item selected icon={CodeIcon}>
+    Code
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={IssueOpenedIcon} counter={30}>
+    Issues
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={GitPullRequestIcon} counter={3}>
+    Pull requests
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={CommentDiscussionIcon}>Discussions</UnderlineNav.Item>
+  <UnderlineNav.Item icon={EyeIcon} counter={9}>
+    Actions
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={EyeIcon} counter={7}>
+    Projects
   </UnderlineNav.Item>
-  <UnderlineNav.Item>Item 2</UnderlineNav.Item>
 </UnderlineNav>
 ```
 
-### Small variant
+### Overflow Behaviour
+
+When overflow occurs, the component first hides icons if present to optimize for space and show as many items as possible. (Only for fine pointer devices)
+
+#### Items without Icons
 
 ```jsx live drafts
-<UnderlineNav label="small variant" variant="small">
-  <UnderlineNav.Item selected>Item 1</UnderlineNav.Item>
-  <UnderlineNav.Item>Item 2</UnderlineNav.Item>
+<UnderlineNav>
+  <UnderlineNav.Item selected icon={CodeIcon}>
+    Code
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={IssueOpenedIcon} counter={30}>
+    Issues
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={GitPullRequestIcon} counter={3}>
+    Pull requests
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={CommentDiscussionIcon}>Discussions</UnderlineNav.Item>
+  <UnderlineNav.Item icon={PlayIcon} counter={9}>
+    Actions
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={ProjectIcon} counter={7}>
+    Projects
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={ShieldLockIcon}>Security</UnderlineNav.Item>
+  <UnderlineNav.Item icon={GraphIcon}>Insights</UnderlineNav.Item>
+  <UnderlineNav.Item icon={GearIcon} counter={1}>
+    Settings
+  </UnderlineNav.Item>
+</UnderlineNav>
+```
+
+#### Display `More` menu
+
+If there is still overflow, the component will behave depending on the pointer.
+
+```jsx live drafts
+<UnderlineNav>
+  <UnderlineNav.Item selected icon={CodeIcon}>
+    Code
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={IssueOpenedIcon} counter={30}>
+    Issues
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={GitPullRequestIcon} counter={3}>
+    Pull requests
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={CommentDiscussionIcon}>Discussions</UnderlineNav.Item>
+  <UnderlineNav.Item icon={EyeIcon} counter={9}>
+    Actions
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={EyeIcon} counter={7}>
+    Projects
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={EyeIcon}>Security</UnderlineNav.Item>
+  <UnderlineNav.Item icon={EyeIcon} counter={14}>
+    Insights
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={EyeIcon} counter={1}>
+    Settings
+  </UnderlineNav.Item>
+  <UnderlineNav.Item icon={EyeIcon}>Wiki</UnderlineNav.Item>
 </UnderlineNav>
 ```
 
@@ -44,19 +119,6 @@ description: Use an underlined nav to allow tab like navigation with overflow be
   <PropsTableRow name="aria-label" type="string" />
   <PropsTableRow name="aria-labelledby" type="string" />
   <PropsTableRow name="aria-describedby" type="string" />
-  <PropsTableRow
-    name="overflow"
-    type="'auto' | 'menu' | 'scroll'"
-    defaultValue="auto"
-    description="Controls the type of overflow behaviour in smaller screens"
-  />
-  <PropsTableRow name="align" type="right | left" defaultValue="left" description="The alignment of the nav links" />
-  <PropsTableRow
-    name="variant"
-    type="default | small"
-    defaultValue="default"
-    description="The alignment of the nav links"
-  />
   <PropsTableRow
     name="afterSelect"
     type="(event) => void"
@@ -68,7 +130,7 @@ description: Use an underlined nav to allow tab like navigation with overflow be
 ### UnderlineNav.Item
 
 <PropsTable>
-  <PropsTableRow name="leadingIcon" type="Component" description="The leading icon comes before item label" />
+  <PropsTableRow name="icon" type="Component" description="The leading icon comes before item label" />
   <PropsTableRow name="selected" type="boolean" description="Whether the link is selected" />
   <PropsTableRow
     name="onSelect"
@@ -89,9 +151,9 @@ description: Use an underlined nav to allow tab like navigation with overflow be
 <ComponentChecklist
   items={{
     propsDocumented: true,
-    noUnnecessaryDeps: false,
-    adaptsToThemes: false,
-    adaptsToScreenSizes: false,
+    noUnnecessaryDeps: true,
+    adaptsToThemes: true,
+    adaptsToScreenSizes: true,
     fullTestCoverage: false,
     usedInProduction: false,
     usageExamplesDocumented: false,

src/UnderlineNav2/UnderlineNav.test.tsx

@@ -1,9 +1,30 @@
 import React from 'react'
 import '@testing-library/jest-dom/extend-expect'
-import {render} from '@testing-library/react'
+import {fireEvent, render} from '@testing-library/react'
+import {CodeIcon, EyeIcon} from '@primer/octicons-react'
 
 import {UnderlineNav} from '.'
 
+// window.matchMedia() is not implemented by JSDOM so we have to create a mock:
+// https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom
+Object.defineProperty(window, 'matchMedia', {
+  writable: true,
+  value: jest.fn().mockImplementation(query => ({
+    matches: false,
+    media: query,
+    onchange: null,
+    addListener: jest.fn(), // deprecated
+    removeListener: jest.fn(), // deprecated
+    addEventListener: jest.fn(),
+    removeEventListener: jest.fn(),
+    dispatchEvent: jest.fn()
+  }))
+})
+
+Object.defineProperty(window.Element.prototype, 'scrollTo', {
+  value: jest.fn(),
+  writable: true
+})
 describe('UnderlineNav', () => {
   test('selected nav', () => {
     const {getByText} = render(
@@ -30,15 +51,47 @@ describe('UnderlineNav', () => {
 
     expect(nav.getAttribute('aria-label')).toBe('Test nav')
   })
-  test('respect align prop', () => {
+  test('with icons', () => {
     const {container} = render(
+      <UnderlineNav label="Test nav">
+        <UnderlineNav.Item icon={CodeIcon}>Code</UnderlineNav.Item>
+        <UnderlineNav.Item icon={EyeIcon} counter={6}>
+          Issues
+        </UnderlineNav.Item>
+        <UnderlineNav.Item>Pull Request</UnderlineNav.Item>
+      </UnderlineNav>
+    )
+    const nav = container.getElementsByTagName('nav')[0]
+    expect(nav.getElementsByTagName('svg').length).toEqual(2)
+  })
+  test('should fire onSelect on click and keypress', async () => {
+    const onSelect = jest.fn()
+    const {getByText} = render(
+      <UnderlineNav label="Test nav">
+        <UnderlineNav.Item onSelect={onSelect}>Item 1</UnderlineNav.Item>
+        <UnderlineNav.Item onSelect={onSelect}>Item 2</UnderlineNav.Item>
+        <UnderlineNav.Item onSelect={onSelect}>Item 3</UnderlineNav.Item>
+      </UnderlineNav>
+    )
+    const item = getByText('Item 1')
+    fireEvent.click(item)
+    expect(onSelect).toHaveBeenCalledTimes(1)
+    fireEvent.keyPress(item, {key: 'Enter', code: 13, charCode: 13})
+    expect(onSelect).toHaveBeenCalledTimes(2)
+  })
+  test('respect counter prop', () => {
+    const {getByText} = render(
       <UnderlineNav label="Test nav" align="right">
-        <UnderlineNav.Item selected>Item 1</UnderlineNav.Item>
+        <UnderlineNav.Item counter={8} selected>
+          Item 1
+        </UnderlineNav.Item>
         <UnderlineNav.Item>Item 2</UnderlineNav.Item>
         <UnderlineNav.Item>Item 3</UnderlineNav.Item>
       </UnderlineNav>
     )
-    const nav = container.getElementsByTagName('nav')[0]
-    expect(nav).toHaveStyle(`justify-content:flex-end`)
+    const item = getByText('Item 1').closest('a')
+    const counter = item?.getElementsByTagName('span')[2]
+    expect(counter?.className).toContain('CounterLabel')
+    expect(counter?.textContent).toBe('8')
   })
 })