UnderlineNav2: Accessibility remediations (Take non-breaking space ou…

…t of JS template literals) (#2603)
primer · Nov 22, 2022 · 22059e9 · 22059e9
1 parent d9c7290
commit 22059e9
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 16 deletions.
diff --git a/.changeset/rotten-games-draw.md b/.changeset/rotten-games-draw.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+UnderlineNav2: Render non-breaking space properly for screen readers
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 &nbsp;(120)'})
+    const item = getByRole('link', {name: 'Issues (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 &nbsp;(120)'})
+    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

diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx
@@ -319,7 +319,7 @@ export const UnderlineNav = forwardRef(
                   trailingIcon={TriangleDownIcon}
                 >
                   <Box as="span">
-                    More <VisuallyHidden as="span">{`${ariaLabel} items`}</VisuallyHidden>
+                    More<VisuallyHidden as="span">&nbsp;{`${ariaLabel} items`}</VisuallyHidden>
                   </Box>
                 </Button>
                 <ActionList
@@ -353,7 +353,7 @@ export const UnderlineNav = forwardRef(
                               actionElementProps.counter !== undefined && (
                                 <Box as="span" data-component="counter">
                                   <CounterLabel aria-hidden="true">{actionElementProps.counter}</CounterLabel>
-                                  <VisuallyHidden>{`&nbsp;(${actionElementProps.counter})`}</VisuallyHidden>
+                                  <VisuallyHidden>&nbsp;{`(${actionElementProps.counter})`}</VisuallyHidden>
                                 </Box>
                               )
                             )}

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

diff --git a/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap b/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap
@@ -317,7 +317,8 @@ exports[`UnderlineNav renders consistently 1`] = `
               <span
                 className="c0"
               >
-                &nbsp;(120)
+                 
+                (120)
               </span>
             </span>
           </div>
@@ -384,7 +385,8 @@ exports[`UnderlineNav renders consistently 1`] = `
               <span
                 className="c0"
               >
-                &nbsp;(13)
+                 
+                (13)
               </span>
             </span>
           </div>
@@ -451,7 +453,8 @@ exports[`UnderlineNav renders consistently 1`] = `
               <span
                 className="c0"
               >
-                &nbsp;(5)
+                 
+                (5)
               </span>
             </span>
           </div>
@@ -490,7 +493,8 @@ exports[`UnderlineNav renders consistently 1`] = `
               <span
                 className="c0"
               >
-                &nbsp;(4)
+                 
+                (4)
               </span>
             </span>
           </div>
@@ -557,7 +561,8 @@ exports[`UnderlineNav renders consistently 1`] = `
               <span
                 className="c0"
               >
-                &nbsp;(9)
+                 
+                (9)
               </span>
             </span>
           </div>
@@ -647,7 +652,8 @@ exports[`UnderlineNav renders consistently 1`] = `
               <span
                 className="c0"
               >
-                &nbsp;(10)
+                 
+                (10)
               </span>
             </span>
           </div>

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 &nbsp;(10)'})
+  let menuItem = canvas.getByRole('link', {name: 'Settings (10)'})
   userEvent.click(menuItem)
 
   expect(activeElement).toHaveFocus()
-  menuItem = canvas.getByRole('link', {name: 'Settings &nbsp;(10)'})
+  menuItem = canvas.getByRole('link', {name: 'Settings (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 &nbsp;(10)'})
+  let menuItem = canvas.getByRole('link', {name: 'Settings (10)'})
   userEvent.click(menuItem)
 
   expect(moreBtn).toHaveFocus()
-  menuItem = canvas.getByRole('link', {name: 'Settings &nbsp;(10)'})
+  menuItem = canvas.getByRole('link', {name: 'Settings (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 &nbsp;(10)'})
+  const selectedItem = canvas.getByRole('link', {name: 'Settings (10)'})
   expect(selectedItem).toHaveAttribute('aria-current', 'page')
   // change viewport
   canvasElement.style.width = '900px'