Skip to content

Commit

Permalink
warn
Browse files Browse the repository at this point in the history
  • Loading branch information
kendallgassner committed Jan 22, 2025
1 parent b641dfb commit 55951e0
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 24 deletions.
4 changes: 2 additions & 2 deletions packages/react/src/ProgressBar/ProgressBar.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, {useEffect} from 'react'
import type {Meta} from '@storybook/react'
import {ProgressBar} from '..'
import type {ProgressBarBaseProps} from './ProgressBar'
import type {ProgressBarProps} from './ProgressBar'

const sectionColorsDefault = [
'success.emphasis',
Expand All @@ -19,7 +19,7 @@ export default {

export const Default = () => <ProgressBar aria-label="Upload test.png" />

export const Playground = ({sections, ...args}: ProgressBarBaseProps & {sections: number}) => {
export const Playground = ({sections, ...args}: ProgressBarProps & {sections: number}) => {
const [sectionColors, setSectionColors] = React.useState(sectionColorsDefault)

useEffect(() => {
Expand Down
49 changes: 27 additions & 22 deletions packages/react/src/ProgressBar/ProgressBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ const ProgressContainer = toggleStyledComponent(
)

export type ProgressBarItems = React.HTMLAttributes<HTMLSpanElement> & {
'aria-label': string
className?: string
} & ProgressProp &
SxProp
Expand All @@ -83,6 +82,7 @@ export const Item = forwardRef<HTMLSpanElement, ProgressBarItems>(
{
progress,
'aria-label': ariaLabel,
'aria-hidden': ariaHidden,
'aria-valuenow': ariaValueNow,
'aria-valuetext': ariaValueText,
className,
Expand Down Expand Up @@ -110,6 +110,24 @@ export const Item = forwardRef<HTMLSpanElement, ProgressBarItems>(
styles[progressBarWidth] = progress ? `${progress}%` : '0%'
styles[progressBarBg] = (bgType && `var(--bgColor-${bgType[0]}-${bgType[1]})`) || 'var(--bgColor-success-emphasis)'

if (__DEV__) {
/**
* The Linter yells because it thinks this conditionally calls an effect,
* but since this is a compile-time flag and not a runtime conditional
* this is safe, and ensures the entire effect is kept out of prod builds
* shaving precious bytes from the output, and avoiding mounting a noop effect
*/
// eslint-disable-next-line react-hooks/rules-of-hooks
React.useEffect(() => {
if (!ariaHidden && !ariaLabel) {
// eslint-disable-next-line no-console
console.warn(
'This component should include an aria-label or should be aria-hidden if surrounding text can be used to perceive the progress.',
)
}
}, [ariaHidden, ariaLabel])
}

return (
<ProgressItem
className={clsx(className, {[classes.ProgressBarItem]: enabled})}
Expand All @@ -127,26 +145,11 @@ export const Item = forwardRef<HTMLSpanElement, ProgressBarItems>(

Item.displayName = 'ProgressBar.Item'

export type ProgressBarBaseProps = Omit<
React.HTMLAttributes<HTMLSpanElement> & {
bg?: string
className?: string
} & StyledProgressContainerProps &
ProgressProp,
'children' | 'aria-label'
>

export type WithChildren = {
children: React.ReactNode
'aria-label'?: never
}

export type WithoutChildren = {
children?: never
'aria-label': string
}

export type ProgressBarProps = ProgressBarBaseProps & (WithChildren | WithoutChildren)
export type ProgressBarProps = React.HTMLAttributes<HTMLSpanElement> & {
bg?: string
className?: string
} & StyledProgressContainerProps &
ProgressProp

export const ProgressBar = forwardRef<HTMLSpanElement, ProgressBarProps>((props, forwardRef) => {
const {
Expand All @@ -158,6 +161,7 @@ export const ProgressBar = forwardRef<HTMLSpanElement, ProgressBarProps>((props,
'aria-label': ariaLabel,
'aria-valuenow': ariaValueNow,
'aria-valuetext': ariaValueText,
'aria-hidden': ariaHidden,
className,
...rest
} = props
Expand Down Expand Up @@ -188,9 +192,10 @@ export const ProgressBar = forwardRef<HTMLSpanElement, ProgressBarProps>((props,
<Item
data-animated={animated}
progress={progress}
aria-label={ariaLabel as string}
aria-label={ariaHidden ? undefined : ariaLabel}
aria-valuenow={ariaValueNow}
aria-valuetext={ariaValueText}
aria-hidden={ariaHidden}
bg={bg}
/>
)}
Expand Down
25 changes: 25 additions & 0 deletions packages/react/src/__tests__/ProgressBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ import axe from 'axe-core'
import {FeatureFlags} from '../FeatureFlags'

describe('ProgressBar', () => {
const mockWarningFn = jest.fn()

beforeEach(() => {
jest.spyOn(global.console, 'warn').mockImplementation(mockWarningFn)
})

afterEach(() => {
jest.clearAllMocks()
})

behavesAsComponent({
Component: ProgressBar,
toRender: () => <ProgressBar aria-label="Upload test.png" aria-valuenow={10} progress={0} />,
Expand Down Expand Up @@ -112,4 +122,19 @@ describe('ProgressBar', () => {

expect(getByRole('progressbar')).toHaveAttribute('aria-valuenow', '0')
})

it('should warn users if aria-label is not provided', () => {
HTMLRender(<ProgressBar.Item progress={50} />)
expect(mockWarningFn).toHaveBeenCalled()
})

it('should not warn users if aria-label is not provided but aria-hidden is', () => {
HTMLRender(<ProgressBar.Item progress={50} aria-hidden={true} />)
expect(mockWarningFn).not.toHaveBeenCalled()
})

it('should not warn users if aria-label is provided', () => {
HTMLRender(<ProgressBar.Item progress={50} aria-label="Uploading test.png" />)
expect(mockWarningFn).not.toHaveBeenCalled()
})
})

0 comments on commit 55951e0

Please sign in to comment.