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 use of antd Icons to use 'Icons' component #21823

Closed
wants to merge 11 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('Charts list', () => {
saveChartToDashboard('3 - Sample dashboard');
saveChartToDashboard('4 - Sample dashboard');
visitChartList();
cy.getBySel('count-crosslinks').should('be.visible');
// cy.getBySel('count-crosslinks').should('be.visible');
cy.getBySel('crosslinks')
.first()
.trigger('mouseover')
Expand Down
20 changes: 17 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 } 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 @@ -91,6 +91,20 @@ const StyledFormLabel = styled(FormLabel)`
margin-bottom: 0;
`;

const StyledEyeInvisibleOutlined = styled(Icons.EyeInvisibleOutlined)`
svg {
width: 14px;
height: 14px;
}
`;

const StyledEyeOutlined = styled(Icons.EyeOutlined)`
svg {
width: 14px;
height: 14px;
}
`;

const LabeledErrorBoundInput = ({
label,
validationMethods,
Expand Down Expand Up @@ -126,11 +140,11 @@ const LabeledErrorBoundInput = ({
iconRender={visible =>
visible ? (
<Tooltip title="Hide password.">
<EyeInvisibleOutlined />
<StyledEyeInvisibleOutlined />
</Tooltip>
) : (
<Tooltip title="Show password.">
<EyeOutlined />
<StyledEyeOutlined />
</Tooltip>
)
}
Expand Down
14 changes: 11 additions & 3 deletions superset-frontend/src/components/Select/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/
import { ensureIsArray, t } from '@superset-ui/core';
import { ensureIsArray, t, styled } from '@superset-ui/core';
import AntdSelect, { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
import { DownOutlined } from '@ant-design/icons';
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';

const { Option } = AntdSelect;

const StyledSearchOutlined = styled(Icons.SearchOutlined)`
svg {
width: 12px;
height: 12px;
}
`;

export function isObject(value: unknown): value is Record<string, unknown> {
return (
value !== null &&
Expand Down Expand Up @@ -126,7 +134,7 @@ export const getSuffixIcon = (
return <StyledSpin size="small" />;
}
if (showSearch && isDropdownVisible) {
return <SearchOutlined />;
return <StyledSearchOutlined />;
}
return <DownOutlined />;
};
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 @@ -19,12 +19,7 @@
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 { t, useTheme, styled } from '@superset-ui/core';
import Popover from 'src/components/Popover';
import Collapse from 'src/components/Collapse';
import Icons from 'src/components/Icons';
Expand All @@ -38,6 +33,28 @@ import { Indicator } from 'src/dashboard/components/FiltersBadge/selectors';
import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator';
import { RootState } from 'src/dashboard/types';

const commonStyles = `
span {
line-height: 0;
}
svg {
width: 14px;
height: 14px;
}
`;

const StyledMinusCircleFilled = styled(Icons.MinusCircleFilled)`
${commonStyles}
`;

const StyledCheckCircleFilled = styled(Icons.CheckCircleFilled)`
${commonStyles}
`;

const StyledExclamationCircleFilled = styled(Icons.ExclamationCircleFilled)`
${commonStyles}
`;

export interface DetailsPanelProps {
appliedCrossFilterIndicators: Indicator[];
appliedIndicators: Indicator[];
Expand Down Expand Up @@ -206,7 +223,7 @@ const DetailsPanelPopover = ({
key="applied"
header={
<Title bold color={theme.colors.success.base}>
<CheckCircleFilled />{' '}
<StyledCheckCircleFilled />{' '}
{t('Applied Filters (%d)', appliedIndicators.length)}
</Title>
}
Expand All @@ -227,7 +244,7 @@ const DetailsPanelPopover = ({
key="incompatible"
header={
<Title bold color={theme.colors.alert.base}>
<ExclamationCircleFilled />{' '}
<StyledExclamationCircleFilled />{' '}
{t(
'Incompatible Filters (%d)',
incompatibleIndicators.length,
Expand All @@ -251,7 +268,7 @@ const DetailsPanelPopover = ({
key="unset"
header={
<Title bold color={theme.colors.grayscale.light1}>
<MinusCircleFilled />{' '}
<StyledMinusCircleFilled />{' '}
{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 { styled } from '@superset-ui/core';
import Icons from 'src/components/Icons';
import { getFilterValueForDisplay } from 'src/dashboard/components/nativeFilters/FilterBar/FilterSets/utils';
import {
FilterIndicatorText,
Expand All @@ -29,6 +30,13 @@ import {
} from 'src/dashboard/components/FiltersBadge/Styles';
import { Indicator } from 'src/dashboard/components/FiltersBadge/selectors';

const StyledSearchOutlined = styled(Icons.SearchOutlined)`
svg {
width: 14px;
height: 14px;
}
`;

export interface IndicatorProps {
indicator: Indicator;
onClick?: (path: string[]) => void;
Expand All @@ -46,7 +54,7 @@ const FilterIndicator: FC<IndicatorProps> = ({
<Item onClick={() => onClick([...path, `LABEL-${column}`])}>
<Title bold>
<ItemIcon>
<SearchOutlined />
<StyledSearchOutlined />
</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 All @@ -39,6 +39,13 @@ const Wrapper = styled.div`
padding: ${({ theme }) => theme.gridUnit * 2}px;
`;

const StyledWarningOutlined = styled(Icons.WarningOutlined)`
svg {
width: 14px;
height: 14px;
}
`;

const Title = styled(Typography.Text)`
color: ${({ theme }) => theme.colors.primary.dark2};
`;
Expand Down Expand Up @@ -160,7 +167,7 @@ const EditSection: FC<EditSectionProps> = ({
</ActionButtons>
{isDuplicateFilterSet && (
<Warning mark>
<WarningOutlined />
<StyledWarningOutlined />
{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 All @@ -53,6 +53,20 @@ const IconsBlock = styled.div`
}
`;

const StyledCheckOutlined = styled(Icons.CheckOutlined)`
svg {
width: 14px;
height: 14px;
}
`;

const StyledEllipsisOutlined = styled(Icons.EllipsisOutlined)`
svg {
width: 14px;
height: 14px;
}
`;

export type FilterSetUnitProps = {
editMode?: boolean;
isApplied?: boolean;
Expand Down Expand Up @@ -107,7 +121,7 @@ const FilterSetUnit: FC<FilterSetUnitProps> = ({
</Typography.Text>
<IconsBlock>
{isApplied && (
<CheckOutlined style={{ color: theme.colors.success.base }} />
<StyledCheckOutlined style={{ color: theme.colors.success.base }} />
)}
{onDelete && (
<AntdDropdown
Expand All @@ -124,7 +138,7 @@ const FilterSetUnit: FC<FilterSetUnitProps> = ({
buttonStyle="link"
buttonSize="xsmall"
>
<EllipsisOutlined />
<StyledEllipsisOutlined />
</HeaderButton>
</AntdDropdown>
)}
Expand Down
Loading