Skip to content

Commit

Permalink
chore: Convert direct uses of antd icons to 'Icons' component (#22516)
Browse files Browse the repository at this point in the history
  • Loading branch information
codyml authored Jan 23, 2023
1 parent 3084763 commit 2a30bbc
Show file tree
Hide file tree
Showing 19 changed files with 125 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ import DatasourceEditor from 'src/components/Datasource/DatasourceEditor';
import mockDatasource from 'spec/fixtures/mockDatasource';
import * as featureFlags from 'src/featureFlags';

jest.mock('src/components/Icons/Icon', () => ({ fileName, role, ...rest }) => (
<span
role={role ?? 'img'}
aria-label={fileName.replace('_', '-')}
{...rest}
/>
));
jest.mock('src/components/Icons/Icon', () => ({
__esModule: true,
default: ({ fileName, role, ...rest }) => (
<span
role={role ?? 'img'}
aria-label={fileName.replace('_', '-')}
{...rest}
/>
),
StyledIcon: () => <span />,
}));

const props = {
datasource: mockDatasource['7__table'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ describe('LabeledErrorBoundInput', () => {
defaultProps.visibilityToggle = true;
render(<LabeledErrorBoundInput {...defaultProps} />);

expect(await screen.findByRole('img', { name: /eye/i })).toBeVisible();
expect(await screen.findByTestId('icon-eye')).toBeVisible();
});

it('becomes a password input if props.name === password (backwards compatibility)', async () => {
defaultProps.name = 'password';
render(<LabeledErrorBoundInput {...defaultProps} />);

expect(await screen.findByRole('img', { name: /eye/i })).toBeVisible();
expect(await screen.findByTestId('icon-eye')).toBeVisible();
});
});
16 changes: 13 additions & 3 deletions superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
*/
import React from 'react';
import { Input, Tooltip } from 'antd';
import { EyeInvisibleOutlined, EyeOutlined } from '@ant-design/icons';
import { styled, css, SupersetTheme, t } from '@superset-ui/core';
import InfoTooltip from 'src/components/InfoTooltip';
import Icons from 'src/components/Icons';
import errorIcon from 'src/assets/images/icons/error.svg';
import FormItem from './FormItem';
import FormLabel from './FormLabel';
Expand Down Expand Up @@ -92,6 +92,12 @@ const StyledFormLabel = styled(FormLabel)`
margin-bottom: 0;
`;

const iconReset = css`
&.anticon > * {
line-height: 0;
}
`;

const LabeledErrorBoundInput = ({
label,
validationMethods,
Expand Down Expand Up @@ -128,11 +134,15 @@ const LabeledErrorBoundInput = ({
iconRender={visible =>
visible ? (
<Tooltip title={t('Hide password.')}>
<EyeInvisibleOutlined />
<Icons.EyeInvisibleOutlined iconSize="m" css={iconReset} />
</Tooltip>
) : (
<Tooltip title={t('Show password.')}>
<EyeOutlined />
<Icons.EyeOutlined
iconSize="m"
css={iconReset}
data-test="icon-eye"
/>
</Tooltip>
)
}
Expand Down
6 changes: 5 additions & 1 deletion superset-frontend/src/components/ListView/ListView.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ import Pagination from 'src/components/Pagination/Wrapper';

import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';

jest.mock('src/components/Icons/Icon', () => () => <span />);
jest.mock('src/components/Icons/Icon', () => ({
__esModule: true,
default: () => <span />,
StyledIcon: () => <span />,
}));

function makeMockLocation(query) {
const queryStr = encodeURIComponent(query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import HeaderReportDropdown, { HeaderReportProps } from '.';

let isFeatureEnabledMock: jest.MockInstance<boolean, [string]>;

jest.mock('src/components/Icons/Icon', () => () => <span />);
jest.mock('src/components/Icons/Icon', () => ({
__esModule: true,
default: () => <span />,
StyledIcon: () => <span />,
}));

const createProps = () => ({
dashboardId: 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ fetchMock.get(REPORT_ENDPOINT, {});

const NOOP = () => {};

jest.mock('src/components/Icons/Icon', () => () => <span />);
jest.mock('src/components/Icons/Icon', () => ({
__esModule: true,
default: () => <span />,
StyledIcon: () => <span />,
}));

const defaultProps = {
addDangerToast: NOOP,
Expand Down
6 changes: 3 additions & 3 deletions superset-frontend/src/components/Select/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import { ensureIsArray, t } from '@superset-ui/core';
import AntdSelect, { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
import React, { ReactElement, RefObject } from 'react';
import { DownOutlined, SearchOutlined } from '@ant-design/icons';
import Icons from 'src/components/Icons';
import { StyledHelperText, StyledLoadingText, StyledSpin } from './styles';
import { LabeledValue, RawValue, SelectOptionsType, V } from './types';

Expand Down Expand Up @@ -132,9 +132,9 @@ export const getSuffixIcon = (
return <StyledSpin size="small" />;
}
if (showSearch && isDropdownVisible) {
return <SearchOutlined />;
return <Icons.SearchOutlined iconSize="s" />;
}
return <DownOutlined />;
return <Icons.DownOutlined iconSize="s" />;
};

export const dropDownRenderHelper = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,11 @@ test('Should render "appliedCrossFilterIndicators"', () => {
userEvent.click(screen.getByTestId('details-panel-content'));
expect(screen.getByText('Applied Cross Filters (1)')).toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'search Clinical Stage' }),
screen.getByRole('button', { name: 'Clinical Stage' }),
).toBeInTheDocument();

expect(props.onHighlightFilterSource).toBeCalledTimes(0);
userEvent.click(
screen.getByRole('button', { name: 'search Clinical Stage' }),
);
userEvent.click(screen.getByRole('button', { name: 'Clinical Stage' }));
expect(props.onHighlightFilterSource).toBeCalledTimes(1);
expect(props.onHighlightFilterSource).toBeCalledWith([
'ROOT_ID',
Expand All @@ -135,12 +133,10 @@ test('Should render "appliedIndicators"', () => {

userEvent.click(screen.getByTestId('details-panel-content'));
expect(screen.getByText('Applied Filters (1)')).toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'search Country' }),
).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Country' })).toBeInTheDocument();

expect(props.onHighlightFilterSource).toBeCalledTimes(0);
userEvent.click(screen.getByRole('button', { name: 'search Country' }));
userEvent.click(screen.getByRole('button', { name: 'Country' }));
expect(props.onHighlightFilterSource).toBeCalledTimes(1);
expect(props.onHighlightFilterSource).toBeCalledWith([
'ROOT_ID',
Expand Down Expand Up @@ -168,12 +164,12 @@ test('Should render "incompatibleIndicators"', () => {
userEvent.click(screen.getByTestId('details-panel-content'));
expect(screen.getByText('Incompatible Filters (1)')).toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'search Vaccine Approach Copy' }),
screen.getByRole('button', { name: 'Vaccine Approach Copy' }),
).toBeInTheDocument();

expect(props.onHighlightFilterSource).toBeCalledTimes(0);
userEvent.click(
screen.getByRole('button', { name: 'search Vaccine Approach Copy' }),
screen.getByRole('button', { name: 'Vaccine Approach Copy' }),
);
expect(props.onHighlightFilterSource).toBeCalledTimes(1);
expect(props.onHighlightFilterSource).toBeCalledWith([
Expand Down Expand Up @@ -202,13 +198,11 @@ test('Should render "unsetIndicators"', () => {
userEvent.click(screen.getByTestId('details-panel-content'));
expect(screen.getByText('Unset Filters (1)')).toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
screen.getByRole('button', { name: 'Vaccine Approach' }),
).toBeInTheDocument();

expect(props.onHighlightFilterSource).toBeCalledTimes(0);
userEvent.click(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
);
userEvent.click(screen.getByRole('button', { name: 'Vaccine Approach' }));
expect(props.onHighlightFilterSource).toBeCalledTimes(1);
expect(props.onHighlightFilterSource).toBeCalledWith([
'ROOT_ID',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ import React, { useEffect, useState } from 'react';
import { useSelector } from 'react-redux';
import { Global, css } from '@emotion/react';
import { t, useTheme } from '@superset-ui/core';
import {
MinusCircleFilled,
CheckCircleFilled,
ExclamationCircleFilled,
} from '@ant-design/icons';
import Popover from 'src/components/Popover';
import Collapse from 'src/components/Collapse';
import Icons from 'src/components/Icons';
Expand All @@ -38,6 +33,12 @@ import { Indicator } from 'src/dashboard/components/FiltersBadge/selectors';
import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator';
import { RootState } from 'src/dashboard/types';

const iconReset = css`
span {
line-height: 0;
}
`;

export interface DetailsPanelProps {
appliedCrossFilterIndicators: Indicator[];
appliedIndicators: Indicator[];
Expand Down Expand Up @@ -206,7 +207,7 @@ const DetailsPanelPopover = ({
key="applied"
header={
<Title bold color={theme.colors.success.base}>
<CheckCircleFilled />{' '}
<Icons.CheckCircleFilled css={iconReset} iconSize="m" />{' '}
{t('Applied Filters (%d)', appliedIndicators.length)}
</Title>
}
Expand All @@ -227,7 +228,7 @@ const DetailsPanelPopover = ({
key="incompatible"
header={
<Title bold color={theme.colors.alert.base}>
<ExclamationCircleFilled />{' '}
<Icons.ExclamationCircleFilled css={iconReset} iconSize="m" />{' '}
{t(
'Incompatible Filters (%d)',
incompatibleIndicators.length,
Expand All @@ -251,7 +252,7 @@ const DetailsPanelPopover = ({
key="unset"
header={
<Title bold color={theme.colors.grayscale.light1}>
<MinusCircleFilled />{' '}
<Icons.MinusCircleFilled css={iconReset} iconSize="m" />{' '}
{t('Unset Filters (%d)', unsetIndicators.length)}
</Title>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,17 @@ test('Should render', () => {
render(<FilterIndicator {...props} />);

expect(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
screen.getByRole('button', { name: 'Vaccine Approach' }),
).toBeInTheDocument();
expect(screen.getByRole('img', { name: 'search' })).toBeInTheDocument();
expect(screen.getByRole('img')).toBeInTheDocument();
});

test('Should call "onClick"', () => {
const props = createProps();
render(<FilterIndicator {...props} />);

expect(props.onClick).toBeCalledTimes(0);
userEvent.click(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
);
userEvent.click(screen.getByRole('button', { name: 'Vaccine Approach' }));
expect(props.onClick).toBeCalledTimes(1);
});

Expand All @@ -66,7 +64,7 @@ test('Should render "value"', () => {

expect(
screen.getByRole('button', {
name: 'search Vaccine Approach: any, string',
name: 'Vaccine Approach: any, string',
}),
).toBeInTheDocument();
});
Expand All @@ -77,9 +75,7 @@ test('Should render with default props', () => {
render(<FilterIndicator indicator={props.indicator} />);

expect(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
screen.getByRole('button', { name: 'Vaccine Approach' }),
).toBeInTheDocument();
userEvent.click(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
);
userEvent.click(screen.getByRole('button', { name: 'Vaccine Approach' }));
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
* under the License.
*/

import { SearchOutlined } from '@ant-design/icons';
import React, { FC } from 'react';
import { css } from '@superset-ui/core';
import Icons from 'src/components/Icons';
import { getFilterValueForDisplay } from 'src/dashboard/components/nativeFilters/FilterBar/FilterSets/utils';
import {
FilterIndicatorText,
Expand Down Expand Up @@ -46,7 +47,14 @@ const FilterIndicator: FC<IndicatorProps> = ({
<Item onClick={() => onClick([...path, `LABEL-${column}`])}>
<Title bold>
<ItemIcon>
<SearchOutlined />
<Icons.SearchOutlined
iconSize="m"
css={css`
span {
vertical-align: 0;
}
`}
/>
</ItemIcon>
{name}
{resultValue ? ': ' : ''}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { Typography, AntdTooltip } from 'src/components';
import { useDispatch } from 'react-redux';
import Button from 'src/components/Button';
import { updateFilterSet } from 'src/dashboard/actions/nativeFilters';
import { WarningOutlined } from '@ant-design/icons';
import Icons from 'src/components/Icons';
import { ActionButtons } from './Footer';
import { useNativeFiltersDataMask, useFilters, useFilterSets } from '../state';
import { APPLY_FILTERS_HINT, findExistingFilterSet } from './utils';
Expand Down Expand Up @@ -160,7 +160,7 @@ const EditSection: FC<EditSectionProps> = ({
</ActionButtons>
{isDuplicateFilterSet && (
<Warning mark>
<WarningOutlined />
<Icons.WarningOutlined iconSize="m" />
{t('This filter set is identical to: "%s"', foundFilterSet?.name)}
</Warning>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const createProps = () => ({
});

function openDropdown() {
const dropdownIcon = screen.getByRole('img', { name: 'ellipsis' });
const dropdownIcon = screen.getAllByRole('img', { name: '' })[0];
userEvent.click(dropdownIcon);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
useTheme,
t,
} from '@superset-ui/core';
import { CheckOutlined, EllipsisOutlined } from '@ant-design/icons';
import Icons from 'src/components/Icons';
import Button from 'src/components/Button';
import { Tooltip } from 'src/components/Tooltip';
import FiltersHeader from './FiltersHeader';
Expand Down Expand Up @@ -107,7 +107,10 @@ const FilterSetUnit: FC<FilterSetUnitProps> = ({
</Typography.Text>
<IconsBlock>
{isApplied && (
<CheckOutlined style={{ color: theme.colors.success.base }} />
<Icons.CheckOutlined
iconSize="m"
iconColor={theme.colors.success.base}
/>
)}
{onDelete && (
<AntdDropdown
Expand All @@ -124,7 +127,7 @@ const FilterSetUnit: FC<FilterSetUnitProps> = ({
buttonStyle="link"
buttonSize="xsmall"
>
<EllipsisOutlined />
<Icons.EllipsisOutlined iconSize="m" />
</HeaderButton>
</AntdDropdown>
)}
Expand Down
Loading

0 comments on commit 2a30bbc

Please sign in to comment.