UnderlineNav2: Add aria-hidden and sr-only class support for descript…

…ive counters (#2551) * aria-hidden for vosible counters and sr-only for screen readers * add changeset * menu item counter a11y support * remove unnecessary wrapper around loading counters * update tests for counters to have accessible names * update tests Co-authored-by: Josep Martins <josepmartins@github.com>
primer · Nov 10, 2022 · 5bc5c70 · 5bc5c70
1 parent 3a4b312
commit 5bc5c70
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 9 deletions.
diff --git a/.changeset/soft-sheep-glow.md b/.changeset/soft-sheep-glow.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+UnderlineNav2: Add aria-hidden and sr-only class support for descriptive counters
diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx
@@ -136,7 +136,7 @@ describe('UnderlineNav', () => {
   })
   it('respects counter prop', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav />)
-    const item = getByRole('link', {name: 'Issues 120'})
+    const item = getByRole('link', {name: 'Issues &nbsp;(120)'})
     const counter = item.getElementsByTagName('span')[3]
     expect(counter.className).toContain('CounterLabel')
     expect(counter.textContent).toBe('120')
@@ -162,7 +162,7 @@ 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 nextItem = getByRole('link', {name: 'Issues &nbsp;(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

diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx
@@ -351,7 +351,10 @@ export const UnderlineNav = forwardRef(
                               <LoadingCounter />
                             ) : (
                               actionElementProps.counter !== undefined && (
-                                <CounterLabel>{actionElementProps.counter}</CounterLabel>
+                                <Box as="span" data-component="counter">
+                                  <CounterLabel aria-hidden="true">{actionElementProps.counter}</CounterLabel>
+                                  <VisuallyHidden>{`&nbsp;(${actionElementProps.counter})`}</VisuallyHidden>
+                                </Box>
                               )
                             )}
                           </Box>

diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx
@@ -7,6 +7,7 @@ import {UnderlineNavContext} from './UnderlineNavContext'
 import CounterLabel from '../CounterLabel'
 import {getLinkStyles, wrapperStyles, iconWrapStyles, counterStyles} from './styles'
 import {LoadingCounter} from './LoadingCounter'
+import VisuallyHidden from '../_VisuallyHidden'
 
 // adopted from React.AnchorHTMLAttributes
 type LinkProps = {
@@ -177,7 +178,8 @@ export const UnderlineNavItem = forwardRef(
             ) : (
               counter !== undefined && (
                 <Box as="span" data-component="counter" sx={counterStyles}>
-                  <CounterLabel>{counter}</CounterLabel>
+                  <CounterLabel aria-hidden="true">{counter}</CounterLabel>
+                  <VisuallyHidden>{`&nbsp;(${counter})`}</VisuallyHidden>
                 </Box>
               )
             )}

diff --git a/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap b/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap
@@ -309,10 +309,16 @@ exports[`UnderlineNav renders consistently 1`] = `
               data-component="counter"
             >
               <span
+                aria-hidden="true"
                 className="c8"
               >
                 120
               </span>
+              <span
+                className="c0"
+              >
+                &nbsp;(120)
+              </span>
             </span>
           </div>
         </a>
@@ -370,10 +376,16 @@ exports[`UnderlineNav renders consistently 1`] = `
               data-component="counter"
             >
               <span
+                aria-hidden="true"
                 className="c8"
               >
                 13
               </span>
+              <span
+                className="c0"
+              >
+                &nbsp;(13)
+              </span>
             </span>
           </div>
         </a>
@@ -431,10 +443,16 @@ exports[`UnderlineNav renders consistently 1`] = `
               data-component="counter"
             >
               <span
+                aria-hidden="true"
                 className="c8"
               >
                 5
               </span>
+              <span
+                className="c0"
+              >
+                &nbsp;(5)
+              </span>
             </span>
           </div>
         </a>
@@ -464,10 +482,16 @@ exports[`UnderlineNav renders consistently 1`] = `
               data-component="counter"
             >
               <span
+                aria-hidden="true"
                 className="c8"
               >
                 4
               </span>
+              <span
+                className="c0"
+              >
+                &nbsp;(4)
+              </span>
             </span>
           </div>
         </a>
@@ -525,10 +549,16 @@ exports[`UnderlineNav renders consistently 1`] = `
               data-component="counter"
             >
               <span
+                aria-hidden="true"
                 className="c8"
               >
                 9
               </span>
+              <span
+                className="c0"
+              >
+                &nbsp;(9)
+              </span>
             </span>
           </div>
         </a>
@@ -609,10 +639,16 @@ exports[`UnderlineNav renders consistently 1`] = `
               data-component="counter"
             >
               <span
+                aria-hidden="true"
                 className="c8"
               >
                 10
               </span>
+              <span
+                className="c0"
+              >
+                &nbsp;(10)
+              </span>
             </span>
           </div>
         </a>

diff --git a/src/UnderlineNav2/interactions.stories.tsx b/src/UnderlineNav2/interactions.stories.tsx
@@ -47,11 +47,11 @@ KeyboardNavigation.play = async ({canvasElement}: {canvasElement: HTMLElement})
   await delay(500)
   await userEvent.tab()
   await delay(500)
-  let menuItem = canvas.getByRole('link', {name: 'Settings 10'})
+  let menuItem = canvas.getByRole('link', {name: 'Settings &nbsp;(10)'})
   userEvent.click(menuItem)
 
   expect(activeElement).toHaveFocus()
-  menuItem = canvas.getByRole('link', {name: 'Settings 10'})
+  menuItem = canvas.getByRole('link', {name: 'Settings &nbsp;(10)'})
 
   expect(menuItem).toHaveAttribute('aria-current', 'page')
   const lastListItem = canvas.getByRole('list').children[5]
@@ -85,11 +85,11 @@ SelectAMenuItem.play = async ({canvasElement}: {canvasElement: HTMLElement}) =>
   userEvent.click(moreBtn)
 
   await delay(1000)
-  let menuItem = canvas.getByRole('link', {name: 'Settings 10'})
+  let menuItem = canvas.getByRole('link', {name: 'Settings &nbsp;(10)'})
   userEvent.click(menuItem)
 
   expect(moreBtn).toHaveFocus()
-  menuItem = canvas.getByRole('link', {name: 'Settings 10'})
+  menuItem = canvas.getByRole('link', {name: 'Settings &nbsp;(10)'})
 
   expect(menuItem).toHaveAttribute('aria-current', 'page')
   const lastListItem = canvas.getByRole('list').children[5]
@@ -107,7 +107,7 @@ KeepSelectedItemVisible.play = async ({canvasElement}: {canvasElement: HTMLEleme
   const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms))
   const canvas = within(canvasElement)
   // await delay(2000)
-  const selectedItem = canvas.getByRole('link', {name: 'Settings 10'})
+  const selectedItem = canvas.getByRole('link', {name: 'Settings &nbsp;(10)'})
   expect(selectedItem).toHaveAttribute('aria-current', 'page')
   // change viewport
   canvasElement.style.width = '900px'