[UnderlineNav2]: Increase the unit test coverage (#2506)

* UnderlineNav unit tests and snapshot * use getByRole and userEvent * add changeset * remove arrow key nav tests as their support is removed in the implementation
primer · Nov 8, 2022 · a20faba · a20faba
1 parent e2a2d78
commit a20faba
Show file tree

Hide file tree

Showing 5 changed files with 800 additions and 61 deletions.
diff --git a/.changeset/breezy-cars-hear.md b/.changeset/breezy-cars-hear.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+UnderlineNav2: Prevent item width calculation when they are null
diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx
@@ -1,6 +1,7 @@
 import React from 'react'
 import '@testing-library/jest-dom/extend-expect'
-import {fireEvent, render} from '@testing-library/react'
+import {render} from '@testing-library/react'
+import userEvent from '@testing-library/user-event'
 import {
   IconProps,
   CodeIcon,
@@ -13,6 +14,7 @@ import {
 } from '@primer/octicons-react'
 
 import {UnderlineNav} from '.'
+import {behavesAsComponent, checkExports, checkStoriesForAxeViolations} from '../utils/testing'
 
 // 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
@@ -32,10 +34,12 @@ Object.defineProperty(window, 'matchMedia', {
 
 const ResponsiveUnderlineNav = ({
   selectedItemText = 'Code',
-  loadingCounters = false
+  loadingCounters = false,
+  displayExtraEl = false
 }: {
   selectedItemText?: string
   loadingCounters?: boolean
+  displayExtraEl?: boolean
 }) => {
   const items: {navigation: string; icon?: React.FC<IconProps>; counter?: number}[] = [
     {navigation: 'Code', icon: CodeIcon},
@@ -48,74 +52,125 @@ const ResponsiveUnderlineNav = ({
     {navigation: 'Settings', counter: 10},
     {navigation: 'Security', icon: ShieldLockIcon}
   ]
+
   return (
-    <UnderlineNav aria-label="Repository" loadingCounters={loadingCounters}>
-      {items.map(item => (
-        <UnderlineNav.Item
-          key={item.navigation}
-          icon={item.icon}
-          selected={item.navigation === selectedItemText}
-          counter={item.counter}
-        >
-          {item.navigation}
-        </UnderlineNav.Item>
-      ))}
-    </UnderlineNav>
+    <div>
+      <UnderlineNav aria-label="Repository" loadingCounters={loadingCounters}>
+        {items.map(item => (
+          <UnderlineNav.Item
+            key={item.navigation}
+            icon={item.icon}
+            selected={item.navigation === selectedItemText}
+            counter={item.counter}
+          >
+            {item.navigation}
+          </UnderlineNav.Item>
+        ))}
+      </UnderlineNav>
+      {displayExtraEl && <button>Custom button</button>}
+    </div>
   )
 }
 
 describe('UnderlineNav', () => {
-  it('renders aria-current attribute to be pages when an item is selected', () => {
-    const {getByText} = render(<ResponsiveUnderlineNav />)
-    const selectedNavLink = getByText('Code').closest('a')
+  behavesAsComponent({
+    Component: UnderlineNav,
+    options: {skipAs: true, skipSx: true},
+    toRender: () => <ResponsiveUnderlineNav />
+  })
 
-    expect(selectedNavLink?.getAttribute('aria-current')).toBe('page')
+  checkExports('UnderlineNav2', {
+    default: undefined,
+    UnderlineNav
+  })
+  it('renders aria-current attribute to be pages when an item is selected', () => {
+    const {getByRole} = render(<ResponsiveUnderlineNav />)
+    const selectedNavLink = getByRole('link', {name: 'Code'})
+    expect(selectedNavLink.getAttribute('aria-current')).toBe('page')
   })
   it('renders aria-label attribute correctly', () => {
-    const {container} = render(<ResponsiveUnderlineNav />)
+    const {container, getByRole} = render(<ResponsiveUnderlineNav />)
     expect(container.getElementsByTagName('nav').length).toEqual(1)
-    const nav = container.getElementsByTagName('nav')[0]
-
+    const nav = getByRole('navigation')
     expect(nav.getAttribute('aria-label')).toBe('Repository')
   })
   it('renders icons correctly', () => {
-    const {container} = render(<ResponsiveUnderlineNav />)
-    const nav = container.getElementsByTagName('nav')[0]
+    const {getByRole} = render(<ResponsiveUnderlineNav />)
+    const nav = getByRole('navigation')
     expect(nav.getElementsByTagName('svg').length).toEqual(7)
   })
-  it('fires onSelect on click and keypress', async () => {
+  it('fires onSelect on click', async () => {
     const onSelect = jest.fn()
-    const {getByText} = render(
+    const {getByRole} = render(
       <UnderlineNav aria-label="Test Navigation">
         <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)
+    const item = getByRole('link', {name: 'Item 1'})
+    const user = userEvent.setup()
+    await user.click(item)
     expect(onSelect).toHaveBeenCalledTimes(1)
-    fireEvent.keyPress(item, {key: 'Enter', code: 13, charCode: 13})
+  })
+  it('fires onSelect on keypress', async () => {
+    const onSelect = jest.fn()
+    const {getByRole} = render(
+      <UnderlineNav aria-label="Test Navigation">
+        <UnderlineNav.Item onSelect={onSelect}>Item 1</UnderlineNav.Item>
+        <UnderlineNav.Item onSelect={onSelect}>Item 2</UnderlineNav.Item>
+        <UnderlineNav.Item selected onSelect={onSelect}>
+          Item 3
+        </UnderlineNav.Item>
+      </UnderlineNav>
+    )
+    const item = getByRole('link', {name: 'Item 1'})
+    const user = userEvent.setup()
+    await user.tab() // tab into the story, this should focus on the first link
+    expect(item).toEqual(document.activeElement)
+    await user.keyboard('{Enter}')
+    // Enter keypress fires both click and keypress events
     expect(onSelect).toHaveBeenCalledTimes(2)
+    await user.keyboard(' ') // space
+    expect(onSelect).toHaveBeenCalledTimes(3)
   })
   it('respects counter prop', () => {
-    const {getByText} = render(<ResponsiveUnderlineNav />)
-    const item = getByText('Issues').closest('a')
-    const counter = item?.getElementsByTagName('span')[3]
-    expect(counter?.className).toContain('CounterLabel')
-    expect(counter?.textContent).toBe('120')
+    const {getByRole} = render(<ResponsiveUnderlineNav />)
+    const item = getByRole('link', {name: 'Issues 120'})
+    const counter = item.getElementsByTagName('span')[3]
+    expect(counter.className).toContain('CounterLabel')
+    expect(counter.textContent).toBe('120')
   })
   it('respects loadingCounters prop', () => {
-    const {getByText} = render(<ResponsiveUnderlineNav loadingCounters={true} />)
-    const item = getByText('Actions').closest('a')
-    const loadingCounter = item?.getElementsByTagName('span')[2]
-    expect(loadingCounter?.className).toContain('LoadingCounter')
-    expect(loadingCounter?.textContent).toBe('')
+    const {getByRole} = render(<ResponsiveUnderlineNav loadingCounters={true} />)
+    const item = getByRole('link', {name: 'Actions'})
+    const loadingCounter = item.getElementsByTagName('span')[2]
+    expect(loadingCounter.className).toContain('LoadingCounter')
+    expect(loadingCounter.textContent).toBe('')
   })
   it('renders a visually hidden h2 heading for screen readers when aria-label is present', () => {
-    const {container} = render(<ResponsiveUnderlineNav />)
-    const heading = container.getElementsByTagName('h2')[0]
+    const {getByRole} = render(<ResponsiveUnderlineNav />)
+    const heading = getByRole('heading', {name: 'Repository navigation'})
+    // check if heading is h2 tag
+    expect(heading.tagName).toBe('H2')
     expect(heading.className).toContain('VisuallyHidden')
     expect(heading.textContent).toBe('Repository navigation')
   })
 })
+
+describe('Keyboard Navigation', () => {
+  it('should move focus to the next/previous item on the list with the tab key', async () => {
+    const {getByRole} = render(<ResponsiveUnderlineNav />)
+    const item = getByRole('link', {name: 'Code'})
+    const nextItem = getByRole('link', {name: 'Issues 120'})
+    const user = userEvent.setup()
+    await user.tab() // tab into the story, this should focus on the first link
+    expect(item).toEqual(document.activeElement) // check if the first item is focused
+    await user.tab()
+    // focus should be on the next item
+    expect(nextItem).toHaveFocus()
+    expect(nextItem.getAttribute('tabindex')).toBe('0')
+  })
+})
+
+checkStoriesForAxeViolations('examples', '../UnderlineNav2/')
diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx
@@ -373,3 +373,5 @@ export const UnderlineNav = forwardRef(
     )
   }
 )
+
+UnderlineNav.displayName = 'UnderlineNav'
diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx
@@ -79,32 +79,34 @@ export const UnderlineNavItem = forwardRef(
     } = useContext(UnderlineNavContext)
 
     useLayoutEffect(() => {
-      const domRect = (ref as MutableRefObject<HTMLElement>).current.getBoundingClientRect()
+      if (ref.current) {
+        const domRect = (ref as MutableRefObject<HTMLElement>).current.getBoundingClientRect()
 
-      const icon = Array.from((ref as MutableRefObject<HTMLElement>).current.children[0].children).find(
-        child => child.getAttribute('data-component') === 'icon'
-      )
+        const icon = Array.from((ref as MutableRefObject<HTMLElement>).current.children[0].children).find(
+          child => child.getAttribute('data-component') === 'icon'
+        )
 
-      const content = Array.from((ref as MutableRefObject<HTMLElement>).current.children[0].children).find(
-        child => child.getAttribute('data-component') === 'text'
-      ) as HTMLElement
-      const text = content.textContent as string
+        const content = Array.from((ref as MutableRefObject<HTMLElement>).current.children[0].children).find(
+          child => child.getAttribute('data-component') === 'text'
+        ) as HTMLElement
+        const text = content.textContent as string
 
-      const iconWidthWithMargin = icon
-        ? icon.getBoundingClientRect().width +
-          Number(getComputedStyle(icon).marginRight.slice(0, -2)) +
-          Number(getComputedStyle(icon).marginLeft.slice(0, -2))
-        : 0
+        const iconWidthWithMargin = icon
+          ? icon.getBoundingClientRect().width +
+            Number(getComputedStyle(icon).marginRight.slice(0, -2)) +
+            Number(getComputedStyle(icon).marginLeft.slice(0, -2))
+          : 0
 
-      setChildrenWidth({text, width: domRect.width})
-      setNoIconChildrenWidth({text, width: domRect.width - iconWidthWithMargin})
-      preSelected && selectedLink === undefined && setSelectedLink(ref as RefObject<HTMLElement>)
+        setChildrenWidth({text, width: domRect.width})
+        setNoIconChildrenWidth({text, width: domRect.width - iconWidthWithMargin})
+        preSelected && selectedLink === undefined && setSelectedLink(ref as RefObject<HTMLElement>)
 
-      // Only runs when a menu item is selected (swapping the menu item with the list item to keep it visible)
-      if (selectedLinkText === text) {
-        setSelectedLink(ref as RefObject<HTMLElement>)
-        if (typeof onSelect === 'function' && selectEvent !== null) onSelect(selectEvent)
-        setSelectedLinkText('')
+        // Only runs when a menu item is selected (swapping the menu item with the list item to keep it visible)
+        if (selectedLinkText === text) {
+          setSelectedLink(ref as RefObject<HTMLElement>)
+          if (typeof onSelect === 'function' && selectEvent !== null) onSelect(selectEvent)
+          setSelectedLinkText('')
+        }
       }
     }, [
       ref,
@@ -185,3 +187,5 @@ export const UnderlineNavItem = forwardRef(
     )
   }
 ) as PolymorphicForwardRefComponent<'a', UnderlineNavItemProps>
+
+UnderlineNavItem.displayName = 'UnderlineNavItem'