From 16795e3bdbd5f00f99c6c6ec108164763393873e Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Fri, 23 Aug 2024 16:45:09 -0400 Subject: [PATCH 1/5] Adjust `ProgressBar.Item` for accessibility --- .../react/src/ProgressBar/ProgressBar.tsx | 62 +++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index 409f8ff58dc..5e0ae07c8c2 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -14,7 +14,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: ${get('colors.success.emphasis')}; @@ -31,8 +31,6 @@ export const Item = styled.span` ${sx}; ` -Item.displayName = 'ProgressBar.Item' - const sizeMap = { small: '5px', large: '10px', @@ -57,21 +55,12 @@ const ProgressContainer = styled.span` ${sx}; ` -export type ProgressBarProps = React.HTMLAttributes & {bg?: string} & StyledProgressContainerProps & - ProgressProp - -export const ProgressBar = forwardRef( - ( - {animated, progress, bg = 'success.emphasis', barSize = 'default', children, ...rest}: ProgressBarProps, - forwardRef, - ) => { - if (children && progress) { - throw new Error('You should pass `progress` or children, not both.') - } +export type ProgressBarItems = React.HTMLAttributes & {'aria-label'?: string} & ProgressProp & SxProp +export const Item = forwardRef( + ({progress, 'aria-label': ariaLabel, ...rest}, forwardRef) => { warning( - children && - typeof (rest as React.AriaAttributes)['aria-valuenow'] === 'undefined' && + 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.', ) @@ -85,8 +74,45 @@ export const ProgressBar = forwardRef( } return ( - - {children ?? } + + ) + }, +) + +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, + 'aria-label': ariaLabel, + ...rest + }: ProgressBarProps, + forwardRef, + ) => { + if (children && progress) { + throw new Error('You should pass `progress` or children, not both.') + } + + return ( + + {children ?? ( + + )} ) }, From 747354d58bfbbc35f0581d0ef0bdfcec444eaf7b Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 26 Aug 2024 10:07:29 -0400 Subject: [PATCH 2/5] Move warning --- packages/react/src/ProgressBar/ProgressBar.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index 5e0ae07c8c2..b7ec1cb92c1 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -59,12 +59,6 @@ export type ProgressBarItems = React.HTMLAttributes & {'aria-la export const Item = forwardRef( ({progress, 'aria-label': ariaLabel, ...rest}, forwardRef) => { - warning( - 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 = { @@ -73,6 +67,13 @@ export const Item = forwardRef( 'aria-valuemax': 100, } + warning( + !ariaAttributes['aria-valuenow'] && + 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.', + ) + return ( Date: Mon, 26 Aug 2024 10:57:15 -0400 Subject: [PATCH 3/5] Include if progressAsNumber === 0 --- packages/react/src/ProgressBar/ProgressBar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index b7ec1cb92c1..1e7b5aa7dcc 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -62,7 +62,7 @@ export const Item = forwardRef( const progressAsNumber = typeof progress === 'string' ? parseInt(progress, 10) : progress const ariaAttributes = { - 'aria-valuenow': progressAsNumber ? Math.round(progressAsNumber) : undefined, + 'aria-valuenow': progressAsNumber && progressAsNumber >= 0 ? Math.round(progressAsNumber) : undefined, 'aria-valuemin': 0, 'aria-valuemax': 100, } From 3cbd244665c1ff77677589b71653cd340851e803 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 26 Aug 2024 11:45:39 -0400 Subject: [PATCH 4/5] Update snapshots, tests, move aria-* attributes --- .../react/src/ProgressBar/ProgressBar.tsx | 11 +++++- .../react/src/__tests__/ProgressBar.test.tsx | 37 ++++++++++++++++++- .../__snapshots__/ProgressBar.test.tsx.snap | 9 ++--- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index 1e7b5aa7dcc..89a1e56e8db 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -101,6 +101,8 @@ export const ProgressBar = forwardRef( barSize = 'default', children, 'aria-label': ariaLabel, + 'aria-valuenow': ariaValueNow, + 'aria-valuetext': ariaValueText, ...rest }: ProgressBarProps, forwardRef, @@ -112,7 +114,14 @@ export const ProgressBar = forwardRef( return ( {children ?? ( - + )} ) diff --git a/packages/react/src/__tests__/ProgressBar.test.tsx b/packages/react/src/__tests__/ProgressBar.test.tsx index be373f9feb2..f683d469ff5 100644 --- a/packages/react/src/__tests__/ProgressBar.test.tsx +++ b/packages/react/src/__tests__/ProgressBar.test.tsx @@ -5,7 +5,7 @@ import {render as HTMLRender} from '@testing-library/react' import axe from 'axe-core' describe('ProgressBar', () => { - behavesAsComponent({Component: ProgressBar}) + behavesAsComponent({Component: ProgressBar, toRender: () => }) checkExports('ProgressBar', { default: undefined, @@ -50,4 +50,39 @@ 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') + }) }) diff --git a/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap index df65e057b10..953bb215fa8 100644 --- a/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap +++ b/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap @@ -34,15 +34,14 @@ exports[`ProgressBar respects the "progress" prop 1`] = ` } `; From 993204c28bbdec361ee297e09dd722c36580b55d Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 29 Aug 2024 10:14:31 -0400 Subject: [PATCH 5/5] Update warning --- packages/react/src/ProgressBar/ProgressBar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index 89a1e56e8db..35131ba08da 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -68,7 +68,7 @@ export const Item = forwardRef( } warning( - !ariaAttributes['aria-valuenow'] && + ariaAttributes['aria-valuenow'] === undefined && 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.',