From 0a28ac9de33177bfdc8843602ee9aa2e0e40e23d Mon Sep 17 00:00:00 2001 From: Gilles Dubreuil Date: Wed, 20 Jan 2021 16:48:18 +0100 Subject: [PATCH 1/6] Declarative approach using union types --- src/components/StatusIcon/StatusIcon.tsx | 142 +++++++++++++---------- 1 file changed, 81 insertions(+), 61 deletions(-) diff --git a/src/components/StatusIcon/StatusIcon.tsx b/src/components/StatusIcon/StatusIcon.tsx index c5449b9..621b6e1 100644 --- a/src/components/StatusIcon/StatusIcon.tsx +++ b/src/components/StatusIcon/StatusIcon.tsx @@ -12,8 +12,9 @@ import { global_success_color_100 as successColor, global_warning_color_100 as warningColor, global_Color_dark_200 as unknownColor, - global_danger_color_100 as dangerColor, + global_danger_color_100 as errorColor, global_info_color_100 as infoColor, + global_info_color_200 as loadingColor, } from '@patternfly/react-tokens'; import './StatusIcon.css'; @@ -27,8 +28,65 @@ export enum StatusType { Unknown = 'Unknown', } +export type StatusIconType = keyof typeof StatusType; + +type IconType = + | { + icon: typeof CheckCircleIcon; + color: typeof successColor; + } + | { + icon: typeof ExclamationTriangleIcon; + color: typeof warningColor; + } + | { + icon: typeof ExclamationCircleIcon; + color: typeof errorColor; + } + | { + icon: typeof InfoCircleIcon; + color: typeof infoColor; + } + | { + icon: typeof Spinner; + color: typeof loadingColor; + } + | { + icon: typeof QuestionCircleIcon; + color: typeof unknownColor; + }; + +type iconListType = { [key in StatusIconType]: IconType }; + +const iconList: iconListType = { + [StatusType.Ok]: { + icon: CheckCircleIcon, + color: successColor, + }, + [StatusType.Warning]: { + icon: ExclamationCircleIcon, + color: warningColor, + }, + [StatusType.Error]: { + icon: ExclamationCircleIcon, + color: errorColor, + }, + [StatusType.Info]: { + icon: InfoCircleIcon, + color: infoColor, + }, + [StatusType.Loading]: { + icon: Spinner, + color: loadingColor, + }, + [StatusType.Unknown]: { + icon: QuestionCircleIcon, + color: unknownColor, + }, +}; + export interface IStatusIconProps { - status: StatusType; + status: StatusIconType; label?: React.ReactNode; isDisabled?: boolean; className?: string; @@ -40,63 +98,25 @@ export const StatusIcon: React.FunctionComponent = ({ isDisabled = false, className = '', }: IStatusIconProps) => { - let icon: React.ReactElement | null = null; - if (status === StatusType.Ok) { - icon = ( - - ); - } - if (status === StatusType.Warning) { - icon = ( - - ); - } - if (status === StatusType.Error) { - icon = ( - - ); - } - if (status === StatusType.Info) { - icon = ( - - ); - } - if (status === StatusType.Loading) { - icon = ; - } - if (status === StatusType.Unknown) { - icon = ( - - ); - } - if (label) { - return ( - - {icon} - {label} - - ); - } - return icon; + const Icon = iconList[status].icon; + + return label ? ( + + + + + {label} + + ) : null; }; From b315a43be3ebc1ca06fe30a809dd1fedc3abfb8e Mon Sep 17 00:00:00 2001 From: Gilles Dubreuil Date: Wed, 20 Jan 2021 23:00:46 +0100 Subject: [PATCH 2/6] Restore retun icon when no label --- src/components/StatusIcon/StatusIcon.tsx | 68 +++++++++++++----------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/src/components/StatusIcon/StatusIcon.tsx b/src/components/StatusIcon/StatusIcon.tsx index 621b6e1..4e425b4 100644 --- a/src/components/StatusIcon/StatusIcon.tsx +++ b/src/components/StatusIcon/StatusIcon.tsx @@ -32,27 +32,27 @@ export type StatusIconType = keyof typeof StatusType; type IconType = | { - icon: typeof CheckCircleIcon; + Icon: typeof CheckCircleIcon; color: typeof successColor; } | { - icon: typeof ExclamationTriangleIcon; + Icon: typeof ExclamationTriangleIcon; color: typeof warningColor; } | { - icon: typeof ExclamationCircleIcon; + Icon: typeof ExclamationCircleIcon; color: typeof errorColor; } | { - icon: typeof InfoCircleIcon; + Icon: typeof InfoCircleIcon; color: typeof infoColor; } | { - icon: typeof Spinner; + Icon: typeof Spinner; color: typeof loadingColor; } | { - icon: typeof QuestionCircleIcon; + Icon: typeof QuestionCircleIcon; color: typeof unknownColor; }; @@ -60,27 +60,27 @@ type iconListType = { [key in StatusIconType]: IconType }; const iconList: iconListType = { [StatusType.Ok]: { - icon: CheckCircleIcon, + Icon: CheckCircleIcon, color: successColor, }, [StatusType.Warning]: { - icon: ExclamationCircleIcon, + Icon: ExclamationCircleIcon, color: warningColor, }, [StatusType.Error]: { - icon: ExclamationCircleIcon, + Icon: ExclamationCircleIcon, color: errorColor, }, [StatusType.Info]: { - icon: InfoCircleIcon, + Icon: InfoCircleIcon, color: infoColor, }, [StatusType.Loading]: { - icon: Spinner, + Icon: Spinner, color: loadingColor, }, [StatusType.Unknown]: { - icon: QuestionCircleIcon, + Icon: QuestionCircleIcon, color: unknownColor, }, }; @@ -98,25 +98,29 @@ export const StatusIcon: React.FunctionComponent = ({ isDisabled = false, className = '', }: IStatusIconProps) => { - const Icon = iconList[status].icon; + const Icon = iconList[status].Icon; + const icon = ( + + ); - return label ? ( - - - - - {label} - - ) : null; + if (label) { + return ( + + {icon} + {label} + + ); + } + return icon; }; From 83a53d49089fe03886f30095c0c1deaab0f34bda Mon Sep 17 00:00:00 2001 From: Gilles Dubreuil Date: Wed, 20 Jan 2021 23:01:25 +0100 Subject: [PATCH 3/6] Follow standard naming for type --- src/components/StatusIcon/StatusIcon.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/StatusIcon/StatusIcon.tsx b/src/components/StatusIcon/StatusIcon.tsx index 4e425b4..9815b13 100644 --- a/src/components/StatusIcon/StatusIcon.tsx +++ b/src/components/StatusIcon/StatusIcon.tsx @@ -56,9 +56,9 @@ type IconType = color: typeof unknownColor; }; -type iconListType = { [key in StatusIconType]: IconType }; +type IconListType = { [key in StatusIconType]: IconType }; -const iconList: iconListType = { +const iconList: IconListType = { [StatusType.Ok]: { Icon: CheckCircleIcon, color: successColor, From a4478151b9ffd0b9429e10a49480eb2b4d238b40 Mon Sep 17 00:00:00 2001 From: Gilles Dubreuil Date: Thu, 21 Jan 2021 18:49:35 +0100 Subject: [PATCH 4/6] Simply use IconListType only --- src/components/StatusIcon/StatusIcon.tsx | 36 ++++++------------------ 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/src/components/StatusIcon/StatusIcon.tsx b/src/components/StatusIcon/StatusIcon.tsx index 9815b13..08b931e 100644 --- a/src/components/StatusIcon/StatusIcon.tsx +++ b/src/components/StatusIcon/StatusIcon.tsx @@ -7,6 +7,8 @@ import { InfoCircleIcon, QuestionCircleIcon, } from '@patternfly/react-icons'; +import { SpinnerProps } from '@patternfly/react-core/dist/esm/components'; +import { SVGIconProps } from '@patternfly/react-icons/dist/esm/createIcon'; import { global_disabled_color_200 as disabledColor, global_success_color_100 as successColor, @@ -30,34 +32,12 @@ export enum StatusType { export type StatusIconType = keyof typeof StatusType; -type IconType = - | { - Icon: typeof CheckCircleIcon; - color: typeof successColor; - } - | { - Icon: typeof ExclamationTriangleIcon; - color: typeof warningColor; - } - | { - Icon: typeof ExclamationCircleIcon; - color: typeof errorColor; - } - | { - Icon: typeof InfoCircleIcon; - color: typeof infoColor; - } - | { - Icon: typeof Spinner; - color: typeof loadingColor; - } - | { - Icon: typeof QuestionCircleIcon; - color: typeof unknownColor; - }; - -type IconListType = { [key in StatusIconType]: IconType }; - +type IconListType = { + [key in StatusType]: { + Icon: React.ComponentClass | React.FunctionComponent; + color: { name: string; value: string; var: string }; + }; +}; const iconList: IconListType = { [StatusType.Ok]: { Icon: CheckCircleIcon, From 4e50d92301bc6a75ca38a905c042f63af2b2f932 Mon Sep 17 00:00:00 2001 From: Gilles Dubreuil Date: Thu, 21 Jan 2021 19:00:35 +0100 Subject: [PATCH 5/6] Fix import and exlamation icons assignation --- src/components/StatusIcon/StatusIcon.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/StatusIcon/StatusIcon.tsx b/src/components/StatusIcon/StatusIcon.tsx index 08b931e..64045c6 100644 --- a/src/components/StatusIcon/StatusIcon.tsx +++ b/src/components/StatusIcon/StatusIcon.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { Flex, FlexItem, Spinner } from '@patternfly/react-core'; +import { Flex, FlexItem, Spinner, SpinnerProps } from '@patternfly/react-core'; import { CheckCircleIcon, ExclamationTriangleIcon, @@ -7,8 +7,7 @@ import { InfoCircleIcon, QuestionCircleIcon, } from '@patternfly/react-icons'; -import { SpinnerProps } from '@patternfly/react-core/dist/esm/components'; -import { SVGIconProps } from '@patternfly/react-icons/dist/esm/createIcon'; +import { SVGIconProps } from '@patternfly/react-icons/dist/js/createIcon'; import { global_disabled_color_200 as disabledColor, global_success_color_100 as successColor, @@ -44,7 +43,7 @@ const iconList: IconListType = { color: successColor, }, [StatusType.Warning]: { - Icon: ExclamationCircleIcon, + Icon: ExclamationTriangleIcon, color: warningColor, }, [StatusType.Error]: { From 89d7d3ccb181eee67294198339014caf2e267dda Mon Sep 17 00:00:00 2001 From: Gilles Dubreuil Date: Thu, 21 Jan 2021 19:22:12 +0100 Subject: [PATCH 6/6] Remove string enums --- .../StatusIcon/StatusIcon.stories.mdx | 38 +++++++-------- src/components/StatusIcon/StatusIcon.test.tsx | 46 +++++++++---------- src/components/StatusIcon/StatusIcon.tsx | 29 ++++-------- 3 files changed, 51 insertions(+), 62 deletions(-) diff --git a/src/components/StatusIcon/StatusIcon.stories.mdx b/src/components/StatusIcon/StatusIcon.stories.mdx index f0c22da..296e3dc 100644 --- a/src/components/StatusIcon/StatusIcon.stories.mdx +++ b/src/components/StatusIcon/StatusIcon.stories.mdx @@ -1,5 +1,5 @@ import { Meta, Story, Canvas, ArgsTable } from '@storybook/addon-docs/blocks'; -import { StatusIcon, StatusType } from './StatusIcon'; +import { StatusIcon } from './StatusIcon'; import GithubLink from '../../../.storybook/helpers/GithubLink'; @@ -15,17 +15,17 @@ WarningTriangleIcon and ExclamationCircleIcon that automatically sets the right - +
- +
- +
- +
- +
- + @@ -33,17 +33,17 @@ WarningTriangleIcon and ExclamationCircleIcon that automatically sets the right - +
- +
- +
- +
- +
- + @@ -51,12 +51,12 @@ WarningTriangleIcon and ExclamationCircleIcon that automatically sets the right - - - - - - + + + + + + diff --git a/src/components/StatusIcon/StatusIcon.test.tsx b/src/components/StatusIcon/StatusIcon.test.tsx index 046b6d7..f3d55da 100644 --- a/src/components/StatusIcon/StatusIcon.test.tsx +++ b/src/components/StatusIcon/StatusIcon.test.tsx @@ -9,7 +9,7 @@ import { global_danger_color_100 as dangerColor, global_info_color_100 as infoColor, } from '@patternfly/react-tokens'; -import { StatusIcon, StatusType, IStatusIconProps } from './StatusIcon'; +import { StatusIcon, IStatusIconProps } from './StatusIcon'; const checkColor = (props: IStatusIconProps, color: string) => { const { container } = render(); @@ -32,85 +32,85 @@ const checkText = (props: IStatusIconProps, text: string) => { describe('StatusIcon', () => { describe('Ok status', () => { it('should have label if present', () => { - checkText({ status: StatusType.Ok, label: 'Ready' }, 'Ready'); + checkText({ status: 'Ok', label: 'Ready' }, 'Ready'); }); it('should have correct color', () => { - checkColor({ status: StatusType.Ok }, successColor.value); + checkColor({ status: 'Ok' }, successColor.value); }); it('should have disabled color if disabled', () => { - checkColor({ status: StatusType.Ok, isDisabled: true }, disabledColor.value); + checkColor({ status: 'Ok', isDisabled: true }, disabledColor.value); }); it('should pass down a given className', () => { - checkClass({ status: StatusType.Ok, className: 'foo' }, 'foo', 'svg'); + checkClass({ status: 'Ok', className: 'foo' }, 'foo', 'svg'); }); }); describe('Warning status', () => { it('should have label if present', () => { - checkText({ status: StatusType.Warning, label: 'Warning' }, 'Warning'); + checkText({ status: 'Warning', label: 'Warning' }, 'Warning'); }); it('should have correct color', () => { - checkColor({ status: StatusType.Warning }, warningColor.value); + checkColor({ status: 'Warning' }, warningColor.value); }); it('should have disabled color if disabled', () => { - checkColor({ status: StatusType.Warning, isDisabled: true }, disabledColor.value); + checkColor({ status: 'Warning', isDisabled: true }, disabledColor.value); }); it('should pass down a given className', () => { - checkClass({ status: StatusType.Warning, className: 'foo' }, 'foo', 'svg'); + checkClass({ status: 'Warning', className: 'foo' }, 'foo', 'svg'); }); }); describe('Error status', () => { it('should have label if present', () => { - checkText({ status: StatusType.Error, label: 'Error' }, 'Error'); + checkText({ status: 'Error', label: 'Error' }, 'Error'); }); it('should have correct color', () => { - checkColor({ status: StatusType.Error }, dangerColor.value); + checkColor({ status: 'Error' }, dangerColor.value); }); it('should have disabled color if disabled', () => { - checkColor({ status: StatusType.Error, isDisabled: true }, disabledColor.value); + checkColor({ status: 'Error', isDisabled: true }, disabledColor.value); }); it('should pass down a given className', () => { - checkClass({ status: StatusType.Error, className: 'foo' }, 'foo', 'svg'); + checkClass({ status: 'Error', className: 'foo' }, 'foo', 'svg'); }); }); describe('Info status', () => { it('should have label if present', () => { - checkText({ status: StatusType.Info, label: 'Info' }, 'Info'); + checkText({ status: 'Info', label: 'Info' }, 'Info'); }); it('should have correct color', () => { - checkColor({ status: StatusType.Info }, infoColor.value); + checkColor({ status: 'Info' }, infoColor.value); }); it('should have disabled color if disabled', () => { - checkColor({ status: StatusType.Info, isDisabled: true }, disabledColor.value); + checkColor({ status: 'Info', isDisabled: true }, disabledColor.value); }); it('should pass down a given className', () => { - checkClass({ status: StatusType.Info, className: 'foo' }, 'foo', 'svg'); + checkClass({ status: 'Info', className: 'foo' }, 'foo', 'svg'); }); }); describe('Loading status', () => { it('should have label if present', () => { - checkText({ status: StatusType.Loading, label: 'Loading' }, 'Loading'); + checkText({ status: 'Loading', label: 'Loading' }, 'Loading'); }); it('should pass down a given className', () => { - checkClass({ status: StatusType.Loading, className: 'foo' }, 'foo', 'span'); + checkClass({ status: 'Loading', className: 'foo' }, 'foo', 'span'); }); }); describe('Unknown status', () => { it('should have label if present', () => { - checkText({ status: StatusType.Unknown, label: 'Unknown' }, 'Unknown'); + checkText({ status: 'Unknown', label: 'Unknown' }, 'Unknown'); }); it('should have correct color', () => { - checkColor({ status: StatusType.Unknown }, unknownColor.value); + checkColor({ status: 'Unknown' }, unknownColor.value); }); it('should have disabled color if disabled', () => { - checkColor({ status: StatusType.Unknown, isDisabled: true }, disabledColor.value); + checkColor({ status: 'Unknown', isDisabled: true }, disabledColor.value); }); it('should pass down a given className', () => { - checkClass({ status: StatusType.Unknown, className: 'foo' }, 'foo', 'svg'); + checkClass({ status: 'Unknown', className: 'foo' }, 'foo', 'svg'); }); }); }); diff --git a/src/components/StatusIcon/StatusIcon.tsx b/src/components/StatusIcon/StatusIcon.tsx index 64045c6..9670d95 100644 --- a/src/components/StatusIcon/StatusIcon.tsx +++ b/src/components/StatusIcon/StatusIcon.tsx @@ -20,16 +20,7 @@ import { import './StatusIcon.css'; -export enum StatusType { - Ok = 'Ok', - Warning = 'Warning', - Error = 'Error', - Info = 'Info', - Loading = 'Loading', - Unknown = 'Unknown', -} - -export type StatusIconType = keyof typeof StatusType; +export type StatusType = 'Ok' | 'Warning' | 'Error' | 'Info' | 'Loading' | 'Unknown'; type IconListType = { [key in StatusType]: { @@ -38,34 +29,34 @@ type IconListType = { }; }; const iconList: IconListType = { - [StatusType.Ok]: { + Ok: { Icon: CheckCircleIcon, color: successColor, }, - [StatusType.Warning]: { + Warning: { Icon: ExclamationTriangleIcon, color: warningColor, }, - [StatusType.Error]: { + Error: { Icon: ExclamationCircleIcon, color: errorColor, }, - [StatusType.Info]: { + Info: { Icon: InfoCircleIcon, color: infoColor, }, - [StatusType.Loading]: { + Loading: { Icon: Spinner, color: loadingColor, }, - [StatusType.Unknown]: { + Unknown: { Icon: QuestionCircleIcon, color: unknownColor, }, }; export interface IStatusIconProps { - status: StatusIconType; + status: StatusType; label?: React.ReactNode; isDisabled?: boolean; className?: string; @@ -81,9 +72,7 @@ export const StatusIcon: React.FunctionComponent = ({ const icon = ( );