diff --git a/superset-frontend/packages/superset-ui-core/src/components/Icons/AsyncIcon.tsx b/superset-frontend/packages/superset-ui-core/src/components/Icons/AsyncIcon.tsx index 7c6aa6d2f79..68f0af35bbb 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Icons/AsyncIcon.tsx +++ b/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>>(); - 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 ( ); diff --git a/superset-frontend/packages/superset-ui-core/test/components/Icons/AsyncIcon.integration.test.tsx b/superset-frontend/packages/superset-ui-core/test/components/Icons/AsyncIcon.integration.test.tsx new file mode 100644 index 00000000000..8ecd9f8b47e --- /dev/null +++ b/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) => ( + + + + ); + 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( + + + , + ); + + // 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( + + + , + ); + + 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( + + + , + ); + + 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( + + + , + ); + + 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'); + }); +}); diff --git a/superset-frontend/spec/helpers/shim.tsx b/superset-frontend/spec/helpers/shim.tsx index 001af25cc79..80beb1ee96c 100644 --- a/superset-frontend/spec/helpers/shim.tsx +++ b/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']; - }) => ( - - ), + 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 + + ); + }, StyledIcon: ({ component: Component, role, diff --git a/superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeControl.test.tsx b/superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeControl.test.tsx index fd93c33de23..b3adc1fd1e5 100644 --- a/superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeControl.test.tsx +++ b/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(); diff --git a/superset-frontend/src/features/alerts/components/RecipientIcon.test.tsx b/superset-frontend/src/features/alerts/components/RecipientIcon.test.tsx index 55f34f94bc8..d3dc44dfa5e 100644 --- a/superset-frontend/src/features/alerts/components/RecipientIcon.test.tsx +++ b/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(); + 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'); + }); + }); });