ProgressBar: Adjust ProgressBar.Item for accessibility

.changeset/lucky-horses-kiss.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+Move `aria-*` attributes to `ProgressBar.Item` and marks `ProgressBar.Item` as `role="progressbar".

packages/react/src/ProgressBar/ProgressBar.tsx

@@ -5,7 +5,6 @@ import {width} from 'styled-system'
 import {get} from '../constants'
 import type {SxProp} from '../sx'
 import sx from '../sx'
-import {warning} from '../utils/warning'
 
 type ProgressProp = {
  progress?: string | number
@@ -17,7 +16,7 @@ const shimmer = keyframes`
  to { mask-position: 0%; }
 `
 
-export const Item = styled.span<ProgressProp & SxProp>`
+const ProgressItem = styled.span<ProgressProp & SxProp>`
  width: ${props => (props.progress ? `${props.progress}%` : 0)};
  background-color: ${props => get(`colors.${props.bg || 'success.emphasis'}`)};
 
@@ -34,8 +33,6 @@ export const Item = styled.span<ProgressProp & SxProp>`
  ${sx};
 `
 
-Item.displayName = 'ProgressBar.Item'
-
 const sizeMap = {
  small: '5px',
  large: '10px',
@@ -60,37 +57,78 @@ const ProgressContainer = styled.span<StyledProgressContainerProps>`
  ${sx};
 `
 
+export type ProgressBarItems = React.HTMLAttributes<HTMLSpanElement> & {'aria-label'?: string} & ProgressProp & SxProp
+
+export const Item = forwardRef<HTMLSpanElement, ProgressBarItems>(
+ (
+ {progress, 'aria-label': ariaLabel, 'aria-valuenow': ariaValueNow, 'aria-valuetext': ariaValueText, ...rest},
+ forwardRef,
+ ) => {
+ const progressAsNumber = typeof progress === 'string' ? parseInt(progress, 10) : progress
+
+ const ariaAttributes = {
+ 'aria-valuenow':
+ ariaValueNow ?? (progressAsNumber !== undefined && progressAsNumber >= 0 ? Math.round(progressAsNumber) : 0),
+ 'aria-valuemin': 0,
+ 'aria-valuemax': 100,
+ 'aria-valuetext': ariaValueText,
+ }
+
+ return (
+ <ProgressItem
+ {...rest}
+ role="progressbar"
+ aria-label={ariaLabel}
+ ref={forwardRef}
+ progress={progress}
+ {...ariaAttributes}
+ />
+ )
+ },
+)
+
+Item.displayName = 'ProgressBar.Item'
+
 export type ProgressBarProps = React.HTMLAttributes<HTMLSpanElement> & {bg?: string} & StyledProgressContainerProps &
  ProgressProp
 
 export const ProgressBar = forwardRef<HTMLSpanElement, ProgressBarProps>(
  (
- {animated, progress, bg = 'success.emphasis', barSize = 'default', children, ...rest}: ProgressBarProps,
+ {
+ animated,
+ progress,
+ bg = 'success.emphasis',
+ barSize = 'default',
+ children,
+ 'aria-label': ariaLabel,
+ 'aria-valuenow': ariaValueNow,
+ 'aria-valuetext': ariaValueText,
+ ...rest
+ }: ProgressBarProps,
  forwardRef,
  ) => {
  if (children && progress) {
  throw new Error('You should pass `progress` or children, not both.')
  }
 
- warning(
- children &&
- typeof (rest as React.AriaAttributes)['aria-valuenow'] === 'undefined' &&
- typeof (rest as React.AriaAttributes)['aria-valuetext'] === 'undefined',
- 'Expected `aria-valuenow` or `aria-valuetext` to be provided to <ProgressBar>. Provide one of these values so screen reader users can determine the current progress. This warning will become an error in the next major release.',
- )
-
- const progressAsNumber = typeof progress === 'string' ? parseInt(progress, 10) : progress
-
- const ariaAttributes = {
- 'aria-valuenow': progressAsNumber ? Math.round(progressAsNumber) : undefined,
- 'aria-valuemin': 0,
- 'aria-valuemax': 100,
- 'aria-valuetext': progressAsNumber ? `${Math.round(progressAsNumber)}%` : undefined,
- }
+ // Get the number of non-empty nodes passed as children, this will exclude
+ // booleans, null, and undefined
+ const validChildren = React.Children.toArray(children).length
 
  return (
- <ProgressContainer ref={forwardRef} role="progressbar" barSize={barSize} {...ariaAttributes} {...rest}>
- {children ?? <Item data-animated={animated} progress={progress} bg={bg} />}
+ <ProgressContainer ref={forwardRef} barSize={barSize} {...rest}>
+ {validChildren ? (
+ children
+ ) : (
+ <Item
+ data-animated={animated}
+ progress={progress}
+ aria-label={ariaLabel}
+ aria-valuenow={ariaValueNow}
+ aria-valuetext={ariaValueText}
+ bg={bg}
+ />
+ )}
  </ProgressContainer>
  )
  },

packages/react/src/__tests__/ProgressBar.test.tsx

@@ -6,7 +6,7 @@ import axe from 'axe-core'
 import {FeatureFlags} from '../FeatureFlags'
 
 describe('ProgressBar', () => {
- behavesAsComponent({Component: ProgressBar})
+ behavesAsComponent({Component: ProgressBar, toRender: () => <ProgressBar aria-valuenow={10} progress={0} />})
 
  checkExports('ProgressBar', {
  default: undefined,
@@ -72,4 +72,50 @@ describe('ProgressBar', () => {
  it('respects the "progress" prop', () => {
  expect(render(<ProgressBar progress={80} aria-label="Upload test.png" />)).toMatchSnapshot()
  })
+
+ it('passed the `aria-label` down to the progress bar', () => {
+ const {getByRole, getByLabelText} = HTMLRender(<ProgressBar progress={80} aria-label="Upload test.png" />)
+ expect(getByRole('progressbar')).toHaveAttribute('aria-label', 'Upload test.png')
+ expect(getByLabelText('Upload test.png')).toBeInTheDocument()
+ })
+
+ it('passed the `aria-valuenow` down to the progress bar', () => {
+ const {getByRole} = HTMLRender(<ProgressBar progress={80} aria-valuenow={80} />)
+ expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '80')
+ })
+
+ it('passed the `aria-valuetext` down to the progress bar', () => {
+ const {getByRole} = HTMLRender(<ProgressBar aria-valuetext="80 percent" />)
+ expect(getByRole('progressbar')).toHaveAttribute('aria-valuetext', '80 percent')
+ })
+
+ it('does not pass the `aria-label` down to the progress bar if there are multiple items', () => {
+ const {getByRole} = HTMLRender(
+ <ProgressBar aria-label="Upload test.png">
+ <ProgressBar.Item progress={80} />
+ </ProgressBar>,
+ )
+ expect(getByRole('progressbar')).not.toHaveAttribute('aria-label')
+ })
+
+ it('passes aria attributes to the progress bar item', () => {
+ const {getByRole} = HTMLRender(
+ <ProgressBar>
+ <ProgressBar.Item progress={50} aria-label="Progress" ria-valuenow="50"></ProgressBar.Item>
+ </ProgressBar>,
+ )
+ expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '50')
+ expect(getByRole('progressbar')).toHaveAttribute('aria-label', 'Progress')
+ })
+
+ it('provides `aria-valuenow` to the progress bar item if it is not already provided', () => {
+ const {getByRole} = HTMLRender(<ProgressBar progress={50} />)
+ expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '50')
+ })
+
+ it('applies `0` as a value for `aria-valuenow`', () => {
+ const {getByRole} = HTMLRender(<ProgressBar progress={0} aria-valuenow={0} aria-label="Upload text.png" />)
+
+ expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '0')
+ })
 })

packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap

@@ -34,16 +34,15 @@ exports[`ProgressBar respects the "progress" prop 1`] = `
 }
 
 <span
- aria-label="Upload test.png"
- aria-valuemax={100}
- aria-valuemin={0}
- aria-valuenow={80}
- aria-valuetext="80%"
  className="c0"
- role="progressbar"
 >
  <span
+ aria-label="Upload test.png"
+ aria-valuemax={100}
+ aria-valuemin={0}
+ aria-valuenow={80}
  className="c1"
+ role="progressbar"
  />
 </span>
 `;