Skip to content

Commit 2ac9dcb

Browse files
rebeccaalpertrebeccaalpert
authored
fix(ClipboardCopy): Adjust aria-labels on buttons (#11996)
* fix(ClipboardCopy): Adjust aria-labels on buttons Remove aria-labelled by from copy and expand buttons in favor of aria-label. In case of copy button, add new prop for aria-label and pull from hoverTip if not provided. Long term, we will want to further separate these two in a breaking release. * Add tests and deprecate prop
1 parent 0e3b26c commit 2ac9dcb

21 files changed

+115
-61
lines changed

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ export interface ClipboardCopyProps extends Omit<React.HTMLProps<HTMLDivElement>
4949
hoverTip?: string;
5050
/** Tooltip message to display when clicking the copy button */
5151
clickTip?: string;
52+
/** Aria-label to use on the copy button */
53+
copyAriaLabel?: string;
5254
/** Aria-label to use on the TextInput. */
5355
textAriaLabel?: string;
5456
/** Aria-label to use on the ClipboardCopyToggle. */
@@ -194,6 +196,7 @@ class ClipboardCopy extends Component<ClipboardCopyProps, ClipboardCopyState> {
194196
truncation,
195197
ouiaId,
196198
ouiaSafe,
199+
copyAriaLabel,
197200
...divProps
198201
} = this.props;
199202
const textIdPrefix = 'text-input-';
@@ -250,8 +253,7 @@ class ClipboardCopy extends Component<ClipboardCopyProps, ClipboardCopyState> {
250253
maxWidth={maxWidth}
251254
position={position}
252255
id={`copy-button-${id}`}
253-
textId={`text-input-${id}`}
254-
aria-label={hoverTip}
256+
aria-label={copyAriaLabel ?? hoverTip}
255257
onClick={(event: any) => {
256258
onCopy(event, copyableText);
257259
this.setState({ copied: true });
@@ -285,7 +287,6 @@ class ClipboardCopy extends Component<ClipboardCopyProps, ClipboardCopyState> {
285287
}
286288
}}
287289
id={`${toggleIdPrefix}${id}`}
288-
textId={`${textIdPrefix}${id}`}
289290
contentId={`${contentIdPrefix}${id}`}
290291
aria-label={toggleAriaLabel}
291292
/>
@@ -304,8 +305,7 @@ class ClipboardCopy extends Component<ClipboardCopyProps, ClipboardCopyState> {
304305
maxWidth={maxWidth}
305306
position={position}
306307
id={`copy-button-${id}`}
307-
textId={`text-input-${id}`}
308-
aria-label={hoverTip}
308+
aria-label={copyAriaLabel ?? hoverTip}
309309
onClick={(event: any) => {
310310
onCopy(event, this.state.expanded ? this.state.textWhenExpanded : copyableText);
311311
this.setState({ copied: true });

‎packages/react-core/src/components/ClipboardCopy/ClipboardCopyButton.tsx‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ export interface ClipboardCopyButtonProps
1111
children: React.ReactNode;
1212
/** ID of the copy button */
1313
id: string;
14-
/** ID of the content that is being copied */
15-
textId: string;
14+
/** @deprecated ID of the content that is being copied */
15+
textId?: string;
1616
/** Additional classes added to the copy button */
1717
className?: string;
1818
/** Exit delay on the copy button tooltip */
@@ -55,7 +55,6 @@ export const ClipboardCopyButton: React.FunctionComponent<ClipboardCopyButtonPro
5555
position = 'top',
5656
'aria-label': ariaLabel = 'Copyable input',
5757
id,
58-
textId,
5958
children,
6059
variant = 'control',
6160
onTooltipHidden = () => {},
@@ -86,7 +85,6 @@ export const ClipboardCopyButton: React.FunctionComponent<ClipboardCopyButtonPro
8685
aria-label={ariaLabel}
8786
className={className}
8887
id={id}
89-
aria-labelledby={`${id} ${textId}`}
9088
icon={<CopyIcon />}
9189
{...props}
9290
ref={triggerRef}

‎packages/react-core/src/components/ClipboardCopy/ClipboardCopyToggle.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ export interface ClipboardCopyToggleProps
77
extends Omit<React.DetailedHTMLProps<React.ButtonHTMLAttributes<HTMLButtonElement>, HTMLButtonElement>, 'ref'> {
88
onClick: (event: React.MouseEvent) => void;
99
id: string;
10-
textId: string;
1110
contentId: string;
1211
isExpanded?: boolean;
1312
className?: string;
@@ -16,7 +15,6 @@ export interface ClipboardCopyToggleProps
1615
export const ClipboardCopyToggle: React.FunctionComponent<ClipboardCopyToggleProps> = ({
1716
onClick,
1817
id,
19-
textId,
2018
contentId,
2119
isExpanded = false,
2220
...props
@@ -26,7 +24,6 @@ export const ClipboardCopyToggle: React.FunctionComponent<ClipboardCopyTogglePro
2624
variant="control"
2725
onClick={onClick}
2826
id={id}
29-
aria-labelledby={`${id} ${textId}`}
3027
aria-controls={isExpanded ? contentId : undefined}
3128
aria-expanded={isExpanded}
3229
{...props}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,22 @@ test('Passes position to ClipboardCopyButton when variant is inline-compact', ()
265265
expect(screen.getByText('position: bottom')).toBeVisible();
266266
});
267267

268+
test('Passes copyAriaLabel to ClipboardCopyButton', () => {
269+
render(<ClipboardCopy copyAriaLabel="Copy text">{children}</ClipboardCopy>);
270+
271+
expect(screen.getByText('button-ariaLabel: Copy text')).toBeVisible();
272+
});
273+
274+
test('Passes copyAriaLabel over hoverTip to ClipboardCopyButton when both are provided', () => {
275+
render(
276+
<ClipboardCopy copyAriaLabel="Copy text" hoverTip="Hover tip">
277+
{children}
278+
</ClipboardCopy>
279+
);
280+
281+
expect(screen.getByText('button-ariaLabel: Copy text')).toBeVisible();
282+
});
283+
268284
test('Passes toggleAriaLabel to ClipboardCopyToggle when variant is expansion', () => {
269285
render(
270286
<ClipboardCopy variant="expansion" toggleAriaLabel="toggle label">

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

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ jest.mock('../../Tooltip', () => ({
2121
const requiredProps = {
2222
onClick: jest.fn(),
2323
children: 'Button content',
24-
id: 'button-id',
25-
textId: 'text-id'
24+
id: 'button-id'
2625
};
2726

2827
// Must be kept as first test to avoid Button's ouiaId updating in snapshots
@@ -37,37 +36,15 @@ test('Renders with passed id prop', () => {
3736
expect(screen.getByRole('button')).toHaveAttribute('id', 'button-id');
3837
});
3938

40-
test('Renders with aria-labelledby with passed id and textId prop values', () => {
39+
test('Renders with aria-label', () => {
4140
render(
4241
<>
43-
<div id="text-id">Copyable text</div>
44-
<ClipboardCopyButton {...requiredProps} />
45-
</>
46-
);
47-
48-
expect(screen.getByRole('button')).toHaveAccessibleName('Copyable input Copyable text');
49-
});
50-
51-
test('Renders with concatenated aria-label by default', () => {
52-
render(
53-
<>
54-
<div id="text-id">Copyable text</div>
55-
<ClipboardCopyButton {...requiredProps} />
56-
</>
57-
);
58-
59-
expect(screen.getByRole('button')).toHaveAccessibleName('Copyable input Copyable text');
60-
});
61-
62-
test('Renders with concatenated aria-label when custom aria-label is passed', () => {
63-
render(
64-
<>
65-
<div id="text-id">Copyable text</div>
66-
<ClipboardCopyButton aria-label="Custom label" {...requiredProps} />
42+
<div>Copyable text</div>
43+
<ClipboardCopyButton aria-label="Copy" {...requiredProps} />
6744
</>
6845
);
6946

70-
expect(screen.getByRole('button')).toHaveAccessibleName('Custom label Copyable text');
47+
expect(screen.getByRole('button')).toHaveAccessibleName('Copy');
7148
});
7249

7350
test('Passes className to Button', () => {

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import userEvent from '@testing-library/user-event';
55
const onClickMock = jest.fn();
66
const requiredProps = {
77
id: 'main-id',
8-
textId: 'text-id',
98
contentId: 'content-id',
109
onClick: onClickMock
1110
};
@@ -33,15 +32,15 @@ test('Renders with id prop', () => {
3332
expect(screen.getByRole('button')).toHaveAttribute('id', requiredProps.id);
3433
});
3534

36-
test('Renders with aria-labelledby concatenated from id and textId props', () => {
35+
test('Renders with aria-label', () => {
3736
render(
3837
<>
3938
<ClipboardCopyToggle aria-label="Toggle content" {...requiredProps} />
40-
<span id={requiredProps.textId}>Test content</span>
39+
<span>Test content</span>
4140
</>
4241
);
4342

44-
expect(screen.getByRole('button')).toHaveAccessibleName('Toggle content Test content');
43+
expect(screen.getByRole('button')).toHaveAccessibleName('Toggle content');
4544
});
4645

4746
test('Does not render with aria-controls when isExpanded is false', () => {

‎packages/react-core/src/components/ClipboardCopy/__tests__/__snapshots__/ClipboardCopy.test.tsx.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ exports[`Matches snapshot 1`] = `
1818
<input
1919
aria-invalid="false"
2020
aria-label="Copyable input"
21-
data-ouia-component-id="OUIA-Generated-TextInputBase-34"
21+
data-ouia-component-id="OUIA-Generated-TextInputBase-36"
2222
data-ouia-component-type="PF6/TextInput"
2323
data-ouia-safe="true"
2424
id="text-input-generated-id"

‎packages/react-core/src/components/ClipboardCopy/__tests__/__snapshots__/ClipboardCopyButton.test.tsx.snap‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ exports[`Matches snapshot 1`] = `
2929
</a>
3030
<button
3131
aria-label="Copyable input"
32-
aria-labelledby="button-id text-id"
3332
class="pf-v6-c-button pf-m-control"
3433
data-ouia-component-id="OUIA-Generated-Button-control-1"
3534
data-ouia-component-type="PF6/Button"

‎packages/react-core/src/components/ClipboardCopy/__tests__/__snapshots__/ClipboardCopyToggle.test.tsx.snap‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ exports[`Matches snapshot 1`] = `
44
<DocumentFragment>
55
<button
66
aria-expanded="false"
7-
aria-labelledby="main-id text-id"
87
class="pf-v6-c-button pf-m-control"
98
data-ouia-component-id="OUIA-Generated-Button-control-1"
109
data-ouia-component-type="PF6/Button"

‎packages/react-core/src/components/ClipboardCopy/examples/ClipboardCopyBasic.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { ClipboardCopy } from '@patternfly/react-core';
22

33
export const ClipboardCopyBasic: React.FunctionComponent = () => (
4-
<ClipboardCopy hoverTip="Copy" clickTip="Copied">
4+
<ClipboardCopy copyAriaLabel="Copy basic example" hoverTip="Copy" clickTip="Copied">
55
This is editable
66
</ClipboardCopy>
77
);

0 commit comments

Comments
 (0)