Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Convert direct uses of antd icons to 'Icons' component #22516

Merged
merged 10 commits into from
Jan 23, 2023
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