feat: Add ariaLabel property to icon component (#3244)

cloudscape-design · Feb 5, 2025 · f8a00dd · f8a00dd
1 parent 1ade36a
commit f8a00dd
Show file tree

Hide file tree

Showing 13 changed files with 97 additions and 46 deletions.
diff --git a/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap b/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap
@@ -8948,12 +8948,19 @@ exports[`Documenter definition for icon matches the snapshot: icon 1`] = `
   "name": "Icon",
   "properties": [
     {
-      "description": "Specifies alternate text for a custom icon (using the \`url\` attribute). We recommend that you provide this for accessibility.
+      "deprecatedTag": "Use \`ariaLabel\` instead.",
+      "description": "Specifies alternate text for a custom icon (using the \`url\` attribute).
 This property is ignored if you use a predefined icon or if you set your custom icon using the \`svg\` slot.",
       "name": "alt",
       "optional": true,
       "type": "string",
     },
+    {
+      "description": "Specifies alternate text for the icon. We recommend that you provide this for accessibility.",
+      "name": "ariaLabel",
+      "optional": true,
+      "type": "string",
+    },
     {
       "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).",
       "description": "Adds the specified classes to the root element of the component.",

diff --git a/src/alert/__tests__/alert.test.tsx b/src/alert/__tests__/alert.test.tsx
@@ -85,7 +85,7 @@ describe('Alert Component', () => {
     });
     it('status icon does not have a label by default', () => {
       const { wrapper } = renderAlert({});
-      expect(wrapper.find('[role="img"]')!.getElement()).not.toHaveAttribute('aria-label');
+      expect(wrapper.find('[role="img"]')).toBeNull();
     });
     it('status icon can have a label', () => {
       const { wrapper } = renderAlert({ i18nStrings });
@@ -215,7 +215,7 @@ describe('Alert Component', () => {
         </TestI18nProvider>
       );
       const wrapper = createWrapper(container)!.findAlert()!;
-      const statusIcon = wrapper.findByClassName(styles.icon)!.getElement();
+      const statusIcon = wrapper.findByClassName(styles.icon)!.findIcon()!.getElement();
       const dismissButton = wrapper.findDismissButton()!.getElement();
       return { statusIcon, dismissButton };
     }

diff --git a/src/alert/internal.tsx b/src/alert/internal.tsx
@@ -132,8 +132,8 @@ const InternalAlert = React.forwardRef(
               )}
             >
               <div className={styles['alert-focus-wrapper']} tabIndex={-1} ref={focusRef}>
-                <div className={clsx(styles.icon, styles.text)} role="img" aria-label={statusIconAriaLabel}>
-                  <InternalIcon name={typeToIcon[type]} size={size} />
+                <div className={clsx(styles.icon, styles.text)}>
+                  <InternalIcon name={typeToIcon[type]} size={size} ariaLabel={statusIconAriaLabel} />
                 </div>
                 <div className={clsx(styles.message, styles.text)}>
                   <div

diff --git a/src/button/internal.tsx b/src/button/internal.tsx
@@ -238,12 +238,11 @@ export const InternalButton = React.forwardRef(
             {external && (
               <>
                 &nbsp;
-                <span
-                  role="img"
-                  aria-label={i18n('i18nStrings.externalIconAriaLabel', i18nStrings?.externalIconAriaLabel)}
-                >
-                  <Icon name="external" className={testUtilStyles['external-icon']} />
-                </span>
+                <Icon
+                  name="external"
+                  className={testUtilStyles['external-icon']}
+                  ariaLabel={i18n('i18nStrings.externalIconAriaLabel', i18nStrings?.externalIconAriaLabel)}
+                />
               </>
             )}
           </>

diff --git a/src/flashbar/collapsible-flashbar.tsx b/src/flashbar/collapsible-flashbar.tsx
@@ -372,10 +372,8 @@ const NotificationTypeCount = ({
 }) => {
   return (
     <span className={styles['type-count']}>
-      <span aria-label={label} role="img">
-        <span title={label} aria-hidden="true">
-          <InternalIcon name={iconName} />
-        </span>
+      <span title={label}>
+        <InternalIcon name={iconName} ariaLabel={label} />
       </span>
       <span className={styles['count-number']}>{count}</span>
     </span>

diff --git a/src/flashbar/flash.tsx b/src/flashbar/flash.tsx
@@ -134,20 +134,25 @@ export const Flash = React.forwardRef(
     const headerRef = useMergeRefs(headerRefAction, headerRefContent, headerRefObject);
     const contentRef = useMergeRefs(contentRefAction, contentRefContent, contentRefObject);
 
-    const iconType = ICON_TYPES[type];
+    const statusIconAriaLabel =
+      props.statusIconAriaLabel ||
+      i18nStrings?.[`${loading || type === 'in-progress' ? 'inProgress' : type}IconAriaLabel`];
 
-    const icon = loading ? <InternalSpinner /> : <InternalIcon name={iconType} />;
+    const iconType = ICON_TYPES[type];
+    const icon = loading ? (
+      <span role="img" aria-label={statusIconAriaLabel}>
+        <InternalSpinner />
+      </span>
+    ) : (
+      <InternalIcon name={iconType} ariaLabel={statusIconAriaLabel} />
+    );
 
     const effectiveType = loading ? 'info' : type;
 
     const analyticsAttributes = {
       [DATA_ATTR_ANALYTICS_FLASHBAR]: effectiveType,
     };
 
-    const statusIconAriaLabel =
-      props.statusIconAriaLabel ||
-      i18nStrings?.[`${loading || type === 'in-progress' ? 'inProgress' : type}IconAriaLabel`];
-
     return (
       // We're not using "polite" or "assertive" here, just turning default behavior off.
       // eslint-disable-next-line @cloudscape-design/prefer-live-region
@@ -175,13 +180,7 @@ export const Flash = React.forwardRef(
       >
         <div className={styles['flash-body']}>
           <div className={styles['flash-focus-container']} tabIndex={-1}>
-            <div
-              className={clsx(styles['flash-icon'], styles['flash-text'])}
-              role="img"
-              aria-label={statusIconAriaLabel}
-            >
-              {icon}
-            </div>
+            <div className={clsx(styles['flash-icon'], styles['flash-text'])}>{icon}</div>
             <div className={clsx(styles['flash-message'], styles['flash-text'])}>
               <div
                 className={clsx(

diff --git a/src/form-field/internal.tsx b/src/form-field/internal.tsx
@@ -52,8 +52,8 @@ export function FormFieldError({ id, children, errorIconAriaLabel }: FormFieldEr
     <>
       <div id={id} className={styles.error}>
         <div className={styles['error-icon-shake-wrapper']}>
-          <div role="img" aria-label={i18nErrorIconAriaLabel} className={styles['error-icon-scale-wrapper']}>
-            <InternalIcon name="status-negative" size="small" />
+          <div className={styles['error-icon-scale-wrapper']}>
+            <InternalIcon name="status-negative" size="small" ariaLabel={i18nErrorIconAriaLabel} />
           </div>
         </div>
         <span className={styles.error__message} ref={contentRef}>
@@ -75,8 +75,8 @@ export function FormFieldWarning({ id, children, warningIconAriaLabel }: FormFie
     <>
       <div id={id} className={styles.warning}>
         <div className={styles['warning-icon-shake-wrapper']}>
-          <div role="img" aria-label={i18nWarningIconAriaLabel} className={styles['warning-icon-scale-wrapper']}>
-            <InternalIcon name="status-warning" size="small" />
+          <div className={styles['warning-icon-scale-wrapper']}>
+            <InternalIcon name="status-warning" size="small" ariaLabel={i18nWarningIconAriaLabel} />
           </div>
         </div>
         <span className={styles.warning__message} ref={contentRef}>

diff --git a/src/icon/__tests__/icon.test.tsx b/src/icon/__tests__/icon.test.tsx
@@ -85,6 +85,28 @@ describe('Icon Component', () => {
       expect(img).toHaveAttribute('alt', 'custom icon');
     });
 
+    test('should render a custom icon with alternate text when a url and ariaLabel are provided', () => {
+      const { container } = render(<Icon url={url} ariaLabel="custom icon" />);
+      const wrapper = createWrapper(container);
+      expect(wrapper.findIcon()!.getElement()).not.toHaveAttribute('aria-label');
+      expect(container.querySelector('img')).toHaveAttribute('alt', 'custom icon');
+    });
+
+    test('should prefer ariaLabel when alt is also provided', () => {
+      const { container } = render(<Icon url={url} alt="icon alt" ariaLabel="custom icon" />);
+      const wrapper = createWrapper(container);
+      expect(wrapper.findIcon()!.getElement()).not.toHaveAttribute('aria-label');
+      expect(container.querySelector('img')).toHaveAttribute('alt', 'custom icon');
+    });
+
+    test('should not set aria-hidden="true" for custom svg icons if an ariaLabel is provided', () => {
+      const { container } = render(<Icon svg={svg} ariaLabel="custom icon" />);
+      const wrapper = createWrapper(container);
+      expect(wrapper.findIcon()!.getElement()).not.toHaveAttribute('aria-hidden', 'true');
+      expect(wrapper.findIcon()!.getElement()).toHaveAttribute('role', 'img');
+      expect(wrapper.findIcon()!.getElement()).toHaveAttribute('aria-label', 'custom icon');
+    });
+
     test('should render a custom icon when both name and url are provided', () => {
       const { container } = render(<Icon url={url} name="calendar" />);
       const img = container.querySelector('img');
@@ -126,6 +148,20 @@ describe('Icon Component', () => {
     expect(container.firstElementChild).toBeEmptyDOMElement();
   });
 
+  test('sets role="img" and the label on the wrapper element when provided', () => {
+    const { container } = render(<Icon name="calendar" ariaLabel="Calendar" />);
+    const wrapper = createWrapper(container);
+    expect(wrapper.findIcon()!.getElement()).toHaveAttribute('role', 'img');
+    expect(wrapper.findIcon()!.getElement()).toHaveAttribute('aria-label', 'Calendar');
+  });
+
+  test('sets role="img" and the label on the wrapper element even when ariaLabel is an empty string', () => {
+    const { container } = render(<Icon name="calendar" ariaLabel="" />);
+    const wrapper = createWrapper(container);
+    expect(wrapper.findIcon()!.getElement()).toHaveAttribute('role', 'img');
+    expect(wrapper.findIcon()!.getElement()).toHaveAttribute('aria-label', '');
+  });
+
   describe('Prototype Pollution attack', () => {
     beforeEach(() => {
       (Object.prototype as any).attack = '<b>vulnerable</b>';

diff --git a/src/icon/interfaces.ts b/src/icon/interfaces.ts
@@ -9,6 +9,7 @@ export interface IconProps extends BaseComponentProps {
    * Specifies the icon to be displayed.
    */
   name?: IconProps.Name;
+
   /**
    * Specifies the size of the icon.
    *
@@ -32,12 +33,20 @@ export interface IconProps extends BaseComponentProps {
    * If you set both `url` and `svg`, `svg` will take precedence.
    */
   url?: string;
+
   /**
-   * Specifies alternate text for a custom icon (using the `url` attribute). We recommend that you provide this for accessibility.
+   * Specifies alternate text for a custom icon (using the `url` attribute).
    * This property is ignored if you use a predefined icon or if you set your custom icon using the `svg` slot.
+   *
+   * @deprecated Use `ariaLabel` instead.
    */
   alt?: string;
 
+  /**
+   * Specifies alternate text for the icon. We recommend that you provide this for accessibility.
+   */
+  ariaLabel?: string;
+
   /**
    * Specifies the SVG of a custom icon.
    *

diff --git a/src/icon/internal.tsx b/src/icon/internal.tsx
@@ -44,6 +44,7 @@ const InternalIcon = ({
   variant = 'normal',
   url,
   alt,
+  ariaLabel,
   svg,
   badge,
   __internalRootRef = null,
@@ -82,6 +83,8 @@ const InternalIcon = ({
   });
 
   const mergedRef = useMergeRefs(iconRef, __internalRootRef);
+  const hasAriaLabel = typeof ariaLabel === 'string';
+  const labelAttributes = hasAriaLabel ? { role: 'img', 'aria-label': ariaLabel } : {};
 
   if (svg) {
     if (url) {
@@ -91,7 +94,7 @@ const InternalIcon = ({
       );
     }
     return (
-      <span {...baseProps} ref={mergedRef} aria-hidden="true" style={inlineStyles}>
+      <span {...baseProps} {...labelAttributes} ref={mergedRef} aria-hidden={!hasAriaLabel} style={inlineStyles}>
         {svg}
       </span>
     );
@@ -100,7 +103,7 @@ const InternalIcon = ({
   if (url) {
     return (
       <span {...baseProps} ref={mergedRef} style={inlineStyles}>
-        <img src={url} alt={alt} />
+        <img src={url} alt={ariaLabel ?? alt} />
       </span>
     );
   }
@@ -131,7 +134,7 @@ const InternalIcon = ({
   }
 
   return (
-    <span {...baseProps} ref={mergedRef} style={inlineStyles}>
+    <span {...baseProps} {...labelAttributes} ref={mergedRef} style={inlineStyles}>
       {validIcon ? iconMap(name) : undefined}
     </span>
   );

diff --git a/src/table/__tests__/header-cell.test.tsx b/src/table/__tests__/header-cell.test.tsx
@@ -119,7 +119,10 @@ describe('i18n', () => {
         </TableWrapper>
       </TestI18nProvider>
     );
-    expect(container.querySelector(`.${styles['edit-icon']}`)).toHaveAttribute('aria-label', 'Custom editable');
+    expect(container.querySelector(`.${styles['edit-icon']} [role=img]`)).toHaveAttribute(
+      'aria-label',
+      'Custom editable'
+    );
   });
 
   test('does not set tab index when negative', () => {

diff --git a/src/table/body-cell/index.tsx b/src/table/body-cell/index.tsx
@@ -104,15 +104,13 @@ function TableCellEditable<ItemType>({
             <>
               <span
                 className={styles['body-cell-success']}
-                aria-label={ariaLabels?.successfulEditLabel?.(column)}
-                role="img"
                 onMouseDown={e => {
                   // Prevent the editor's Button blur event to be fired when clicking the success icon.
                   // This prevents unfocusing the button and triggers the `TableTdElement` onClick event which initiates the edit mode.
                   e.preventDefault();
                 }}
               >
-                <Icon name="status-positive" variant="success" />
+                <Icon name="status-positive" variant="success" ariaLabel={ariaLabels?.successfulEditLabel?.(column)} />
               </span>
               <InternalLiveRegion tagName="span" hidden={true}>
                 {i18n('ariaLabels.successfulEditLabel', ariaLabels?.successfulEditLabel?.(column))}

diff --git a/src/table/header-cell/index.tsx b/src/table/header-cell/index.tsx
@@ -182,12 +182,11 @@ export function TableHeaderCell<ItemType>({
         >
           {column.header}
           {isEditable ? (
-            <span
-              className={styles['edit-icon']}
-              role="img"
-              aria-label={i18n('columnDefinitions.editConfig.editIconAriaLabel', column.editConfig?.editIconAriaLabel)}
-            >
-              <InternalIcon name="edit" />
+            <span className={styles['edit-icon']}>
+              <InternalIcon
+                name="edit"
+                ariaLabel={i18n('columnDefinitions.editConfig.editIconAriaLabel', column.editConfig?.editIconAriaLabel)}
+              />
             </span>
           ) : null}
         </div>