Skip to content

Commit 56fab43

Browse files
wise-king-sullymanwise-king-sullyman
authored
fix(Card): indicate card selectivity and status if using a screen reader (#7144)
* fix(Card): indicate card selectivity and status if using a screen reader * rework hidden input implementation to fix a11y structure issues * Add temporary styling on hidden input focus for PR demo purposes * add automatic aria label determination * replace spaces with dashes in component ids * update to only pass an id when that id isn't empty * update snapshots * refactor aria label determination to use the effect hook * fix bug causing unintended warnings to be printed to the console * add aria label to demo galleries * remove temporary demonstration styling * improve hasHiddenInput prop description * rename hiddenInput props * add tests for new a11y functionality * refactor card title registering tests to be pure unit tests * add example to better explain selectable a11y props * improve new example copy
1 parent 4257981 commit 56fab43

File tree

12 files changed

+321
-37
lines changed

12 files changed

+321
-37
lines changed

‎packages/react-catalog-view-extension/src/components/CatalogTile/__snapshots__/CatalogTile.test.tsx.snap‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ exports[`CatalogTile href renders properly 1`] = `
1313
>
1414
<div
1515
class="pf-c-card__title catalog-tile-pf-header"
16+
id="test-href-title"
1617
>
1718
<div
1819
class="catalog-tile-pf-title"
@@ -111,6 +112,7 @@ exports[`CatalogTile renders properly 1`] = `
111112
</div>
112113
<div
113114
class="pf-c-card__title catalog-tile-pf-header"
115+
id="single-badge-test-title"
114116
>
115117
<div
116118
class="catalog-tile-pf-title"
@@ -215,6 +217,7 @@ exports[`CatalogTile renders properly 1`] = `
215217
</div>
216218
<div
217219
class="pf-c-card__title catalog-tile-pf-header"
220+
id="multi-badge-test-title"
218221
>
219222
<div
220223
class="catalog-tile-pf-title"
@@ -291,6 +294,7 @@ exports[`CatalogTile renders properly 1`] = `
291294
</div>
292295
<div
293296
class="pf-c-card__title catalog-tile-pf-header"
297+
id="test-iconClass-title"
294298
>
295299
<div
296300
class="catalog-tile-pf-title"
@@ -364,6 +368,7 @@ exports[`CatalogTile renders properly 1`] = `
364368
</div>
365369
<div
366370
class="pf-c-card__title catalog-tile-pf-header"
371+
id="tile-footer-test-title"
367372
>
368373
<div
369374
class="catalog-tile-pf-title"
@@ -442,6 +447,7 @@ exports[`CatalogTile renders properly 1`] = `
442447
</div>
443448
<div
444449
class="pf-c-card__title catalog-tile-pf-header"
450+
id="custom-icon-svg-test-title"
445451
>
446452
<div
447453
class="catalog-tile-pf-title"

‎packages/react-core/src/components/Card/Card.tsx‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,28 @@ export interface CardProps extends React.HTMLProps<HTMLElement>, OUIAProps {
3636
isPlain?: boolean;
3737
/** Flag indicating if a card is expanded. Modifies the card to be expandable. */
3838
isExpanded?: boolean;
39+
/** Flag indicating that the card should render a hidden input to make it selectable */
40+
hasSelectableInput?: boolean;
41+
/** Aria label to apply to the selectable input if one is rendered */
42+
selectableInputAriaLabel?: string;
43+
/** Callback that executes when the selectable input is changed */
44+
onSelectableInputChange?: (labelledBy: string, event: React.FormEvent<HTMLInputElement>) => void;
3945
}
4046

4147
interface CardContextProps {
4248
cardId: string;
49+
registerTitleId: (id: string) => void;
4350
isExpanded: boolean;
4451
}
4552

53+
interface AriaProps {
54+
'aria-label'?: string;
55+
'aria-labelledby'?: string;
56+
}
57+
4658
export const CardContext = React.createContext<Partial<CardContextProps>>({
4759
cardId: '',
60+
registerTitleId: () => {},
4861
isExpanded: false
4962
});
5063

@@ -67,10 +80,16 @@ export const Card: React.FunctionComponent<CardProps> = ({
6780
isPlain = false,
6881
ouiaId,
6982
ouiaSafe = true,
83+
hasSelectableInput = false,
84+
selectableInputAriaLabel,
85+
onSelectableInputChange = () => {},
7086
...props
7187
}: CardProps) => {
7288
const Component = component as any;
7389
const ouiaProps = useOUIAProps(Card.displayName, ouiaId, ouiaSafe);
90+
const [titleId, setTitleId] = React.useState('');
91+
const [ariaProps, setAriaProps] = React.useState<AriaProps>();
92+
7493
if (isCompact && isLarge) {
7594
// eslint-disable-next-line no-console
7695
console.warn('Card: Cannot use isCompact with isLarge. Defaulting to isCompact');
@@ -90,13 +109,47 @@ export const Card: React.FunctionComponent<CardProps> = ({
90109
return '';
91110
};
92111

112+
const containsCardTitleChildRef = React.useRef(false);
113+
114+
const registerTitleId = (id: string) => {
115+
setTitleId(id);
116+
containsCardTitleChildRef.current = !!id;
117+
};
118+
119+
React.useEffect(() => {
120+
if (selectableInputAriaLabel) {
121+
setAriaProps({ 'aria-label': selectableInputAriaLabel });
122+
} else if (titleId) {
123+
setAriaProps({ 'aria-labelledby': titleId });
124+
} else if (hasSelectableInput && !containsCardTitleChildRef.current) {
125+
setAriaProps({});
126+
// eslint-disable-next-line no-console
127+
console.warn(
128+
'If no CardTitle component is passed as a child of Card the selectableInputAriaLabel prop must be passed'
129+
);
130+
}
131+
}, [hasSelectableInput, selectableInputAriaLabel, titleId]);
132+
93133
return (
94134
<CardContext.Provider
95135
value={{
96136
cardId: id,
137+
registerTitleId,
97138
isExpanded
98139
}}
99140
>
141+
{hasSelectableInput && (
142+
<input
143+
className="pf-screen-reader"
144+
id={`${id}-input`}
145+
{...ariaProps}
146+
type="checkbox"
147+
checked={isSelected}
148+
onChange={event => onSelectableInputChange(id, event)}
149+
disabled={isDisabledRaised}
150+
tabIndex={-1}
151+
/>
152+
)}
100153
<Component
101154
id={id}
102155
className={css(

‎packages/react-core/src/components/Card/CardTitle.tsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as React from 'react';
22
import { css } from '@patternfly/react-styles';
33
import styles from '@patternfly/react-styles/css/components/Card/card';
4+
import { CardContext } from './Card';
45

56
export interface CardTitleProps extends React.HTMLProps<HTMLDivElement> {
67
/** Content rendered inside the CardTitle */
@@ -17,9 +18,18 @@ export const CardTitle: React.FunctionComponent<CardTitleProps> = ({
1718
component = 'div',
1819
...props
1920
}: CardTitleProps) => {
21+
const { cardId, registerTitleId } = React.useContext(CardContext);
2022
const Component = component as any;
23+
const titleId = cardId ? `${cardId}-title` : '';
24+
25+
React.useEffect(() => {
26+
registerTitleId(titleId);
27+
28+
return () => registerTitleId('');
29+
}, [registerTitleId, titleId]);
30+
2131
return (
22-
<Component className={css(styles.cardTitle, className)} {...props}>
32+
<Component className={css(styles.cardTitle, className)} id={titleId || undefined} {...props}>
2333
{children}
2434
</Component>
2535
);

‎packages/react-core/src/components/Card/__tests__/Card.test.tsx‎

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import React from 'react';
2+
23
import { render, screen } from '@testing-library/react';
3-
import { Card } from '../Card';
4+
import '@testing-library/jest-dom';
5+
6+
import { Card, CardContext } from '../Card';
47

58
describe('Card', () => {
69
test('renders with PatternFly Core styles', () => {
@@ -118,4 +121,90 @@ describe('Card', () => {
118121
render(<Card isLarge isCompact />);
119122
expect(consoleWarnMock).toHaveBeenCalled();
120123
});
124+
125+
test('card renders with a hidden input to improve a11y when hasSelectableInput is passed', () => {
126+
render(<Card isSelectable hasSelectableInput />);
127+
128+
const selectableInput = screen.getByRole('checkbox', { hidden: true });
129+
130+
expect(selectableInput).toBeInTheDocument();
131+
});
132+
133+
test('card does not render the hidden input when hasSelectableInput is not passed', () => {
134+
render(<Card isSelectable />);
135+
136+
const selectableInput = screen.queryByRole('checkbox', { hidden: true });
137+
138+
expect(selectableInput).not.toBeInTheDocument();
139+
});
140+
141+
test('card warns when hasSelectableInput is passed without selectableInputAriaLabel or a card title', () => {
142+
const consoleWarnMock = jest.fn();
143+
global.console = { warn: consoleWarnMock } as any;
144+
145+
render(<Card isSelectable hasSelectableInput />);
146+
147+
const selectableInput = screen.getByRole('checkbox', { hidden: true });
148+
149+
expect(consoleWarnMock).toBeCalled();
150+
expect(selectableInput).toHaveAccessibleName('');
151+
});
152+
153+
test('card applies selectableInputAriaLabel to the hidden input', () => {
154+
render(<Card isSelectable hasSelectableInput selectableInputAriaLabel="Input label test" />);
155+
156+
const selectableInput = screen.getByRole('checkbox', { hidden: true });
157+
158+
expect(selectableInput).toHaveAccessibleName('Input label test');
159+
});
160+
161+
test('card applies the supplied card title as the aria label of the hidden input', () => {
162+
163+
// this component is used to mock the CardTitle's title registry behavior to keep this a pure unit test
164+
const MockCardTitle = ({ children }) => {
165+
const { registerTitleId } = React.useContext(CardContext);
166+
const id = 'card-title-id';
167+
168+
React.useEffect(() => {
169+
registerTitleId(id);
170+
});
171+
172+
return <div id={id}>{children}</div>;
173+
};
174+
175+
render(
176+
<Card id="card" isSelectable hasSelectableInput>
177+
<MockCardTitle>Card title from title component</MockCardTitle>
178+
</Card>
179+
);
180+
181+
const selectableInput = screen.getByRole('checkbox', { hidden: true });
182+
183+
expect(selectableInput).toHaveAccessibleName('Card title from title component');
184+
});
185+
186+
test('card prioritizes selectableInputAriaLabel over card title labelling via card title', () => {
187+
188+
// this component is used to mock the CardTitle's title registry behavior to keep this a pure unit test
189+
const MockCardTitle = ({ children }) => {
190+
const { registerTitleId } = React.useContext(CardContext);
191+
const id = 'card-title-id';
192+
193+
React.useEffect(() => {
194+
registerTitleId(id);
195+
});
196+
197+
return <div id={id}>{children}</div>;
198+
};
199+
200+
render(
201+
<Card id="card" isSelectable hasSelectableInput selectableInputAriaLabel="Input label test">
202+
<MockCardTitle>Card title from title component</MockCardTitle>
203+
</Card>
204+
);
205+
206+
const selectableInput = screen.getByRole('checkbox', { hidden: true });
207+
208+
expect(selectableInput).toHaveAccessibleName('Input label test');
209+
});
121210
});

‎packages/react-core/src/components/Card/__tests__/CardTitle.test.tsx‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React from 'react';
22
import { render, screen } from '@testing-library/react';
33
import { CardTitle } from '../CardTitle';
4+
import { CardContext } from '../Card';
45

56
describe('CardTitle', () => {
67
test('renders with PatternFly Core styles', () => {
@@ -19,4 +20,16 @@ describe('CardTitle', () => {
1920
render(<CardTitle data-testid={testId} />);
2021
expect(screen.getByTestId(testId)).toBeInTheDocument();
2122
});
23+
24+
test('calls the registerTitleId function provided by the CardContext with the generated title id', () => {
25+
const mockRegisterTitleId = jest.fn();
26+
27+
render(
28+
<CardContext.Provider value={{ cardId: 'card', registerTitleId: mockRegisterTitleId }}>
29+
<CardTitle>text</CardTitle>
30+
</CardContext.Provider>
31+
);
32+
33+
expect(mockRegisterTitleId).toHaveBeenCalledWith('card-title');
34+
});
2235
});

‎packages/react-core/src/components/Card/examples/Card.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,23 @@ import pfLogoSmall from './pf-logo-small.svg';
7171
```ts file='./CardSelectable.tsx'
7272
```
7373

74+
### Selectable accessibility highlight
75+
76+
This example demonstrates how the `hasSelectableInput` and `onSelectableInputChange` props improve accessibility for selectable cards.
77+
78+
The first card sets `hasSelectableInput` to true, which renders a checkbox input that is only visible to, and navigable by, screen readers. This input communicates to assistive technology users that a card is selectable, and if so, it communicates the current selection state as well.
79+
80+
By default this input will have an aria-label that corresponds to the title given to the card if using the card title component. If you don't use the card title component in your selectable card, you must pass a custom aria-label for this input using the `selectableInputAriaLabel` prop.
81+
82+
The first card also (by passing an onchange callback to `onSelectableInputChange`) enables the selection/deselection of the associated card by checking/unchecking the checkbox input.
83+
84+
The second card does not set `hasSelectableInput` to true, so the input is not rendered. It does not communicate to screen reader users that it is selectable or if it is currently selected.
85+
86+
To best understand this example it is encouraged that you navigate both of these cards using a screen reader.
87+
88+
```ts file='./CardSelectableA11yHighlight.tsx'
89+
```
90+
7491
### With heading element
7592

7693
```ts file='./CardWithHeadingElement.tsx'

‎packages/react-core/src/components/Card/examples/CardLegacySelectable.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ export const CardLegacySelectable: React.FunctionComponent = () => {
3131
setSelected(newSelected);
3232
};
3333

34+
const onChange = (labelledById: string, _event: React.FormEvent<HTMLInputElement>) => {
35+
const newSelected = labelledById === selected ? null : labelledById;
36+
setSelected(newSelected);
37+
};
38+
3439
const onToggle = (isOpen: boolean, event: any) => {
3540
event.stopPropagation();
3641
setIsKebabOpen(isOpen);
@@ -65,8 +70,10 @@ export const CardLegacySelectable: React.FunctionComponent = () => {
6570
id="legacy-first-card"
6671
onKeyDown={onKeyDown}
6772
onClick={onClick}
73+
onSelectableInputChange={onChange}
6874
isSelectable
6975
isSelected={selected === 'legacy-first-card'}
76+
hasSelectableInput
7077
>
7178
<CardHeader>
7279
<CardActions>
@@ -80,18 +87,20 @@ export const CardLegacySelectable: React.FunctionComponent = () => {
8087
/>
8188
</CardActions>
8289
</CardHeader>
83-
<CardTitle>First card</CardTitle>
90+
<CardTitle>First legacy selectable card</CardTitle>
8491
<CardBody>This is a selectable card. Click me to select me. Click again to deselect me.</CardBody>
8592
</Card>
8693
<br />
8794
<Card
8895
id="legacy-second-card"
8996
onKeyDown={onKeyDown}
9097
onClick={onClick}
98+
onSelectableInputChange={onChange}
9199
isSelectable
92100
isSelected={selected === 'legacy-second-card'}
101+
hasSelectableInput
93102
>
94-
<CardTitle>Second card</CardTitle>
103+
<CardTitle>Second legacy selectable card</CardTitle>
95104
<CardBody>This is a selectable card. Click me to select me. Click again to deselect me.</CardBody>
96105
</Card>
97106
</>

0 commit comments

Comments
 (0)