Skip to content

Commit bdc821d

Browse files
committed
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.
1 parent 0e3b26c commit bdc821d

19 files changed

Lines changed: 96 additions & 60 deletions

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: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ 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;
1614
/** Additional classes added to the copy button */
1715
className?: string;
1816
/** Exit delay on the copy button tooltip */
@@ -55,7 +53,6 @@ export const ClipboardCopyButton: React.FunctionComponent<ClipboardCopyButtonPro
5553
position = 'top',
5654
'aria-label': ariaLabel = 'Copyable input',
5755
id,
58-
textId,
5956
children,
6057
variant = 'control',
6158
onTooltipHidden = () => {},
@@ -86,7 +83,6 @@ export const ClipboardCopyButton: React.FunctionComponent<ClipboardCopyButtonPro
8683
aria-label={ariaLabel}
8784
className={className}
8885
id={id}
89-
aria-labelledby={`${id} ${textId}`}
9086
icon={<CopyIcon />}
9187
{...props}
9288
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__/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__/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"
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
);

packages/react-core/src/components/ClipboardCopy/examples/ClipboardCopyExpanded.tsx

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

33
export const ClipboardCopyExpanded: React.FunctionComponent = () => (
4-
<ClipboardCopy hoverTip="Copy" clickTip="Copied" variant={ClipboardCopyVariant.expansion}>
4+
<ClipboardCopy
5+
toggleAriaLabel="Show content for expanded example"
6+
copyAriaLabel="Copy expanded example"
7+
hoverTip="Copy"
8+
clickTip="Copied"
9+
variant={ClipboardCopyVariant.expansion}
10+
>
511
Got a lot of text here, need to see all of it? Click that arrow on the left side and check out the resulting
612
expansion.
713
</ClipboardCopy>

packages/react-core/src/components/ClipboardCopy/examples/ClipboardCopyExpandedWithArray.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@ const text = [
77
];
88

99
export const ClipboardCopyExpandedWithArray: React.FunctionComponent = () => (
10-
<ClipboardCopy hoverTip="Copy" clickTip="Copied" variant={ClipboardCopyVariant.expansion}>
10+
<ClipboardCopy
11+
copyAriaLabel="Copy expanded example with array"
12+
toggleAriaLabel="Show content for expanded example with array"
13+
hoverTip="Copy"
14+
clickTip="Copied"
15+
variant={ClipboardCopyVariant.expansion}
16+
>
1117
{text.join(' ')}
1218
</ClipboardCopy>
1319
);

0 commit comments

Comments
 (0)