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

feat: Dynamic imports for the Icons component #14318

Merged
merged 14 commits into from
Apr 29, 2021
19 changes: 19 additions & 0 deletions superset-frontend/images/icons/transparent.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion superset-frontend/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module.exports = {
moduleNameMapper: {
'\\.(css|less)$': '<rootDir>/spec/__mocks__/styleMock.js',
'\\.(gif|ttf|eot|png)$': '<rootDir>/spec/__mocks__/fileMock.js',
'\\.svg$': '<rootDir>/spec/__mocks__/svgrMock.js',
'\\.svg$': '<rootDir>/spec/__mocks__/svgrMock.tsx',
'^src/(.*)$': '<rootDir>/src/$1',
'^spec/(.*)$': '<rootDir>/spec/$1',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,14 @@
* specific language governing permissions and limitations
* under the License.
*/
export default 'SvgrURL';
export const ReactComponent = 'svg';

import React, { SVGProps } from 'react';

const SvgrMock = React.forwardRef<SVGSVGElement, SVGProps<SVGSVGElement>>(
(props, ref) => <svg ref={ref} {...props} />,
);

SvgrMock.displayName = 'SvgrMock';

export const ReactComponent = SvgrMock;
export default SvgrMock;
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import { sliceId } from 'spec/fixtures/mockChartQueries';
import { dashboardFilters } from 'spec/fixtures/mockDashboardFilters';
import { dashboardWithFilter } from 'spec/fixtures/mockDashboardLayout';
import Icons from 'src/components/Icons';
import { FeatureFlag } from 'src/featureFlags';

describe('FiltersBadge', () => {
Expand Down Expand Up @@ -129,7 +130,7 @@ describe('FiltersBadge', () => {
).toHaveText('1');
// to look at the shape of the wrapper use:
// console.log(wrapper.dive().debug())
expect(wrapper.dive().find('Icon[name="alert-solid"]')).toExist();
expect(wrapper.dive().find(Icons.AlertSolid)).toExist();
});
});

Expand Down Expand Up @@ -216,7 +217,7 @@ describe('FiltersBadge', () => {
expect(
wrapper.dive().find('[data-test="incompatible-filter-count"]'),
).toHaveText('1');
expect(wrapper.dive().find('Icon[name="alert-solid"]')).toExist();
expect(wrapper.dive().find(Icons.AlertSolid)).toExist();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { Menu } from 'src/common/components';
import DatasourceModal from 'src/datasource/DatasourceModal';
import ChangeDatasourceModal from 'src/datasource/ChangeDatasourceModal';
import DatasourceControl from 'src/explore/components/controls/DatasourceControl';
import Icon from 'src/components/Icon';
import Icons from 'src/components/Icons';
import { Tooltip } from 'src/components/Tooltip';

const defaultProps = {
Expand Down Expand Up @@ -119,8 +119,7 @@ describe('DatasourceControl', () => {

it('should render health check message', () => {
const wrapper = setup();
const alert = wrapper.find(Icon);
expect(alert.at(1).prop('name')).toBe('alert-solid');
expect(wrapper.find(Icons.AlertSolid)).toExist();
const tooltip = wrapper.find(Tooltip).at(0);
expect(tooltip.prop('title')).toBe(
defaultProps.datasource.health_check_message,
Expand Down
32 changes: 20 additions & 12 deletions superset-frontend/src/components/Alert/Alert.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,37 @@
* under the License.
*/
import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import Alert, { AlertProps } from 'src/components/Alert';

type AlertType = Pick<AlertProps, 'type'>;
type AlertTypeValue = AlertType[keyof AlertType];

test('renders with default props', () => {
test('renders with default props', async () => {
render(<Alert message="Message" />);

expect(screen.getByRole('alert')).toHaveTextContent('Message');
expect(screen.queryByLabelText(`info icon`)).toBeInTheDocument();
expect(screen.queryByLabelText('close icon')).toBeInTheDocument();
expect(await screen.findByLabelText(`info icon`)).toBeInTheDocument();
expect(await screen.findByLabelText('close icon')).toBeInTheDocument();
});

test('renders each type', () => {
test('renders each type', async () => {
const types: AlertTypeValue[] = ['info', 'error', 'warning', 'success'];
types.forEach(type => {
for (let i = 0; i < types.length; i += 1) {
const type = types[i];
render(<Alert type={type} message="Message" />);
expect(screen.queryByLabelText(`${type} icon`)).toBeInTheDocument();
});
// eslint-disable-next-line no-await-in-loop
expect(await screen.findByLabelText(`${type} icon`)).toBeInTheDocument();
}
});

test('renders without close button', () => {
test('renders without close button', async () => {
render(<Alert message="Message" closable={false} />);
expect(screen.queryByLabelText('close icon')).not.toBeInTheDocument();

await waitFor(() => {
expect(screen.queryByLabelText('close icon')).not.toBeInTheDocument();
});
});

test('disappear when closed', () => {
Expand All @@ -50,10 +56,12 @@ test('disappear when closed', () => {
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
});

test('renders without icon', () => {
test('renders without icon', async () => {
const type = 'info';
render(<Alert type={type} message="Message" showIcon={false} />);
expect(screen.queryByLabelText(`${type} icon`)).not.toBeInTheDocument();
await waitFor(() => {
expect(screen.queryByLabelText(`${type} icon`)).not.toBeInTheDocument();
});
});

test('renders message', () => {
Expand Down
13 changes: 7 additions & 6 deletions superset-frontend/src/components/Alert/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import {
AlertProps as AntdAlertProps,
} from 'src/common/components';
import { useTheme } from '@superset-ui/core';
import Icon, { IconName } from 'src/components/Icon';
import Icon from 'src/components/Icon';
import Icons from 'src/components/Icons';

export type AlertProps = PropsWithChildren<AntdAlertProps>;

Expand All @@ -40,23 +41,23 @@ export default function Alert(props: AlertProps) {
const { alert, error, info, success } = colors;

let baseColor = info;
let iconName: IconName = 'info-solid';
let AlertIcon = Icons.InfoSolid;
if (type === 'error') {
baseColor = error;
iconName = 'error-solid';
AlertIcon = Icons.ErrorSolid;
} else if (type === 'warning') {
baseColor = alert;
iconName = 'alert-solid';
AlertIcon = Icons.AlertSolid;
} else if (type === 'success') {
baseColor = success;
iconName = 'circle-check-solid';
AlertIcon = Icons.CircleCheckSolid;
}

return (
<AntdAlert
role="alert"
showIcon={showIcon}
icon={<Icon name={iconName} aria-label={`${type} icon`} />}
icon={<AlertIcon aria-label={`${type} icon`} />}
closeText={closable && <Icon name="x-small" aria-label="close icon" />}
css={{
padding: '6px 10px',
Expand Down
6 changes: 4 additions & 2 deletions superset-frontend/src/components/Icons/AntdEnhanced.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@

import React from 'react';
import * as AntdIcons from '@ant-design/icons/lib/icons';
import Icon from './Icon';
import { StyledIcon } from './Icon';
import IconType from './IconType';

const AntdEnhancedIcons = Object.keys(AntdIcons)
.map(k => ({
[k]: (props: IconType) => <Icon component={AntdIcons[k]} {...props} />,
[k]: (props: IconType) => (
<StyledIcon component={AntdIcons[k]} {...props} />
),
}))
.reduce((l, r) => ({ ...l, ...r }));

Expand Down
54 changes: 37 additions & 17 deletions superset-frontend/src/components/Icons/Icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,54 @@
* under the License.
*/

import React from 'react';
import React, { useEffect, useRef, useState } from 'react';
import AntdIcon from '@ant-design/icons';
import { styled } from '@superset-ui/core';
import { CustomIconComponentProps } from '@ant-design/icons/lib/components/Icon';
import { ReactComponent as TransparentIcon } from 'images/icons/transparent.svg';
import IconType from './IconType';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const EnhancedIcon = ({ iconColor, iconSize, ...rest }: IconType) => (
const AntdIconComponent = ({ iconColor, iconSize, ...rest }: IconType) => (
<AntdIcon viewBox={rest.viewBox || '0 0 24 24'} {...rest} />
);

const Icon = styled(EnhancedIcon)<IconType>`
export const StyledIcon = styled(AntdIconComponent)<IconType>`
${({ iconColor }) => iconColor && `color: ${iconColor};`};
font-size: ${({ iconSize, theme }) =>
iconSize ? `${theme.typography.sizes[iconSize]}px` : '24px'};
iconSize
? `${theme.typography.sizes[iconSize] || theme.typography.sizes.m}px`
: '24px'};
`;

export const renderIcon = (
SVGComponent:
| React.ComponentClass<
CustomIconComponentProps | React.SVGProps<SVGSVGElement>,
any
>
| React.FunctionComponent<
CustomIconComponentProps | React.SVGProps<SVGSVGElement>
>
| undefined,
props: IconType,
) => <Icon component={SVGComponent} {...props} />;
interface IconProps extends IconType {
filePath: string;
}

export const Icon = (props: IconProps) => {
const { filePath, ...iconProps } = props;
const [, setLoaded] = useState(false);
const ImportedSVG = useRef<React.FC<React.SVGProps<SVGSVGElement>>>();
const name = filePath.replace('_', '-');

useEffect(() => {
async function importIcon(): Promise<void> {
ImportedSVG.current = (
await import(
`!!@svgr/webpack?-svgo,+titleProp,+ref!images/icons/${filePath}.svg`
)
).default;
setLoaded(true);
}
importIcon();
}, [filePath, ImportedSVG]);

return (
<StyledIcon
component={ImportedSVG.current || TransparentIcon}
rusackas marked this conversation as resolved.
Show resolved Hide resolved
aria-label={name}
{...iconProps}
/>
);
};

export default Icon;
1 change: 1 addition & 0 deletions superset-frontend/src/components/Icons/IconType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import AntdIcon from '@ant-design/icons';
type AntdIconType = typeof AntdIcon.defaultProps;
type IconType = AntdIconType & {
iconColor?: string;
twoToneColor?: string;
iconSize?: 's' | 'm' | 'l' | 'xl' | 'xxl';
};

Expand Down
5 changes: 5 additions & 0 deletions superset-frontend/src/components/Icons/Icons.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ InteractiveIcons.argTypes = {
defaultValue: null,
control: { type: 'select', options: palette },
},
// @TODO twoToneColor is being ignored
twoToneColor: {
defaultValue: null,
control: { type: 'select', options: palette },
},
theme: {
table: {
disable: true,
Expand Down
Loading