Skip to content

Commit 9169658

Browse files
authored
fix(Spinner): define default aria-label via destructuring (#12420)
The Spinner default "Contents" aria-label was emitted by an inline conditional in the JSX, so the props table for the component did not show that there was a default value at all (#11750). Move the default to the destructuring pattern so react-docgen surfaces it, and replace the three conditional spread expressions with a direct attribute plus the still-conditional aria-labelledBy spread. Existing snapshots are unchanged (`<Spinner />` still renders with aria-label="Contents"). Added unit tests cover the default value, an explicitly provided aria-label, and aria-labelledBy. Closes #11750 Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
1 parent 0b41291 commit 9169658

2 files changed

Lines changed: 47 additions & 7 deletions

File tree

packages/react-core/src/components/Spinner/Spinner.tsx

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ export interface SpinnerProps extends React.SVGProps<SVGSVGElement> {
2222
isInline?: boolean;
2323
/** Accessible label to describe what is loading */
2424
'aria-label'?: string;
25-
/** Id of element which describes what is being loaded */
25+
/** ID of the element(s) that provide an accessible name for the spinner. This prop is deprecated and you should instead use the aria-labelledby prop (with a lowercase "b") instead. */
2626
'aria-labelledBy'?: string;
27+
/** ID of the element(s) that provide an accessible name for the spinner. */
28+
'aria-labelledby'?: string;
2729
}
2830

2931
export const Spinner: React.FunctionComponent<SpinnerProps> = ({
@@ -32,8 +34,9 @@ export const Spinner: React.FunctionComponent<SpinnerProps> = ({
3234
'aria-valuetext': ariaValueText = 'Loading...',
3335
diameter,
3436
isInline = false,
35-
'aria-label': ariaLabel,
36-
'aria-labelledBy': ariaLabelledBy,
37+
'aria-label': ariaLabel = 'Contents',
38+
'aria-labelledBy': deprecatedAriaLabelledBy,
39+
'aria-labelledby': ariaLabelledby,
3740
...props
3841
}: SpinnerProps) => (
3942
<svg
@@ -42,9 +45,10 @@ export const Spinner: React.FunctionComponent<SpinnerProps> = ({
4245
aria-valuetext={ariaValueText}
4346
viewBox="0 0 100 100"
4447
{...(diameter && { style: { [cssDiameter.name]: diameter } as React.CSSProperties })}
45-
{...(ariaLabel && { 'aria-label': ariaLabel })}
46-
{...(ariaLabelledBy && { 'aria-labelledBy': ariaLabelledBy })}
47-
{...(!ariaLabel && !ariaLabelledBy && { 'aria-label': 'Contents' })}
48+
aria-label={ariaLabel}
49+
{...((ariaLabelledby ?? deprecatedAriaLabelledBy) && {
50+
'aria-labelledby': ariaLabelledby ?? deprecatedAriaLabelledBy
51+
})}
4852
{...props}
4953
>
5054
<circle className={styles.spinnerPath} cx="50" cy="50" r="45" fill="none" />

packages/react-core/src/components/Spinner/__tests__/Spinner.test.tsx

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,47 @@
1-
import { render } from '@testing-library/react';
1+
import { render, screen } from '@testing-library/react';
22
import { Spinner } from '../Spinner';
33

44
test('simple spinner', () => {
55
const { asFragment } = render(<Spinner />);
66
expect(asFragment()).toMatchSnapshot();
77
});
88

9+
test('uses default aria-label of "Contents" when none is provided', () => {
10+
render(<Spinner />);
11+
expect(screen.getByRole('progressbar')).toHaveAccessibleName('Contents');
12+
});
13+
14+
test('uses a custom aria-label when one is provided', () => {
15+
render(<Spinner aria-label="Loading users" />);
16+
expect(screen.getByRole('progressbar')).toHaveAccessibleName('Loading users');
17+
});
18+
19+
test('Renders with accessible name via aria-labelledby when passed', () => {
20+
render(
21+
<>
22+
<span id="spinner-label">Loading reports</span>
23+
<Spinner aria-labelledby="spinner-label" />
24+
</>
25+
);
26+
27+
expect(screen.getByRole('progressbar')).toHaveAccessibleName('Loading reports');
28+
});
29+
30+
test('Renders deprecated aria-labelledBy as aria-labelledby', () => {
31+
render(<Spinner aria-labelledBy="external-label" />);
32+
expect(screen.getByRole('progressbar')).toHaveAttribute('aria-labelledby', 'external-label');
33+
});
34+
35+
test('Prefers aria-labelledby over the deprecated aria-labelledBy when both are passed', () => {
36+
render(<Spinner aria-labelledby="lowercase-label" aria-labelledBy="deprecated-label" />);
37+
expect(screen.getByRole('progressbar')).toHaveAttribute('aria-labelledby', 'lowercase-label');
38+
});
39+
40+
test('Renders with aria-label even when aria-labelledby is passed', () => {
41+
render(<Spinner aria-labelledby="external-label" />);
42+
expect(screen.getByRole('progressbar')).toHaveAccessibleName('Contents');
43+
});
44+
945
test('small spinner', () => {
1046
const { asFragment } = render(<Spinner size="sm" />);
1147
expect(asFragment()).toMatchSnapshot();

0 commit comments

Comments
 (0)