From 16795e3bdbd5f00f99c6c6ec108164763393873e Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Fri, 23 Aug 2024 16:45:09 -0400 Subject: [PATCH 01/13] 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 02/13] 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 03/13] 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 04/13] 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 05/13] 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.', From d73c56674194cdb3621d3f16a6f4cc3649eb4995 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Fri, 30 Aug 2024 20:42:48 +0000 Subject: [PATCH 06/13] Update `ProgressBar.Item` props --- packages/react/src/ProgressBar/ProgressBar.tsx | 13 ++++++++----- packages/react/src/__tests__/ProgressBar.test.tsx | 14 ++++++++++++++ .../__snapshots__/ProgressBar.test.tsx.snap | 1 + 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index 35131ba08da..593a1cb931e 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -58,19 +58,22 @@ const ProgressContainer = styled.span` export type ProgressBarItems = React.HTMLAttributes & {'aria-label'?: string} & ProgressProp & SxProp export const Item = forwardRef( - ({progress, 'aria-label': ariaLabel, ...rest}, 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': progressAsNumber && progressAsNumber >= 0 ? Math.round(progressAsNumber) : undefined, + 'aria-valuenow': + ariaValueNow || (progressAsNumber && progressAsNumber >= 0 ? Math.round(progressAsNumber) : undefined), 'aria-valuemin': 0, 'aria-valuemax': 100, + 'aria-valuetext': ariaValueText, } warning( - ariaAttributes['aria-valuenow'] === undefined && - typeof (rest as React.AriaAttributes)['aria-valuenow'] === 'undefined' && - typeof (rest as React.AriaAttributes)['aria-valuetext'] === 'undefined', + ariaAttributes['aria-valuenow'] === undefined && 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.', ) diff --git a/packages/react/src/__tests__/ProgressBar.test.tsx b/packages/react/src/__tests__/ProgressBar.test.tsx index f683d469ff5..d2eb9e6d0bd 100644 --- a/packages/react/src/__tests__/ProgressBar.test.tsx +++ b/packages/react/src/__tests__/ProgressBar.test.tsx @@ -85,4 +85,18 @@ describe('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() + expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '50') + }) + + it('should warn users if they do not pass the correct props or ARIA attributes', async () => { + const spy = jest.spyOn(console, 'warn').mockImplementationOnce(() => {}) + + render() + + expect(spy).toHaveBeenCalledTimes(1) + spy.mockRestore() + }) }) diff --git a/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap index 953bb215fa8..1ef7c197bcc 100644 --- a/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap +++ b/packages/react/src/__tests__/__snapshots__/ProgressBar.test.tsx.snap @@ -40,6 +40,7 @@ exports[`ProgressBar respects the "progress" prop 1`] = ` aria-label="Upload test.png" aria-valuemax={100} aria-valuemin={0} + aria-valuenow={80} className="c1" role="progressbar" /> From 0ca44f21c145236df102c2fba5077deaf9320ef4 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 25 Sep 2024 09:31:40 -0400 Subject: [PATCH 07/13] Account for `0` --- packages/react/src/ProgressBar/ProgressBar.tsx | 3 ++- packages/react/src/__tests__/ProgressBar.test.tsx | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index 92158e87247..4acf12d5532 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -69,7 +69,8 @@ export const Item = forwardRef( const ariaAttributes = { 'aria-valuenow': - ariaValueNow || (progressAsNumber && progressAsNumber >= 0 ? Math.round(progressAsNumber) : undefined), + ariaValueNow || + (progressAsNumber !== undefined && progressAsNumber >= 0 ? Math.round(progressAsNumber) : undefined), 'aria-valuemin': 0, 'aria-valuemax': 100, 'aria-valuetext': ariaValueText, diff --git a/packages/react/src/__tests__/ProgressBar.test.tsx b/packages/react/src/__tests__/ProgressBar.test.tsx index d2eb9e6d0bd..8ce9423f504 100644 --- a/packages/react/src/__tests__/ProgressBar.test.tsx +++ b/packages/react/src/__tests__/ProgressBar.test.tsx @@ -99,4 +99,10 @@ describe('ProgressBar', () => { expect(spy).toHaveBeenCalledTimes(1) spy.mockRestore() }) + + it('applies `0` as a value for `aria-valuenow`', () => { + const {getByRole} = HTMLRender() + + expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '0') + }) }) From c07904e4582aa52ece9a4ad628b7b4172bd950cc Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Fri, 27 Sep 2024 09:55:42 -0400 Subject: [PATCH 08/13] Add changeset --- .changeset/lucky-horses-kiss.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/lucky-horses-kiss.md 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". From b6afe92d25ad42ed6928cd81fd69f24498d7897c Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Tue, 22 Oct 2024 10:19:40 -0400 Subject: [PATCH 09/13] Change children conditional --- packages/react/src/ProgressBar/ProgressBar.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index 4acf12d5532..a9a3a79758b 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -118,9 +118,13 @@ export const ProgressBar = forwardRef( throw new Error('You should pass `progress` or children, not both.') } + const validChildren = React.Children.toArray(children).length + return ( - {children ?? ( + {validChildren ? ( + children + ) : ( Date: Tue, 22 Oct 2024 11:17:57 -0400 Subject: [PATCH 10/13] Remove warning, add default 0 --- packages/react/src/ProgressBar/ProgressBar.tsx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index a9a3a79758b..fe04d5ebbee 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -69,18 +69,12 @@ export const Item = forwardRef( const ariaAttributes = { 'aria-valuenow': - ariaValueNow || - (progressAsNumber !== undefined && progressAsNumber >= 0 ? Math.round(progressAsNumber) : undefined), + ariaValueNow || (progressAsNumber !== undefined && progressAsNumber >= 0 ? Math.round(progressAsNumber) : 0), 'aria-valuemin': 0, 'aria-valuemax': 100, 'aria-valuetext': ariaValueText, } - warning( - ariaAttributes['aria-valuenow'] === undefined && 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: Thu, 24 Oct 2024 10:08:34 -0400 Subject: [PATCH 11/13] Fix lint, test --- packages/react/src/ProgressBar/ProgressBar.tsx | 1 - packages/react/src/__tests__/ProgressBar.test.tsx | 9 --------- 2 files changed, 10 deletions(-) diff --git a/packages/react/src/ProgressBar/ProgressBar.tsx b/packages/react/src/ProgressBar/ProgressBar.tsx index fe04d5ebbee..d72c45f1877 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 diff --git a/packages/react/src/__tests__/ProgressBar.test.tsx b/packages/react/src/__tests__/ProgressBar.test.tsx index d7cefaf89a7..ead938b3e18 100644 --- a/packages/react/src/__tests__/ProgressBar.test.tsx +++ b/packages/react/src/__tests__/ProgressBar.test.tsx @@ -113,15 +113,6 @@ describe('ProgressBar', () => { expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '50') }) - it('should warn users if they do not pass the correct props or ARIA attributes', async () => { - const spy = jest.spyOn(console, 'warn').mockImplementationOnce(() => {}) - - render() - - expect(spy).toHaveBeenCalledTimes(1) - spy.mockRestore() - }) - it('applies `0` as a value for `aria-valuenow`', () => { const {getByRole} = HTMLRender() From 4823c8e9791c9d2108592f1faee6f34abe79db6c Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 24 Oct 2024 11:08:08 -0400 Subject: [PATCH 12/13] Update packages/react/src/ProgressBar/ProgressBar.tsx Co-authored-by: Josh Black --- 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 d72c45f1877..1130b1d79b2 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -76,12 +76,12 @@ export const Item = forwardRef( return ( ) }, From a2ca83b0ebf5c5094b38f3301a0bcb6bcb37ea98 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 24 Oct 2024 11:08:46 -0400 Subject: [PATCH 13/13] Update packages/react/src/ProgressBar/ProgressBar.tsx Co-authored-by: Josh Black --- 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 1130b1d79b2..c24f0364862 100644 --- a/packages/react/src/ProgressBar/ProgressBar.tsx +++ b/packages/react/src/ProgressBar/ProgressBar.tsx @@ -68,7 +68,7 @@ export const Item = forwardRef( const ariaAttributes = { 'aria-valuenow': - ariaValueNow || (progressAsNumber !== undefined && progressAsNumber >= 0 ? Math.round(progressAsNumber) : 0), + ariaValueNow ?? (progressAsNumber !== undefined && progressAsNumber >= 0 ? Math.round(progressAsNumber) : 0), 'aria-valuemin': 0, 'aria-valuemax': 100, 'aria-valuetext': ariaValueText,