CounterLabel: Accessibility improvements for better screen reader ann…

…ouncing (#2604)

* Accessibility improvements on CounterLabel for better screen reader announcing

* use updated CounterLabel on UnderlineNav and fix tests

* update snapshots

Co-authored-by: Josep Martins <josepmartins@github.com>

.changeset/two-timers-shout.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Accessibility improvements on CounterLabel for better screen reader announcing

src/CounterLabel.tsx

@@ -1,50 +1,48 @@
-import styled from 'styled-components'
-import {get} from './constants'
-import sx, {SxProp} from './sx'
-import {ComponentProps} from './utils/types'
+import React from 'react'
+import Box from './Box'
+import {BetterSystemStyleObject, SxProp, merge} from './sx'
+import VisuallyHidden from './_VisuallyHidden'
 
-type StyledCounterLabelProps = {
+export type CounterLabelProps = {
   scheme?: 'primary' | 'secondary'
 } & SxProp
 
-const colorStyles = ({scheme, ...props}: StyledCounterLabelProps) => {
-  return {
-    color:
-      scheme === 'secondary'
-        ? get('colors.fg.default')(props)
-        : scheme === 'primary'
-        ? get('colors.fg.onEmphasis')(props)
-        : get('colors.fg.default')(props)
-  }
+const CounterLabel: React.FC<React.PropsWithChildren<CounterLabelProps>> = ({
+  scheme = 'secondary',
+  sx = {},
+  children,
+  ...props
+}) => {
+  return (
+    <>
+      <Box
+        as="span"
+        aria-hidden="true"
+        sx={merge<BetterSystemStyleObject>(
+          {
+            display: 'inline-block',
+            padding: '2px 5px',
+            fontSize: 0,
+            fontWeight: 'bold',
+            lineHeight: 'condensedUltra',
+            borderRadius: '20px',
+            backgroundColor: scheme === 'primary' ? 'neutral.emphasis' : 'neutral.muted',
+            color: scheme === 'primary' ? 'fg.onEmphasis' : 'fg.default',
+            '&:empty': {
+              display: 'none'
+            }
+          },
+          sx
+        )}
+        {...props}
+      >
+        {children}
+      </Box>
+      <VisuallyHidden>&nbsp;{`(${children})`}</VisuallyHidden>
+    </>
+  )
 }
 
-const bgStyles = ({scheme, ...props}: StyledCounterLabelProps) => {
-  return {
-    backgroundColor:
-      scheme === 'secondary'
-        ? get('colors.neutral.muted')(props)
-        : scheme === 'primary'
-        ? get('colors.neutral.emphasis')(props)
-        : get('colors.neutral.muted')(props)
-  }
-}
-
-const CounterLabel = styled.span<StyledCounterLabelProps>`
-  display: inline-block;
-  padding: 2px 5px;
-  font-size: ${get('fontSizes.0')};
-  font-weight: ${get('fontWeights.bold')};
-  line-height: ${get('lineHeights.condensedUltra')};
-  border-radius: 20px;
-  ${colorStyles};
-  ${bgStyles};
-
-  &:empty {
-    display: none;
-  }
-
-  ${sx};
-`
+CounterLabel.displayName = 'CounterLabel'
 
-export type CounterLabelProps = ComponentProps<typeof CounterLabel>
 export default CounterLabel

src/UnderlineNav2/UnderlineNav.test.tsx

@@ -138,8 +138,15 @@ describe('UnderlineNav', () => {
     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')
+    expect(counter).toHaveAttribute('aria-hidden', 'true')
+  })
+  it('renders the content of visually hidden span properly for screen readers', () => {
+    const {getByRole} = render(<ResponsiveUnderlineNav />)
+    const item = getByRole('link', {name: 'Issues (120)'})
+    const counter = item.getElementsByTagName('span')[4]
+    // non breaking space unified code
+    expect(counter.textContent).toBe('\u00A0(120)')
   })
   it('respects loadingCounters prop', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav loadingCounters={true} />)

src/UnderlineNav2/UnderlineNav.tsx

@@ -378,8 +378,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>
+                                  <CounterLabel>{actionElementProps.counter}</CounterLabel>
                                 </Box>
                               )
                             )}

src/UnderlineNav2/UnderlineNavItem.tsx

@@ -7,7 +7,6 @@ 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 = {
@@ -184,8 +183,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>
+                  <CounterLabel>{counter}</CounterLabel>
                 </Box>
               )
             )}

src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap

@@ -132,8 +132,8 @@ exports[`UnderlineNav renders consistently 1`] = `
   font-weight: 600;
   line-height: 1;
   border-radius: 20px;
-  color: #24292f;
   background-color: rgba(175,184,193,0.2);
+  color: #24292f;
 }
 
 .c8:empty {

src/__tests__/CounterLabel.test.tsx

@@ -1,21 +1,31 @@
 import React from 'react'
 import {CounterLabel} from '..'
-import {render, behavesAsComponent, checkExports} from '../utils/testing'
-import theme from '../theme'
+import {behavesAsComponent, checkExports} from '../utils/testing'
 import {render as HTMLRender} from '@testing-library/react'
 import {axe, toHaveNoViolations} from 'jest-axe'
 
 expect.extend(toHaveNoViolations)
 
 describe('CounterLabel', () => {
-  behavesAsComponent({Component: CounterLabel})
+  behavesAsComponent({Component: CounterLabel, options: {skipAs: true, skipSx: true}})
 
   checkExports('CounterLabel', {
     default: CounterLabel
   })
 
   it('renders a <span>', () => {
-    expect(render(<CounterLabel />).type).toEqual('span')
+    const {container} = HTMLRender(<CounterLabel>1234</CounterLabel>)
+    expect(container.firstChild?.nodeName).toEqual('SPAN')
+  })
+
+  it('renders the counter correctly', () => {
+    const {container} = HTMLRender(<CounterLabel>12K</CounterLabel>)
+    expect(container.firstChild).toHaveTextContent('12K')
+  })
+
+  it('renders the visually visible span with "aria-hidden=true"', () => {
+    const {container} = HTMLRender(<CounterLabel>1234</CounterLabel>)
+    expect(container.firstChild).toHaveAttribute('aria-hidden', 'true')
   })
 
   it('should have no axe violations', async () => {
@@ -25,24 +35,19 @@ describe('CounterLabel', () => {
   })
 
   it('respects the primary "scheme" prop', () => {
-    expect(render(<CounterLabel scheme="primary" />)).toHaveStyleRule(
-      'color',
-      theme.colorSchemes.light.colors.fg?.onEmphasis.trim()
-    )
-    expect(render(<CounterLabel scheme="primary" />)).toHaveStyleRule(
-      'background-color',
-      theme.colorSchemes.light.colors.neutral?.emphasis.trim()
-    )
-  })
-
-  it('respects the secondary "scheme" prop', () => {
-    expect(render(<CounterLabel scheme="secondary" />)).toHaveStyleRule(
-      'color',
-      theme.colorSchemes.light.colors.fg?.default.trim()
-    )
-    expect(render(<CounterLabel scheme="secondary" />)).toHaveStyleRule(
-      'background-color',
-      theme.colorSchemes.light.colors.neutral?.muted
-    )
+    const {container} = HTMLRender(<CounterLabel scheme="primary">1234</CounterLabel>)
+    expect(container).toMatchSnapshot()
+  })
+
+  it('renders with secondary scheme when no "scheme" prop is provided', () => {
+    const {container} = HTMLRender(<CounterLabel>1234</CounterLabel>)
+    expect(container).toMatchSnapshot()
+  })
+
+  it('should render visually hidden span correctly for screen readers', () => {
+    const {container} = HTMLRender(<CounterLabel>1234</CounterLabel>)
+    // Second span renders as visually hidden for screen readers and the content is &nbsp;(counter).
+    // Non-breaking space is used because browsers tend to strip a standard space.
+    expect(container.children[1].textContent).toEqual('\u00a0(1234)')
   })
 })

src/__tests__/__snapshots__/CounterLabel.test.tsx.snap

@@ -1,22 +1,134 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`CounterLabel renders consistently 1`] = `
-.c0 {
+[
+  .c0 {
   display: inline-block;
   padding: 2px 5px;
   font-size: 12px;
   font-weight: 600;
   line-height: 1;
   border-radius: 20px;
-  color: #24292f;
   background-color: rgba(175,184,193,0.2);
+  color: #24292f;
 }
 
 .c0:empty {
   display: none;
 }
 
 <span
-  className="c0"
-/>
+    aria-hidden="true"
+    className="c0"
+  />,
+  .c0 {
+  position: absolute;
+  width: 1px;
+  height: 1px;
+  padding: 0;
+  margin: -1px;
+  overflow: hidden;
+  -webkit-clip: rect(0,0,0,0);
+  clip: rect(0,0,0,0);
+  white-space: nowrap;
+  border-width: 0;
+}
+
+<span
+    className="c0"
+  >
+     
+    (undefined)
+  </span>,
+]
+`;
+
+exports[`CounterLabel renders with secondary scheme when no "scheme" prop is provided 1`] = `
+.c0 {
+  display: inline-block;
+  padding: 2px 5px;
+  font-size: 12px;
+  font-weight: bold;
+  line-height: condensedUltra;
+  border-radius: 20px;
+  background-color: neutral.muted;
+  color: fg.default;
+}
+
+.c0:empty {
+  display: none;
+}
+
+.c1 {
+  position: absolute;
+  width: 1px;
+  height: 1px;
+  padding: 0;
+  margin: -1px;
+  overflow: hidden;
+  -webkit-clip: rect(0,0,0,0);
+  clip: rect(0,0,0,0);
+  white-space: nowrap;
+  border-width: 0;
+}
+
+<div>
+  <span
+    aria-hidden="true"
+    class="c0"
+  >
+    1234
+  </span>
+  <span
+    class="c1"
+  >
+     
+    (1234)
+  </span>
+</div>
+`;
+
+exports[`CounterLabel respects the primary "scheme" prop 1`] = `
+.c0 {
+  display: inline-block;
+  padding: 2px 5px;
+  font-size: 12px;
+  font-weight: bold;
+  line-height: condensedUltra;
+  border-radius: 20px;
+  background-color: neutral.emphasis;
+  color: fg.onEmphasis;
+}
+
+.c0:empty {
+  display: none;
+}
+
+.c1 {
+  position: absolute;
+  width: 1px;
+  height: 1px;
+  padding: 0;
+  margin: -1px;
+  overflow: hidden;
+  -webkit-clip: rect(0,0,0,0);
+  clip: rect(0,0,0,0);
+  white-space: nowrap;
+  border-width: 0;
+}
+
+<div>
+  <span
+    aria-hidden="true"
+    class="c0"
+  >
+    1234
+  </span>
+  <span
+    class="c1"
+  >
+     
+    (1234)
+  </span>
+</div>
 `;

src/__tests__/__snapshots__/ToggleSwitch.test.tsx.snap

@@ -85,14 +85,6 @@ exports[`renders consistently 1`] = `
   flex-direction: row;
 }
 
-.c1 {
-  font-size: 14px;
-  color: #24292f;
-  margin-left: 8px;
-  margin-right: 8px;
-  position: relative;
-}
-
 .c5 {
   position: absolute;
   width: 1px;
@@ -106,6 +98,14 @@ exports[`renders consistently 1`] = `
   border-width: 0;
 }
 
+.c1 {
+  font-size: 14px;
+  color: #24292f;
+  margin-left: 8px;
+  margin-right: 8px;
+  position: relative;
+}
+
 .c4 {
   vertical-align: middle;
   cursor: pointer;