Skip to content

Commit 913d41e

Browse files
avinashbotgeorgylobko
authored andcommitted
feat: Add ariaLabel property to icon component (#3244)
1 parent e2f8294 commit 913d41e

File tree

13 files changed

+97
-46
lines changed

13 files changed

+97
-46
lines changed

src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8948,12 +8948,19 @@ exports[`Documenter definition for icon matches the snapshot: icon 1`] = `
89488948
"name": "Icon",
89498949
"properties": [
89508950
{
8951-
"description": "Specifies alternate text for a custom icon (using the \`url\` attribute). We recommend that you provide this for accessibility.
8951+
"deprecatedTag": "Use \`ariaLabel\` instead.",
8952+
"description": "Specifies alternate text for a custom icon (using the \`url\` attribute).
89528953
This property is ignored if you use a predefined icon or if you set your custom icon using the \`svg\` slot.",
89538954
"name": "alt",
89548955
"optional": true,
89558956
"type": "string",
89568957
},
8958+
{
8959+
"description": "Specifies alternate text for the icon. We recommend that you provide this for accessibility.",
8960+
"name": "ariaLabel",
8961+
"optional": true,
8962+
"type": "string",
8963+
},
89578964
{
89588965
"deprecatedTag": "Custom CSS is not supported. For testing and other use cases, use [data attributes](https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes).",
89598966
"description": "Adds the specified classes to the root element of the component.",

src/alert/__tests__/alert.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ describe('Alert Component', () => {
8585
});
8686
it('status icon does not have a label by default', () => {
8787
const { wrapper } = renderAlert({});
88-
expect(wrapper.find('[role="img"]')!.getElement()).not.toHaveAttribute('aria-label');
88+
expect(wrapper.find('[role="img"]')).toBeNull();
8989
});
9090
it('status icon can have a label', () => {
9191
const { wrapper } = renderAlert({ i18nStrings });
@@ -215,7 +215,7 @@ describe('Alert Component', () => {
215215
</TestI18nProvider>
216216
);
217217
const wrapper = createWrapper(container)!.findAlert()!;
218-
const statusIcon = wrapper.findByClassName(styles.icon)!.getElement();
218+
const statusIcon = wrapper.findByClassName(styles.icon)!.findIcon()!.getElement();
219219
const dismissButton = wrapper.findDismissButton()!.getElement();
220220
return { statusIcon, dismissButton };
221221
}

src/alert/internal.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ const InternalAlert = React.forwardRef(
132132
)}
133133
>
134134
<div className={styles['alert-focus-wrapper']} tabIndex={-1} ref={focusRef}>
135-
<div className={clsx(styles.icon, styles.text)} role="img" aria-label={statusIconAriaLabel}>
136-
<InternalIcon name={typeToIcon[type]} size={size} />
135+
<div className={clsx(styles.icon, styles.text)}>
136+
<InternalIcon name={typeToIcon[type]} size={size} ariaLabel={statusIconAriaLabel} />
137137
</div>
138138
<div className={clsx(styles.message, styles.text)}>
139139
<div

src/button/internal.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,11 @@ export const InternalButton = React.forwardRef(
238238
{external && (
239239
<>
240240
&nbsp;
241-
<span
242-
role="img"
243-
aria-label={i18n('i18nStrings.externalIconAriaLabel', i18nStrings?.externalIconAriaLabel)}
244-
>
245-
<Icon name="external" className={testUtilStyles['external-icon']} />
246-
</span>
241+
<Icon
242+
name="external"
243+
className={testUtilStyles['external-icon']}
244+
ariaLabel={i18n('i18nStrings.externalIconAriaLabel', i18nStrings?.externalIconAriaLabel)}
245+
/>
247246
</>
248247
)}
249248
</>

src/flashbar/collapsible-flashbar.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -372,10 +372,8 @@ const NotificationTypeCount = ({
372372
}) => {
373373
return (
374374
<span className={styles['type-count']}>
375-
<span aria-label={label} role="img">
376-
<span title={label} aria-hidden="true">
377-
<InternalIcon name={iconName} />
378-
</span>
375+
<span title={label}>
376+
<InternalIcon name={iconName} ariaLabel={label} />
379377
</span>
380378
<span className={styles['count-number']}>{count}</span>
381379
</span>

src/flashbar/flash.tsx

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -134,20 +134,25 @@ export const Flash = React.forwardRef(
134134
const headerRef = useMergeRefs(headerRefAction, headerRefContent, headerRefObject);
135135
const contentRef = useMergeRefs(contentRefAction, contentRefContent, contentRefObject);
136136

137-
const iconType = ICON_TYPES[type];
137+
const statusIconAriaLabel =
138+
props.statusIconAriaLabel ||
139+
i18nStrings?.[`${loading || type === 'in-progress' ? 'inProgress' : type}IconAriaLabel`];
138140

139-
const icon = loading ? <InternalSpinner /> : <InternalIcon name={iconType} />;
141+
const iconType = ICON_TYPES[type];
142+
const icon = loading ? (
143+
<span role="img" aria-label={statusIconAriaLabel}>
144+
<InternalSpinner />
145+
</span>
146+
) : (
147+
<InternalIcon name={iconType} ariaLabel={statusIconAriaLabel} />
148+
);
140149

141150
const effectiveType = loading ? 'info' : type;
142151

143152
const analyticsAttributes = {
144153
[DATA_ATTR_ANALYTICS_FLASHBAR]: effectiveType,
145154
};
146155

147-
const statusIconAriaLabel =
148-
props.statusIconAriaLabel ||
149-
i18nStrings?.[`${loading || type === 'in-progress' ? 'inProgress' : type}IconAriaLabel`];
150-
151156
return (
152157
// We're not using "polite" or "assertive" here, just turning default behavior off.
153158
// eslint-disable-next-line @cloudscape-design/prefer-live-region
@@ -175,13 +180,7 @@ export const Flash = React.forwardRef(
175180
>
176181
<div className={styles['flash-body']}>
177182
<div className={styles['flash-focus-container']} tabIndex={-1}>
178-
<div
179-
className={clsx(styles['flash-icon'], styles['flash-text'])}
180-
role="img"
181-
aria-label={statusIconAriaLabel}
182-
>
183-
{icon}
184-
</div>
183+
<div className={clsx(styles['flash-icon'], styles['flash-text'])}>{icon}</div>
185184
<div className={clsx(styles['flash-message'], styles['flash-text'])}>
186185
<div
187186
className={clsx(

src/form-field/internal.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ export function FormFieldError({ id, children, errorIconAriaLabel }: FormFieldEr
5252
<>
5353
<div id={id} className={styles.error}>
5454
<div className={styles['error-icon-shake-wrapper']}>
55-
<div role="img" aria-label={i18nErrorIconAriaLabel} className={styles['error-icon-scale-wrapper']}>
56-
<InternalIcon name="status-negative" size="small" />
55+
<div className={styles['error-icon-scale-wrapper']}>
56+
<InternalIcon name="status-negative" size="small" ariaLabel={i18nErrorIconAriaLabel} />
5757
</div>
5858
</div>
5959
<span className={styles.error__message} ref={contentRef}>
@@ -75,8 +75,8 @@ export function FormFieldWarning({ id, children, warningIconAriaLabel }: FormFie
7575
<>
7676
<div id={id} className={styles.warning}>
7777
<div className={styles['warning-icon-shake-wrapper']}>
78-
<div role="img" aria-label={i18nWarningIconAriaLabel} className={styles['warning-icon-scale-wrapper']}>
79-
<InternalIcon name="status-warning" size="small" />
78+
<div className={styles['warning-icon-scale-wrapper']}>
79+
<InternalIcon name="status-warning" size="small" ariaLabel={i18nWarningIconAriaLabel} />
8080
</div>
8181
</div>
8282
<span className={styles.warning__message} ref={contentRef}>

src/icon/__tests__/icon.test.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,28 @@ describe('Icon Component', () => {
8585
expect(img).toHaveAttribute('alt', 'custom icon');
8686
});
8787

88+
test('should render a custom icon with alternate text when a url and ariaLabel are provided', () => {
89+
const { container } = render(<Icon url={url} ariaLabel="custom icon" />);
90+
const wrapper = createWrapper(container);
91+
expect(wrapper.findIcon()!.getElement()).not.toHaveAttribute('aria-label');
92+
expect(container.querySelector('img')).toHaveAttribute('alt', 'custom icon');
93+
});
94+
95+
test('should prefer ariaLabel when alt is also provided', () => {
96+
const { container } = render(<Icon url={url} alt="icon alt" ariaLabel="custom icon" />);
97+
const wrapper = createWrapper(container);
98+
expect(wrapper.findIcon()!.getElement()).not.toHaveAttribute('aria-label');
99+
expect(container.querySelector('img')).toHaveAttribute('alt', 'custom icon');
100+
});
101+
102+
test('should not set aria-hidden="true" for custom svg icons if an ariaLabel is provided', () => {
103+
const { container } = render(<Icon svg={svg} ariaLabel="custom icon" />);
104+
const wrapper = createWrapper(container);
105+
expect(wrapper.findIcon()!.getElement()).not.toHaveAttribute('aria-hidden', 'true');
106+
expect(wrapper.findIcon()!.getElement()).toHaveAttribute('role', 'img');
107+
expect(wrapper.findIcon()!.getElement()).toHaveAttribute('aria-label', 'custom icon');
108+
});
109+
88110
test('should render a custom icon when both name and url are provided', () => {
89111
const { container } = render(<Icon url={url} name="calendar" />);
90112
const img = container.querySelector('img');
@@ -126,6 +148,20 @@ describe('Icon Component', () => {
126148
expect(container.firstElementChild).toBeEmptyDOMElement();
127149
});
128150

151+
test('sets role="img" and the label on the wrapper element when provided', () => {
152+
const { container } = render(<Icon name="calendar" ariaLabel="Calendar" />);
153+
const wrapper = createWrapper(container);
154+
expect(wrapper.findIcon()!.getElement()).toHaveAttribute('role', 'img');
155+
expect(wrapper.findIcon()!.getElement()).toHaveAttribute('aria-label', 'Calendar');
156+
});
157+
158+
test('sets role="img" and the label on the wrapper element even when ariaLabel is an empty string', () => {
159+
const { container } = render(<Icon name="calendar" ariaLabel="" />);
160+
const wrapper = createWrapper(container);
161+
expect(wrapper.findIcon()!.getElement()).toHaveAttribute('role', 'img');
162+
expect(wrapper.findIcon()!.getElement()).toHaveAttribute('aria-label', '');
163+
});
164+
129165
describe('Prototype Pollution attack', () => {
130166
beforeEach(() => {
131167
(Object.prototype as any).attack = '<b>vulnerable</b>';

src/icon/interfaces.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export interface IconProps extends BaseComponentProps {
99
* Specifies the icon to be displayed.
1010
*/
1111
name?: IconProps.Name;
12+
1213
/**
1314
* Specifies the size of the icon.
1415
*
@@ -32,12 +33,20 @@ export interface IconProps extends BaseComponentProps {
3233
* If you set both `url` and `svg`, `svg` will take precedence.
3334
*/
3435
url?: string;
36+
3537
/**
36-
* Specifies alternate text for a custom icon (using the `url` attribute). We recommend that you provide this for accessibility.
38+
* Specifies alternate text for a custom icon (using the `url` attribute).
3739
* This property is ignored if you use a predefined icon or if you set your custom icon using the `svg` slot.
40+
*
41+
* @deprecated Use `ariaLabel` instead.
3842
*/
3943
alt?: string;
4044

45+
/**
46+
* Specifies alternate text for the icon. We recommend that you provide this for accessibility.
47+
*/
48+
ariaLabel?: string;
49+
4150
/**
4251
* Specifies the SVG of a custom icon.
4352
*

src/icon/internal.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const InternalIcon = ({
4444
variant = 'normal',
4545
url,
4646
alt,
47+
ariaLabel,
4748
svg,
4849
badge,
4950
__internalRootRef = null,
@@ -82,6 +83,8 @@ const InternalIcon = ({
8283
});
8384

8485
const mergedRef = useMergeRefs(iconRef, __internalRootRef);
86+
const hasAriaLabel = typeof ariaLabel === 'string';
87+
const labelAttributes = hasAriaLabel ? { role: 'img', 'aria-label': ariaLabel } : {};
8588

8689
if (svg) {
8790
if (url) {
@@ -91,7 +94,7 @@ const InternalIcon = ({
9194
);
9295
}
9396
return (
94-
<span {...baseProps} ref={mergedRef} aria-hidden="true" style={inlineStyles}>
97+
<span {...baseProps} {...labelAttributes} ref={mergedRef} aria-hidden={!hasAriaLabel} style={inlineStyles}>
9598
{svg}
9699
</span>
97100
);
@@ -100,7 +103,7 @@ const InternalIcon = ({
100103
if (url) {
101104
return (
102105
<span {...baseProps} ref={mergedRef} style={inlineStyles}>
103-
<img src={url} alt={alt} />
106+
<img src={url} alt={ariaLabel ?? alt} />
104107
</span>
105108
);
106109
}
@@ -131,7 +134,7 @@ const InternalIcon = ({
131134
}
132135

133136
return (
134-
<span {...baseProps} ref={mergedRef} style={inlineStyles}>
137+
<span {...baseProps} {...labelAttributes} ref={mergedRef} style={inlineStyles}>
135138
{validIcon ? iconMap(name) : undefined}
136139
</span>
137140
);

0 commit comments

Comments
 (0)