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: update alert/report icons and column order #12081

Merged
merged 3 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions superset-frontend/images/icons/alert_solid_small.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 9 additions & 8 deletions superset-frontend/images/icons/slack.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions superset-frontend/src/components/Icon/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import React, { SVGProps } from 'react';

import { ReactComponent as AlertIcon } from 'images/icons/alert.svg';
import { ReactComponent as AlertSolidIcon } from 'images/icons/alert_solid.svg';
import { ReactComponent as AlertSolidSmallIcon } from 'images/icons/alert_solid_small.svg';
import { ReactComponent as BinocularsIcon } from 'images/icons/binoculars.svg';
import { ReactComponent as BoltIcon } from 'images/icons/bolt.svg';
import { ReactComponent as BoltSmallIcon } from 'images/icons/bolt_small.svg';
Expand Down Expand Up @@ -144,6 +145,7 @@ import { ReactComponent as XSmallIcon } from 'images/icons/x-small.svg';
export type IconName =
| 'alert'
| 'alert-solid'
| 'alert-solid-small'
| 'binoculars'
| 'bolt'
| 'bolt-small'
Expand Down Expand Up @@ -270,6 +272,7 @@ export const iconsRegistry: Record<
React.ComponentType<SVGProps<SVGSVGElement>>
> = {
'alert-solid': AlertSolidIcon,
'alert-solid-small': AlertSolidSmallIcon,
'bolt-small': BoltSmallIcon,
'bolt-small-run': BoltSmallRunIcon,
'cancel-solid': CancelSolidIcon,
Expand Down
50 changes: 36 additions & 14 deletions superset-frontend/src/views/CRUD/alert/AlertList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import React, { useState, useMemo, useEffect } from 'react';
import { useHistory } from 'react-router-dom';
import { t, SupersetClient, makeApi, styled } from '@superset-ui/core';
import moment from 'moment';
import ActionsBar, { ActionProps } from 'src/components/ListView/ActionsBar';
import Button from 'src/components/Button';
import FacePile from 'src/components/FacePile';
Expand All @@ -32,6 +33,7 @@ import ListView, {
} from 'src/components/ListView';
import SubMenu, { SubMenuProps } from 'src/components/Menu/SubMenu';
import { Switch } from 'src/common/components/Switch';
import { DATETIME_WITH_TIME_ZONE } from 'src/constants';
import withToasts from 'src/messageToasts/enhancers/withToasts';
import AlertStatusIcon from 'src/views/CRUD/alert/components/AlertStatusIcon';
import RecipientIcon from 'src/views/CRUD/alert/components/RecipientIcon';
Expand Down Expand Up @@ -192,27 +194,33 @@ function AlertList({
row: {
original: { last_state: lastState },
},
}: any) => <AlertStatusIcon state={lastState} />,
}: any) => (
<AlertStatusIcon
state={lastState}
isReportEnabled={isReportEnabled}
/>
),
accessor: 'last_state',
size: 'xs',
disableSortBy: true,
},
{
accessor: 'name',
Header: t('Name'),
},
{
Cell: ({
row: {
original: { recipients },
original: { last_eval_dttm: lastEvalDttm },
},
}: any) =>
recipients.map((r: any) => (
<RecipientIcon key={r.id} type={r.type} />
)),
accessor: 'recipients',
Header: t('Notification Method'),
disableSortBy: true,
}: any) => {
return lastEvalDttm
? moment.utc(lastEvalDttm).local().format(DATETIME_WITH_TIME_ZONE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated but we should update other lists where we use moment to properly handle timezones

: '';
},
accessor: 'last_eval_dttm',
Header: t('Last Run'),
size: 'lg',
},
{
accessor: 'name',
Header: t('Name'),
size: 'xl',
},
{
Expand All @@ -229,6 +237,20 @@ function AlertList({
</Tooltip>
),
},
{
Cell: ({
row: {
original: { recipients },
},
}: any) =>
recipients.map((r: any) => (
<RecipientIcon key={r.id} type={r.type} />
)),
accessor: 'recipients',
Header: t('Notification Method'),
disableSortBy: true,
size: 'xl',
},
{
accessor: 'created_by',
disableSortBy: true,
Expand Down Expand Up @@ -307,7 +329,7 @@ function AlertList({
size: 'xl',
},
],
[canDelete, canEdit],
[canDelete, canEdit, isReportEnabled],
);

const subMenuButtons: SubMenuProps['buttons'] = [];
Expand Down
6 changes: 4 additions & 2 deletions superset-frontend/src/views/CRUD/alert/ExecutionLog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ function ExecutionLog({ addDangerToast, isReportEnabled }: ExecutionLogProps) {
row: {
original: { state },
},
}: any) => <AlertStatusIcon state={state} />,
}: any) => (
<AlertStatusIcon state={state} isReportEnabled={isReportEnabled} />
),
accessor: 'state',
Header: t('State'),
size: 'xs',
Expand Down Expand Up @@ -125,7 +127,7 @@ function ExecutionLog({ addDangerToast, isReportEnabled }: ExecutionLogProps) {
Header: t('Error Message'),
},
],
[],
[isReportEnabled],
);
const path = `/${isReportEnabled ? 'report' : 'alert'}/list/`;
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,53 +22,87 @@ import { Tooltip } from 'src/common/components/Tooltip';
import Icon, { IconName } from 'src/components/Icon';
import { AlertState } from '../types';

const StatusIcon = styled(Icon)<{ status: string }>`
color: ${({ status, theme }) => {
const StatusIcon = styled(Icon)<{ status: string; isReportEnabled: boolean }>`
color: ${({ status, theme, isReportEnabled }) => {
switch (status) {
case AlertState.working:
return theme.colors.alert.base;
return theme.colors.primary.base;
case AlertState.error:
return theme.colors.error.base;
case AlertState.success:
return isReportEnabled
? theme.colors.success.base
: theme.colors.alert.base;
case AlertState.noop:
return theme.colors.success.base;
case AlertState.grace:
return theme.colors.alert.base;
default:
return theme.colors.grayscale.base;
}
}};
`;

export default function AlertStatusIcon({ state }: { state: string }) {
export default function AlertStatusIcon({
state,
isReportEnabled = false,
}: {
state: string;
isReportEnabled: boolean;
}) {
const lastStateConfig = {
name: '',
label: '',
status: '',
};
switch (state) {
case AlertState.success:
lastStateConfig.name = 'check';
lastStateConfig.label = t(`${AlertState.success}`);
lastStateConfig.name = isReportEnabled ? 'check' : 'alert-solid-small';
lastStateConfig.label = isReportEnabled
? t('Report Sent')
: t('Alert Triggered, Notification Sent');
lastStateConfig.status = AlertState.success;
break;
case AlertState.working:
lastStateConfig.name = 'exclamation';
lastStateConfig.label = t(`${AlertState.working}`);
lastStateConfig.name = 'running';
lastStateConfig.label = isReportEnabled
? t('Report Sending')
: t('Alert Running');
lastStateConfig.status = AlertState.working;
break;
case AlertState.error:
lastStateConfig.name = 'x-small';
lastStateConfig.label = t(`${AlertState.error}`);
lastStateConfig.label = isReportEnabled
? t('Report Failed')
: t('Alert Failed');
lastStateConfig.status = AlertState.error;
break;
case AlertState.noop:
lastStateConfig.name = 'check';
lastStateConfig.label = t('Nothing Triggered');
lastStateConfig.status = AlertState.noop;
break;
case AlertState.grace:
lastStateConfig.name = 'alert-solid-small';
lastStateConfig.label = t('Alert Triggered, In Grace Period');
lastStateConfig.status = AlertState.grace;
break;
default:
lastStateConfig.name = 'exclamation';
lastStateConfig.label = t(`${AlertState.working}`);
lastStateConfig.status = AlertState.working;
lastStateConfig.name = 'check';
lastStateConfig.label = t('Nothing Triggered');
lastStateConfig.status = AlertState.noop;
}
return (
<Tooltip title={lastStateConfig.label} placement="bottom">
<Tooltip title={lastStateConfig.label} placement="bottomLeft">
<StatusIcon
name={lastStateConfig.name as IconName}
status={lastStateConfig.status}
isReportEnabled={isReportEnabled}
viewBox={
lastStateConfig.name === 'alert-solid-small'
? '-6 -6 24 24'
: '0 0 24 24'
Comment on lines +100 to +104
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be a better way to assign viewBox

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to do this, anyway? The viewbox on that icon is 0 0 24 24, is there something strange going on with the spacing?

}
/>
</Tooltip>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@
* specific language governing permissions and limitations
* under the License.
*/
import { t } from '@superset-ui/core';
import { styled, t } from '@superset-ui/core';
import React from 'react';
import { Tooltip } from 'src/common/components/Tooltip';
import Icon, { IconName } from 'src/components/Icon';
import { RecipientIconName } from '../types';

const StyledIcon = styled(Icon)`
color: ${({ theme }) => theme.colors.grayscale.light1};
margin-right: ${({ theme }) => theme.gridUnit * 2}px;
`;

export default function RecipientIcon({ type }: { type: string }) {
const recipientIconConfig = {
name: '',
Expand All @@ -42,7 +47,7 @@ export default function RecipientIcon({ type }: { type: string }) {
}
return recipientIconConfig.name.length ? (
<Tooltip title={recipientIconConfig.label} placement="bottom">
<Icon name={recipientIconConfig.name as IconName} />
<StyledIcon name={recipientIconConfig.name as IconName} />
</Tooltip>
) : null;
}