fix(Icons): Add missing data-test and aria-label attributes to custom icons (#34809)

Co-authored-by: Claude <noreply@anthropic.com>
(cherry picked from commit 5c3c2599db)

superset-frontend/packages/superset-ui-core/src/components/Icons/AsyncIcon.tsx

@@ -25,7 +25,8 @@ import { BaseIconComponent } from './BaseIcon';
 const AsyncIcon = (props: IconType) => {
   const [, setLoaded] = useState(false);
   const ImportedSVG = useRef<FC<SVGProps<SVGSVGElement>>>();
-  const { fileName, ...restProps } = props;
+  const { fileName, customIcons, iconSize, iconColor, viewBox, ...restProps } =
+    props;
 
   useEffect(() => {
     let cancelled = false;
@@ -46,6 +47,11 @@ const AsyncIcon = (props: IconType) => {
   return (
     <BaseIconComponent
       component={ImportedSVG.current || TransparentIcon}
+      fileName={fileName}
+      customIcons={customIcons}
+      iconSize={iconSize}
+      iconColor={iconColor}
+      viewBox={viewBox}
       {...restProps}
     />
   );

superset-frontend/packages/superset-ui-core/test/components/Icons/AsyncIcon.integration.test.tsx

@@ -0,0 +1,122 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import '@testing-library/jest-dom';
+import { render, fireEvent } from '@testing-library/react';
+import { SupersetTheme, ThemeProvider } from '@superset-ui/core';
+
+// CRITICAL: Don't import from the mocked path - import directly to avoid global mocks
+import AsyncIcon from '../../../src/components/Icons/AsyncIcon';
+
+// Mock only the SVG import to prevent dynamic import issues
+jest.mock(
+  '!!@svgr/webpack!../../../src/assets/images/icons/slack.svg',
+  () => {
+    const MockSlackSVG = (props: any) => (
+      <svg {...props} viewBox="0 0 24 24" data-testid="slack-svg">
+        <path d="M5.042 15.165a2.528 2.528 0 0 1-2.52 2.523A2.528 2.528 0 0 1 0 15.165a2.527 2.527 0 0 1 2.522-2.52h2.52v2.52z" />
+      </svg>
+    );
+    return { default: MockSlackSVG };
+  },
+  { virtual: true },
+);
+
+// Basic theme for testing
+const mockTheme: SupersetTheme = {
+  fontSize: 16,
+  sizeUnit: 4,
+} as SupersetTheme;
+
+describe('AsyncIcon Integration Tests (Real Component)', () => {
+  it('should have data-test and aria-label attributes with real component', () => {
+    const { container } = render(
+      <ThemeProvider theme={mockTheme}>
+        <AsyncIcon customIcons fileName="slack" iconSize="l" />
+      </ThemeProvider>,
+    );
+
+    // Don't wait for SVG since it's mocked - just check the span wrapper
+    const spanElement = container.querySelector('span');
+
+    // Test the ACTUAL component behavior (not the mock)
+    expect(spanElement).toHaveAttribute('aria-label', 'slack');
+    expect(spanElement).toHaveAttribute('role', 'img');
+    expect(spanElement).toHaveAttribute('data-test', 'slack');
+  });
+
+  it('should always have aria-label and data-test for testing', () => {
+    const { container } = render(
+      <ThemeProvider theme={mockTheme}>
+        <AsyncIcon customIcons fileName="slack" iconSize="l" />
+      </ThemeProvider>,
+    );
+
+    const spanElement = container.querySelector('span');
+
+    // The critical requirement: we MUST have these attributes for accessibility and testing
+    expect(spanElement).toHaveAttribute('aria-label');
+    expect(spanElement).toHaveAttribute('data-test');
+
+    // The values should be consistent
+    const ariaLabel = spanElement?.getAttribute('aria-label');
+    const dataTest = spanElement?.getAttribute('data-test');
+    expect(ariaLabel).toBe('slack');
+    expect(dataTest).toBe('slack');
+  });
+
+  it('should set role to button when onClick is provided in real component', () => {
+    const onClick = jest.fn();
+    const { container } = render(
+      <ThemeProvider theme={mockTheme}>
+        <AsyncIcon
+          customIcons
+          fileName="slack"
+          iconSize="l"
+          onClick={onClick}
+        />
+      </ThemeProvider>,
+    );
+
+    const spanElement = container.querySelector('span');
+
+    expect(spanElement).toHaveAttribute('role', 'button');
+    expect(spanElement).toHaveAttribute('aria-label', 'slack');
+    expect(spanElement).toHaveAttribute('data-test', 'slack');
+
+    // Verify onClick handler actually works
+    fireEvent.click(spanElement!);
+    expect(onClick).toHaveBeenCalledTimes(1);
+  });
+
+  it('should handle complex fileName patterns like BaseIcon', () => {
+    const { container } = render(
+      <ThemeProvider theme={mockTheme}>
+        <AsyncIcon customIcons fileName="slack_notification" iconSize="l" />
+      </ThemeProvider>,
+    );
+
+    const spanElement = container.querySelector('span');
+
+    // Should follow BaseIcon's genAriaLabel logic:
+    // fileName="slack_notification" -> name="slack-notification" -> "slack-notification" (not just "slack")
+    expect(spanElement).toHaveAttribute('aria-label', 'slack-notification');
+    expect(spanElement).toHaveAttribute('data-test', 'slack-notification');
+  });
+});

superset-frontend/spec/helpers/shim.tsx

@@ -92,18 +92,27 @@ jest.mock('@superset-ui/core/components/Icons/AsyncIcon', () => ({
     fileName,
     role,
     'aria-label': ariaLabel,
+    onClick,
     ...rest
   }: {
     fileName: string;
-    role: string;
-    'aria-label': AriaAttributes['aria-label'];
-  }) => (
-    <span
-      role={role ?? 'img'}
-      aria-label={ariaLabel || fileName.replace('_', '-')}
-      {...rest}
-    />
-  ),
+    role?: string;
+    'aria-label'?: AriaAttributes['aria-label'];
+    onClick?: () => void;
+  }) => {
+    // Simple mock that provides the essential attributes for testing
+    const label = ariaLabel || fileName?.replace(/_/g, '-').toLowerCase() || '';
+    return (
+      // eslint-disable-next-line jsx-a11y/no-static-element-interactions
+      <span
+        role={role || (onClick ? 'button' : 'img')}
+        aria-label={label}
+        data-test={label}
+        onClick={onClick}
+        {...rest}
+      />
+    );
+  },
   StyledIcon: ({
     component: Component,
     role,

superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeControl.test.tsx

@@ -124,7 +124,7 @@ describe('VizTypeControl', () => {
     };
     await waitForRenderWrapper(props);
     expect(screen.getByLabelText('table')).toBeVisible();
-    expect(screen.getByLabelText('big-number_chart_tile')).toBeVisible();
+    expect(screen.getByLabelText('big-number-chart-tile')).toBeVisible();
     expect(screen.getByLabelText('pie-chart')).toBeVisible();
     expect(screen.getByLabelText('bar-chart')).toBeVisible();
     expect(screen.getByLabelText('area-chart')).toBeVisible();

superset-frontend/src/features/alerts/components/RecipientIcon.test.tsx

@@ -48,4 +48,23 @@ describe('RecipientIcon', () => {
     const icons = screen.queryByLabelText(/.*/);
     expect(icons).not.toBeInTheDocument();
   });
+
+  it('All recipient types should have consistent accessibility attributes', () => {
+    const types = [
+      NotificationMethodOption.Email,
+      NotificationMethodOption.Slack,
+      NotificationMethodOption.SlackV2,
+    ];
+
+    types.forEach(type => {
+      const { container } = render(<RecipientIcon type={type} />);
+      const iconElement = container.querySelector('[role="img"]');
+
+      // Every icon should exist and have these attributes for accessibility and testing
+      expect(iconElement).toBeInTheDocument();
+      expect(iconElement).toHaveAttribute('aria-label');
+      expect(iconElement).toHaveAttribute('data-test');
+      expect(iconElement).toHaveAttribute('role', 'img');
+    });
+  });
 });