diff --git a/.changeset/lucky-horses-kiss.md b/.changeset/lucky-horses-kiss.md new file mode 100644 index 00000000000..5233032f1e3 --- /dev/null +++ b/.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". diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index 9678af725f2..1a49d5a828a 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/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` +const ProgressItem = styled.span` 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` ${sx}; ` -Item.displayName = 'ProgressBar.Item' - const sizeMap = { small: '5px', large: '10px', @@ -60,37 +57,78 @@ const ProgressContainer = styled.span` ${sx}; ` +export type ProgressBarItems = React.HTMLAttributes & {'aria-label'?: string} & ProgressProp & SxProp + +export const Item = forwardRef( + ( + {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 ( + + ) + }, +) + +Item.displayName = 'ProgressBar.Item' + export type ProgressBarProps = React.HTMLAttributes & {bg?: string} & StyledProgressContainerProps & ProgressProp export const ProgressBar = forwardRef( ( - {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 . 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 ( - - {children ?? } + + {validChildren ? ( + children + ) : ( + + )} ) }, diff --git a/packages/react/src/__tests__/ProgressBar.test.tsx b/packages/react/src/__tests__/ProgressBar.test.tsx index 498154a56ea..ead938b3e18 100644 --- a/packages/react/src/__tests__/ProgressBar.test.tsx +++ b/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: () => }) checkExports('ProgressBar', { default: undefined, @@ -72,4 +72,50 @@ describe('ProgressBar', () => { it('respects the "progress" prop', () => { expect(render()).toMatchSnapshot() }) + + it('passed the `aria-label` down to the progress bar', () => { + const {getByRole, getByLabelText} = HTMLRender() + 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() + expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '80') + }) + + it('passed the `aria-valuetext` down to the progress bar', () => { + const {getByRole} = HTMLRender() + 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( + + + , + ) + expect(getByRole('progressbar')).not.toHaveAttribute('aria-label') + }) + + it('passes aria attributes to the progress bar item', () => { + const {getByRole} = HTMLRender( + + + , + ) + 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() + expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '50') + }) + + it('applies `0` as a value for `aria-valuenow`', () => { + const {getByRole} = HTMLRender() + + expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '0') + }) }) diff --git a/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap index cd0422b606b..f47852ca0bc 100644 --- a/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap +++ b/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap @@ -34,16 +34,15 @@ exports[`ProgressBar respects the "progress" prop 1`] = ` } `;