This is an automated email from the ASF dual-hosted git repository. diegopucci pushed a commit to branch geido/refactor/icons-typing-support in repository https://gitbox.apache.org/repos/asf/superset.git
commit 131451ab549fa87ffb113490dc8235358e977b5a Author: Diego Pucci <[email protected]> AuthorDate: Thu Mar 27 13:11:27 2025 +0200 refactor(Icons): Add typing support and improve structure --- .../cypress/e2e/chart_list/list.test.ts | 4 +- .../cypress/e2e/dashboard_list/list.test.ts | 4 +- .../cypress-base/cypress/support/directories.ts | 8 ++-- .../src/SqlLab/components/TableElement/index.tsx | 5 +- ...er-chart-tile.svg => big_number_chart_tile.svg} | Bin .../icons/{checkbox-half.svg => checkbox_half.svg} | Bin .../icons/{checkbox-off.svg => checkbox_off.svg} | Bin .../icons/{checkbox-on.svg => checkbox_on.svg} | Bin .../src/components/Icons/AntdEnhanced.tsx | 35 ++++++++------ .../components/Icons/{Icon.tsx => CustomIcon.tsx} | 40 +++++++++++++++- .../src/components/Icons/Icons.stories.tsx | 8 ++-- superset-frontend/src/components/Icons/index.tsx | 52 +++++---------------- .../IndeterminateCheckbox.test.tsx | 12 ++--- .../src/components/ListView/ActionsBar.tsx | 4 +- .../src/components/Radio/Radio.stories.tsx | 13 ------ .../components/menu/BackgroundStyleDropdown.tsx | 7 ++- .../VizTypeControl/VizTypeControl.test.tsx | 2 +- .../databases/DatabaseModal/index.test.tsx | 2 +- 18 files changed, 100 insertions(+), 96 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts b/superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts index 9f689c014b..0f905fa3e2 100644 --- a/superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts @@ -123,7 +123,7 @@ describe('Charts list', () => { it('should bulk select in list mode', () => { toggleBulkSelect(); cy.get('#header-toggle-all').click(); - cy.get('[aria-label="checkbox-on"]').should('have.length', 26); + cy.get('[aria-label="checkbox_on"]').should('have.length', 26); cy.getBySel('bulk-select-copy').contains('25 Selected'); cy.getBySel('bulk-select-action') .should('have.length', 2) @@ -132,7 +132,7 @@ describe('Charts list', () => { expect($btns).to.contain('Export'); }); cy.getBySel('bulk-select-deselect-all').click(); - cy.get('[aria-label="checkbox-on"]').should('have.length', 0); + cy.get('[aria-label="checkbox_on"]').should('have.length', 0); cy.getBySel('bulk-select-copy').contains('0 Selected'); cy.getBySel('bulk-select-action').should('not.exist'); }); diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts index 322306a4c6..de0f20f40e 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts @@ -88,7 +88,7 @@ describe('Dashboards list', () => { it('should bulk select in list mode', () => { toggleBulkSelect(); cy.get('#header-toggle-all').click(); - cy.get('[aria-label="checkbox-on"]').should('have.length', 6); + cy.get('[aria-label="checkbox_on"]').should('have.length', 6); cy.getBySel('bulk-select-copy').contains('5 Selected'); cy.getBySel('bulk-select-action') .should('have.length', 2) @@ -97,7 +97,7 @@ describe('Dashboards list', () => { expect($btns).to.contain('Export'); }); cy.getBySel('bulk-select-deselect-all').click(); - cy.get('[aria-label="checkbox-on"]').should('have.length', 0); + cy.get('[aria-label="checkbox_on"]').should('have.length', 0); cy.getBySel('bulk-select-copy').contains('0 Selected'); cy.getBySel('bulk-select-action').should('not.exist'); }); diff --git a/superset-frontend/cypress-base/cypress/support/directories.ts b/superset-frontend/cypress-base/cypress/support/directories.ts index 3c8e8eb367..539064b66a 100644 --- a/superset-frontend/cypress-base/cypress/support/directories.ts +++ b/superset-frontend/cypress-base/cypress/support/directories.ts @@ -294,8 +294,8 @@ export const chartListView = { }, table: { bulkSelect: { - checkboxOff: '[aria-label="checkbox-off"]', - checkboxOn: '[aria-label="checkbox-on"]', + checkboxOff: '[aria-label="checkbox_off"]', + checkboxOn: '[aria-label="checkbox_on"]', action: dataTestLocator('bulk-select-action'), }, tableList: dataTestLocator('listview-table'), @@ -413,8 +413,8 @@ export const dashboardListView = { selectedStarIcon: "[aria-label='star']", unselectedStarIcon: "[aria-label='star']", bulkSelect: { - checkboxOff: '[aria-label="checkbox-off"]', - checkboxOn: '[aria-label="checkbox-on"]', + checkboxOff: '[aria-label="checkbox_off"]', + checkboxOn: '[aria-label="checkbox_on"]', action: dataTestLocator('bulk-select-action'), }, tableRow: { diff --git a/superset-frontend/src/SqlLab/components/TableElement/index.tsx b/superset-frontend/src/SqlLab/components/TableElement/index.tsx index 8185448ba0..80fd781460 100644 --- a/superset-frontend/src/SqlLab/components/TableElement/index.tsx +++ b/superset-frontend/src/SqlLab/components/TableElement/index.tsx @@ -263,7 +263,10 @@ const TableElement = ({ table, ...props }: TableElementProps) => { className="pull-left m-l-2" tooltip={t('View keys & indexes (%s)', tableData.indexes.length)} > - <Icons.KeyOutlined iconSize="s" /> + <Icons.TableOutlined + iconSize="m" + iconColor={theme.colors.primary.dark2} + /> </IconTooltip> } /> diff --git a/superset-frontend/src/assets/images/icons/big-number-chart-tile.svg b/superset-frontend/src/assets/images/icons/big_number_chart_tile.svg similarity index 100% rename from superset-frontend/src/assets/images/icons/big-number-chart-tile.svg rename to superset-frontend/src/assets/images/icons/big_number_chart_tile.svg diff --git a/superset-frontend/src/assets/images/icons/checkbox-half.svg b/superset-frontend/src/assets/images/icons/checkbox_half.svg similarity index 100% rename from superset-frontend/src/assets/images/icons/checkbox-half.svg rename to superset-frontend/src/assets/images/icons/checkbox_half.svg diff --git a/superset-frontend/src/assets/images/icons/checkbox-off.svg b/superset-frontend/src/assets/images/icons/checkbox_off.svg similarity index 100% rename from superset-frontend/src/assets/images/icons/checkbox-off.svg rename to superset-frontend/src/assets/images/icons/checkbox_off.svg diff --git a/superset-frontend/src/assets/images/icons/checkbox-on.svg b/superset-frontend/src/assets/images/icons/checkbox_on.svg similarity index 100% rename from superset-frontend/src/assets/images/icons/checkbox-on.svg rename to superset-frontend/src/assets/images/icons/checkbox_on.svg diff --git a/superset-frontend/src/components/Icons/AntdEnhanced.tsx b/superset-frontend/src/components/Icons/AntdEnhanced.tsx index 42c9ad7463..8501c7acc8 100644 --- a/superset-frontend/src/components/Icons/AntdEnhanced.tsx +++ b/superset-frontend/src/components/Icons/AntdEnhanced.tsx @@ -111,6 +111,7 @@ import { UnorderedListOutlined, WarningOutlined, } from '@ant-design/icons'; +import { FC } from 'react'; import { IconType } from './types'; import { BaseIconComponent } from './BaseIcon'; @@ -205,19 +206,25 @@ const AntdIcons = { FilterOutlined, UnorderedListOutlined, WarningOutlined, -}; +} as const; -const AntdEnhancedIcons = Object.keys(AntdIcons) - .filter(k => !k.includes('TwoTone')) - .map(k => ({ - [k]: (props: IconType) => ( - <BaseIconComponent - component={AntdIcons[k as keyof typeof AntdIcons]} - fileName={k} - {...props} - /> - ), - })) - .reduce((l, r) => ({ ...l, ...r })); +type AntdIconNames = keyof typeof AntdIcons; -export default AntdEnhancedIcons; +export const antdEnhancedIcons: Record< + AntdIconNames, + FC<IconType> +> = Object.keys(AntdIcons) + .filter(k => !k.includes('TwoTone')) // Filter unwanted keys if needed + .reduce( + (acc, key) => { + acc[key as AntdIconNames] = (props: IconType) => ( + <BaseIconComponent + component={AntdIcons[key as AntdIconNames]} + fileName={key} + {...props} + /> + ); + return acc; + }, + {} as Record<AntdIconNames, FC<IconType>>, + ); diff --git a/superset-frontend/src/components/Icons/Icon.tsx b/superset-frontend/src/components/Icons/CustomIcon.tsx similarity index 69% rename from superset-frontend/src/components/Icons/Icon.tsx rename to superset-frontend/src/components/Icons/CustomIcon.tsx index 3c27ca6470..36833e6516 100644 --- a/superset-frontend/src/components/Icons/Icon.tsx +++ b/superset-frontend/src/components/Icons/CustomIcon.tsx @@ -22,6 +22,34 @@ import TransparentIcon from 'src/assets/images/icons/transparent.svg'; import { IconType } from './types'; import { BaseIconComponent } from './BaseIcon'; +const customIcons = [ + 'Ballot', + 'BigNumberChartTile', + 'Binoculars', + 'Category', + 'Certified', + 'CheckboxHalf', + 'CheckboxOff', + 'CheckboxOn', + 'CircleSolid', + 'Drag', + 'ErrorSolidSmallRed', + 'Error', + 'Full', + 'Layers', + 'Queued', + 'Redo', + 'Running', + 'Slack', + 'Square', + 'SortAsc', + 'SortDesc', + 'Sort', + 'Transparent', + 'TriangleDown', + 'Undo', +] as const; + export const Icon = (props: IconType) => { const [, setLoaded] = useState(false); const ImportedSVG = useRef<FC<SVGProps<SVGSVGElement>>>(); @@ -51,4 +79,14 @@ export const Icon = (props: IconType) => { ); }; -export default Icon; +export const iconOverrides = Object.fromEntries( + customIcons.map(customIcon => { + const fileName = customIcon + .replace(/([a-z0-9])([A-Z])/g, '$1_$2') + .toLowerCase(); + return [ + customIcon, + (props: IconType) => <Icon customIcons fileName={fileName} {...props} />, + ]; + }), +) as Record<(typeof customIcons)[number], FC<IconType>>; diff --git a/superset-frontend/src/components/Icons/Icons.stories.tsx b/superset-frontend/src/components/Icons/Icons.stories.tsx index 017be29521..3c4f4ab3d8 100644 --- a/superset-frontend/src/components/Icons/Icons.stories.tsx +++ b/superset-frontend/src/components/Icons/Icons.stories.tsx @@ -19,13 +19,13 @@ import { useState } from 'react'; import { styled, supersetTheme } from '@superset-ui/core'; import { Input } from 'antd-v5'; -import Icons from '.'; +import Icons, { IconNameType } from '.'; import IconType from './types'; -import Icon from './Icon'; +import { BaseIconComponent } from './BaseIcon'; export default { title: 'Icons', - component: Icon, + component: BaseIconComponent, }; const palette: Record<string, string | null> = { Default: null }; @@ -84,7 +84,7 @@ export const InteractiveIcons = ({ /> <IconSet> {filteredIcons.map(k => { - const IconComponent = Icons[k]; + const IconComponent = Icons[k as IconNameType]; return ( <IconBlock key={k}> <IconComponent {...rest} /> diff --git a/superset-frontend/src/components/Icons/index.tsx b/superset-frontend/src/components/Icons/index.tsx index eb3fa51605..ce00c8a7d3 100644 --- a/superset-frontend/src/components/Icons/index.tsx +++ b/superset-frontend/src/components/Icons/index.tsx @@ -18,51 +18,21 @@ */ import { FC } from 'react'; -import { startCase } from 'lodash'; -import AntdEnhancedIcons from './AntdEnhanced'; -import Icon from './Icon'; +import { antdEnhancedIcons } from './AntdEnhanced'; +import { iconOverrides } from './CustomIcon'; import IconType from './types'; -const IconFileNames = [ - // to keep custom - 'ballot', - 'big-number-chart-tile', - 'binoculars', - 'category', - 'certified', - 'checkbox-half', - 'checkbox-off', - 'checkbox-on', - 'circle_solid', - 'drag', - 'error_solid_small_red', - 'error', - 'full', - 'layers', - 'queued', - 'redo', - 'running', - 'slack', - 'square', - 'sort_asc', - 'sort_desc', - 'sort', - 'transparent', - 'triangle_down', - 'undo', -]; +export type { IconType }; -const iconOverrides: Record<string, FC<IconType>> = {}; -IconFileNames.forEach(fileName => { - const keyName = startCase(fileName).replace(/ /g, ''); - iconOverrides[keyName] = (props: IconType) => ( - <Icon customIcons fileName={fileName} {...props} /> - ); -}); +export type IconNameType = + | keyof typeof antdEnhancedIcons + | keyof typeof iconOverrides; -export type { IconType }; +type IconComponentType = Record<IconNameType, FC<IconType>>; -export default { - ...AntdEnhancedIcons, +const Icons: IconComponentType = { + ...antdEnhancedIcons, ...iconOverrides, }; + +export default Icons; diff --git a/superset-frontend/src/components/IndeterminateCheckbox/IndeterminateCheckbox.test.tsx b/superset-frontend/src/components/IndeterminateCheckbox/IndeterminateCheckbox.test.tsx index c60c146c18..9879841b08 100644 --- a/superset-frontend/src/components/IndeterminateCheckbox/IndeterminateCheckbox.test.tsx +++ b/superset-frontend/src/components/IndeterminateCheckbox/IndeterminateCheckbox.test.tsx @@ -51,7 +51,7 @@ test('should render the checkbox', async () => { expect(screen.getByRole('checkbox')).toBeInTheDocument(); }); -test('should render the checkbox-half icon', async () => { +test('should render the checkbox_half icon', async () => { const indeterminateProps = { ...mockedProps, indeterminate: true, @@ -60,24 +60,24 @@ test('should render the checkbox-half icon', async () => { expect(screen.getByRole('img')).toBeInTheDocument(); expect(screen.getByRole('img')).toHaveAttribute( 'aria-label', - 'checkbox-half', + 'checkbox_half', ); }); -test('should render the checkbox-off icon', async () => { +test('should render the checkbox_off icon', async () => { await asyncRender(); expect(screen.getByRole('img')).toBeInTheDocument(); - expect(screen.getByRole('img')).toHaveAttribute('aria-label', 'checkbox-off'); + expect(screen.getByRole('img')).toHaveAttribute('aria-label', 'checkbox_off'); }); -test('should render the checkbox-on icon', async () => { +test('should render the checkbox_on icon', async () => { const checkboxOnProps = { ...mockedProps, checked: true, }; await asyncRender(checkboxOnProps); expect(screen.getByRole('img')).toBeInTheDocument(); - expect(screen.getByRole('img')).toHaveAttribute('aria-label', 'checkbox-on'); + expect(screen.getByRole('img')).toHaveAttribute('aria-label', 'checkbox_on'); }); test('should call the onChange', async () => { diff --git a/superset-frontend/src/components/ListView/ActionsBar.tsx b/superset-frontend/src/components/ListView/ActionsBar.tsx index d9d6170133..fc5357b81e 100644 --- a/superset-frontend/src/components/ListView/ActionsBar.tsx +++ b/superset-frontend/src/components/ListView/ActionsBar.tsx @@ -19,7 +19,7 @@ import { ReactElement } from 'react'; import { styled } from '@superset-ui/core'; import { Tooltip, TooltipPlacement } from 'src/components/Tooltip'; -import Icons from 'src/components/Icons'; +import Icons, { IconNameType } from 'src/components/Icons'; export type ActionProps = { label: string; @@ -56,7 +56,7 @@ export default function ActionsBar({ actions }: ActionsBarProps) { return ( <StyledActions className="actions"> {actions.map((action, index) => { - const ActionIcon = Icons[action.icon]; + const ActionIcon = Icons[action.icon as IconNameType]; if (action.tooltip) { return ( <Tooltip diff --git a/superset-frontend/src/components/Radio/Radio.stories.tsx b/superset-frontend/src/components/Radio/Radio.stories.tsx index 5155c7d23c..81e33498ae 100644 --- a/superset-frontend/src/components/Radio/Radio.stories.tsx +++ b/superset-frontend/src/components/Radio/Radio.stories.tsx @@ -121,19 +121,6 @@ RadioGroupWithOptionsStory.args = { </Space> ), }, - { - value: 2, - label: ( - <Space align="center" direction="vertical"> - <Icons.DotChartOutlined - css={css` - font-size: 18; - `} - /> - DotChart - </Space> - ), - }, { value: 3, label: ( diff --git a/superset-frontend/src/dashboard/components/menu/BackgroundStyleDropdown.tsx b/superset-frontend/src/dashboard/components/menu/BackgroundStyleDropdown.tsx index 3919181ea6..eb44ee81c0 100644 --- a/superset-frontend/src/dashboard/components/menu/BackgroundStyleDropdown.tsx +++ b/superset-frontend/src/dashboard/components/menu/BackgroundStyleDropdown.tsx @@ -57,13 +57,12 @@ const BackgroundStyleOption = styled.div` /* Create the transparent rect icon */ &.background--transparent:before { - background-image: - linear-gradient(45deg, ${theme.colors.text.label} 25%, transparent 25%), - linear-gradient( - -45deg, + background-image: linear-gradient( + 45deg, ${theme.colors.text.label} 25%, transparent 25% ), + linear-gradient(-45deg, ${theme.colors.text.label} 25%, transparent 25%), linear-gradient(45deg, transparent 75%, ${theme.colors.text.label} 75%), linear-gradient(-45deg, transparent 75%, ${theme.colors.text.label} 75%); background-size: ${theme.gridUnit * 2}px ${theme.gridUnit * 2}px; 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 f427bc9f03..157f265c4c 100644 --- a/superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeControl.test.tsx @@ -121,7 +121,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/databases/DatabaseModal/index.test.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx index 4a0dd068d6..198e9f7060 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx @@ -678,7 +678,7 @@ describe('DatabaseModal', () => { }); // These are the checkbox SVGs that cover the actual checkboxes const checkboxOffSVGs = screen.getAllByRole('img', { - name: /checkbox-off/i, + name: /checkbox_off/i, }); const tooltipIcons = within(advancedTabPanel).getAllByRole('img', { name: /info-tooltip/i,
