From d2d13ea39a50a682acd050ff00f7ef44dae753f5 Mon Sep 17 00:00:00 2001 From: Domas Date: Thu, 6 May 2021 11:21:58 +0300 Subject: [PATCH] Alerting: miscllaneous UI fixes & improvements (#33734) --- .../features/alerting/unified/AmRoutes.tsx | 7 +-- .../features/alerting/unified/Receivers.tsx | 10 +++-- .../features/alerting/unified/Silences.tsx | 26 +++++++---- .../unified/components/AlertManagerPicker.tsx | 45 ++++++++++++------- .../alerting/unified/components/StateTag.tsx | 30 +++++-------- .../components/amroutes/AmRootRouteForm.tsx | 2 +- .../amroutes/AmRoutesExpandedForm.tsx | 2 +- .../components/receivers/ReceiversSection.tsx | 25 +++++++---- .../components/receivers/ReceiversTable.tsx | 10 +++++ .../components/receivers/TemplateForm.tsx | 1 + .../receivers/form/ReceiverForm.tsx | 3 +- .../rule-editor/GrafanaAlertStatePicker.tsx | 12 ++--- .../components/rules/AlertInstanceDetails.tsx | 8 ++-- .../components/rules/AlertInstancesTable.tsx | 25 +++++++---- .../components/rules/AlertStateTag.tsx | 20 +++++++++ .../unified/components/rules/RuleDetails.tsx | 20 +++++---- .../unified/components/rules/RuleQuery.tsx | 34 -------------- .../unified/components/rules/RulesTable.tsx | 9 ++-- .../components/silences/AmAlertStateTag.tsx | 15 +++++++ .../components/silences/SilenceStateTag.tsx | 17 +++++++ .../components/silences/SilenceTableRow.tsx | 4 +- .../silences/SilencedAlertsTableRow.tsx | 4 +- .../components/silences/SilencesTable.tsx | 14 +----- .../alerting/unified/types/rule-form.ts | 6 +-- .../alerting/unified/utils/rule-form.ts | 6 +-- public/app/types/unified-alerting-dto.ts | 16 +++++-- public/app/types/unified-alerting.ts | 3 +- 27 files changed, 217 insertions(+), 157 deletions(-) create mode 100644 public/app/features/alerting/unified/components/rules/AlertStateTag.tsx delete mode 100644 public/app/features/alerting/unified/components/rules/RuleQuery.tsx create mode 100644 public/app/features/alerting/unified/components/silences/AmAlertStateTag.tsx create mode 100644 public/app/features/alerting/unified/components/silences/SilenceStateTag.tsx diff --git a/public/app/features/alerting/unified/AmRoutes.tsx b/public/app/features/alerting/unified/AmRoutes.tsx index a52434cc9bb..ce12516fb3c 100644 --- a/public/app/features/alerting/unified/AmRoutes.tsx +++ b/public/app/features/alerting/unified/AmRoutes.tsx @@ -1,7 +1,7 @@ import React, { FC, useCallback, useEffect, useMemo, useState } from 'react'; import { css } from '@emotion/css'; import { GrafanaTheme2 } from '@grafana/data'; -import { Alert, Field, LoadingPlaceholder, useStyles2 } from '@grafana/ui'; +import { Alert, LoadingPlaceholder, useStyles2 } from '@grafana/ui'; import { useDispatch } from 'react-redux'; import { Redirect } from 'react-router-dom'; import { Receiver } from 'app/plugins/datasource/alertmanager/types'; @@ -96,9 +96,7 @@ const AmRoutes: FC = () => { return ( - - - + {savingError && !saving && ( {savingError.message || 'Unknown error.'} @@ -112,7 +110,6 @@ const AmRoutes: FC = () => { {resultLoading && } {result && !resultLoading && !resultError && ( <> -
{ return ( - - - + {error && !loading && ( {error.message || 'Unknown error.'} diff --git a/public/app/features/alerting/unified/Silences.tsx b/public/app/features/alerting/unified/Silences.tsx index 787240c92d1..240ed57f856 100644 --- a/public/app/features/alerting/unified/Silences.tsx +++ b/public/app/features/alerting/unified/Silences.tsx @@ -2,23 +2,29 @@ import React, { FC, useEffect, useCallback } from 'react'; import { Alert, LoadingPlaceholder } from '@grafana/ui'; import { useDispatch } from 'react-redux'; -import { Redirect, Route, RouteChildrenProps, Switch } from 'react-router-dom'; +import { Redirect, Route, RouteChildrenProps, Switch, useLocation } from 'react-router-dom'; import { AlertingPageWrapper } from './components/AlertingPageWrapper'; import SilencesTable from './components/silences/SilencesTable'; import { useAlertManagerSourceName } from './hooks/useAlertManagerSourceName'; import { useUnifiedAlertingSelector } from './hooks/useUnifiedAlertingSelector'; import { fetchAmAlertsAction, fetchSilencesAction } from './state/actions'; import { SILENCES_POLL_INTERVAL_MS } from './utils/constants'; -import { initialAsyncRequestState } from './utils/redux'; +import { AsyncRequestState, initialAsyncRequestState } from './utils/redux'; import SilencesEditor from './components/silences/SilencesEditor'; +import { AlertManagerPicker } from './components/AlertManagerPicker'; +import { Silence } from 'app/plugins/datasource/alertmanager/types'; const Silences: FC = () => { - const [alertManagerSourceName = '', setAlertManagerSourceName] = useAlertManagerSourceName(); + const [alertManagerSourceName, setAlertManagerSourceName] = useAlertManagerSourceName(); const dispatch = useDispatch(); const silences = useUnifiedAlertingSelector((state) => state.silences); + const alertsRequests = useUnifiedAlertingSelector((state) => state.amAlerts); + const alertsRequest = alertManagerSourceName + ? alertsRequests[alertManagerSourceName] || initialAsyncRequestState + : undefined; - const alerts = - useUnifiedAlertingSelector((state) => state.amAlerts)[alertManagerSourceName] || initialAsyncRequestState; + const location = useLocation(); + const isRoot = location.pathname.endsWith('/alerting/silences'); useEffect(() => { function fetchAll() { @@ -34,7 +40,9 @@ const Silences: FC = () => { }; }, [alertManagerSourceName, dispatch]); - const { result, loading, error } = silences[alertManagerSourceName] || initialAsyncRequestState; + const { result, loading, error }: AsyncRequestState = + (alertManagerSourceName && silences[alertManagerSourceName]) || initialAsyncRequestState; + const getSilenceById = useCallback((id: string) => result && result.find((silence) => silence.id === id), [result]); if (!alertManagerSourceName) { @@ -43,20 +51,20 @@ const Silences: FC = () => { return ( + {error && !loading && ( {error.message || 'Unknown error.'} )} {loading && } - {result && !error && alerts.result && ( + {result && !error && alertsRequest?.result && ( diff --git a/public/app/features/alerting/unified/components/AlertManagerPicker.tsx b/public/app/features/alerting/unified/components/AlertManagerPicker.tsx index e7a437bf80b..59984cd1381 100644 --- a/public/app/features/alerting/unified/components/AlertManagerPicker.tsx +++ b/public/app/features/alerting/unified/components/AlertManagerPicker.tsx @@ -1,8 +1,9 @@ -import { SelectableValue } from '@grafana/data'; +import { SelectableValue, GrafanaTheme2 } from '@grafana/data'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; import React, { FC, useMemo } from 'react'; -import { Select } from '@grafana/ui'; +import { Field, Select, useStyles2 } from '@grafana/ui'; import { getAllDataSources } from '../utils/config'; +import { css } from '@emotion/css'; interface Props { onChange: (alertManagerSourceName: string) => void; @@ -11,6 +12,8 @@ interface Props { } export const AlertManagerPicker: FC = ({ onChange, current, disabled = false }) => { + const styles = useStyles2(getStyles); + const options: Array> = useMemo(() => { return [ { @@ -30,20 +33,30 @@ export const AlertManagerPicker: FC = ({ onChange, current, disabled = fa ]; }, []); + // no need to show the picker if there's only one option + if (options.length === 1) { + return null; + } + return ( - value.value && onChange(value.value)} + options={options} + maxMenuHeight={500} + noOptionsMessage="No datasources found" + value={current} + getOptionLabel={(o) => o.label} + /> + ); }; + +const getStyles = (theme: GrafanaTheme2) => ({ + field: css` + margin-bottom: ${theme.spacing(4)}; + `, +}); diff --git a/public/app/features/alerting/unified/components/StateTag.tsx b/public/app/features/alerting/unified/components/StateTag.tsx index 7f86b9f6bd2..6a7bbcaa02f 100644 --- a/public/app/features/alerting/unified/components/StateTag.tsx +++ b/public/app/features/alerting/unified/components/StateTag.tsx @@ -1,18 +1,18 @@ import { GrafanaTheme2 } from '@grafana/data'; import { useStyles2 } from '@grafana/ui'; -import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; -import { SilenceState, AlertState } from 'app/plugins/datasource/alertmanager/types'; import { css, cx } from '@emotion/css'; import React, { FC } from 'react'; +export type State = 'good' | 'bad' | 'warning' | 'neutral' | 'info'; + type Props = { - status: PromAlertingRuleState | SilenceState | AlertState; + state: State; }; -export const StateTag: FC = ({ children, status }) => { +export const StateTag: FC = ({ children, state }) => { const styles = useStyles2(getStyles); - return {children || status}; + return {children || state}; }; const getStyles = (theme: GrafanaTheme2) => ({ @@ -25,37 +25,27 @@ const getStyles = (theme: GrafanaTheme2) => ({ text-transform: capitalize; line-height: 1.2; `, - [PromAlertingRuleState.Inactive]: css` + good: css` background-color: ${theme.colors.success.main}; border: solid 1px ${theme.colors.success.main}; color: ${theme.colors.success.contrastText}; `, - [PromAlertingRuleState.Pending]: css` + warning: css` background-color: ${theme.colors.warning.main}; border: solid 1px ${theme.colors.warning.main}; color: ${theme.colors.warning.contrastText}; `, - [PromAlertingRuleState.Firing]: css` + bad: css` background-color: ${theme.colors.error.main}; border: solid 1px ${theme.colors.error.main}; color: ${theme.colors.error.contrastText}; `, - [SilenceState.Expired]: css` + neutral: css` background-color: ${theme.colors.secondary.main}; border: solid 1px ${theme.colors.secondary.main}; color: ${theme.colors.secondary.contrastText}; `, - [SilenceState.Active]: css` - background-color: ${theme.colors.success.main}; - border: solid 1px ${theme.colors.success.main}; - color: ${theme.colors.success.contrastText}; - `, - [AlertState.Unprocessed]: css` - background-color: ${theme.colors.secondary.main}; - border: solid 1px ${theme.colors.secondary.main}; - color: ${theme.colors.secondary.contrastText}; - `, - [AlertState.Suppressed]: css` + info: css` background-color: ${theme.colors.primary.main}; border: solid 1px ${theme.colors.primary.main}; color: ${theme.colors.primary.contrastText}; diff --git a/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx b/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx index 9cdf5cbbcdf..123609841ac 100644 --- a/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx +++ b/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx @@ -187,7 +187,7 @@ export const AmRootRouteForm: FC = ({
-
diff --git a/public/app/features/alerting/unified/components/amroutes/AmRoutesExpandedForm.tsx b/public/app/features/alerting/unified/components/amroutes/AmRoutesExpandedForm.tsx index 479b565f41c..5325fce4c31 100644 --- a/public/app/features/alerting/unified/components/amroutes/AmRoutesExpandedForm.tsx +++ b/public/app/features/alerting/unified/components/amroutes/AmRoutesExpandedForm.tsx @@ -257,7 +257,7 @@ export const AmRoutesExpandedForm: FC = ({ onCancel, )}
-
diff --git a/public/app/features/alerting/unified/components/receivers/ReceiversSection.tsx b/public/app/features/alerting/unified/components/receivers/ReceiversSection.tsx index f11494d8959..421ca28d833 100644 --- a/public/app/features/alerting/unified/components/receivers/ReceiversSection.tsx +++ b/public/app/features/alerting/unified/components/receivers/ReceiversSection.tsx @@ -1,6 +1,6 @@ -import { css } from '@emotion/css'; -import { GrafanaTheme } from '@grafana/data'; -import { LinkButton, useStyles } from '@grafana/ui'; +import { css, cx } from '@emotion/css'; +import { GrafanaTheme2 } from '@grafana/data'; +import { LinkButton, useStyles2 } from '@grafana/ui'; import React, { FC } from 'react'; interface Props { @@ -8,13 +8,21 @@ interface Props { description: string; addButtonLabel: string; addButtonTo: string; + className?: string; } -export const ReceiversSection: FC = ({ title, description, addButtonLabel, addButtonTo, children }) => { - const styles = useStyles(getStyles); +export const ReceiversSection: FC = ({ + className, + title, + description, + addButtonLabel, + addButtonTo, + children, +}) => { + const styles = useStyles2(getStyles); return ( <> -
+

{title}

{description}

@@ -28,13 +36,12 @@ export const ReceiversSection: FC = ({ title, description, addButtonLabel ); }; -const getStyles = (theme: GrafanaTheme) => ({ +const getStyles = (theme: GrafanaTheme2) => ({ heading: css` - margin-top: ${theme.spacing.xl}; display: flex; justify-content: space-between; `, description: css` - color: ${theme.colors.textSemiWeak}; + color: ${theme.colors.text.secondary}; `, }); diff --git a/public/app/features/alerting/unified/components/receivers/ReceiversTable.tsx b/public/app/features/alerting/unified/components/receivers/ReceiversTable.tsx index de6f49db336..11747967dc2 100644 --- a/public/app/features/alerting/unified/components/receivers/ReceiversTable.tsx +++ b/public/app/features/alerting/unified/components/receivers/ReceiversTable.tsx @@ -7,6 +7,8 @@ import { extractReadableNotifierTypes } from '../../utils/receivers'; import { ActionIcon } from '../rules/ActionIcon'; import { ReceiversSection } from './ReceiversSection'; import { makeAMLink } from '../../utils/misc'; +import { GrafanaTheme2 } from '@grafana/data'; +import { css } from '@emotion/css'; interface Props { config: AlertManagerCortexConfig; @@ -15,6 +17,7 @@ interface Props { export const ReceiversTable: FC = ({ config, alertManagerName }) => { const tableStyles = useStyles2(getAlertTableStyles); + const styles = useStyles2(getStyles); const grafanaNotifiers = useUnifiedAlertingSelector((state) => state.grafanaNotifiers); @@ -29,6 +32,7 @@ export const ReceiversTable: FC = ({ config, alertManagerName }) => { return ( = ({ config, alertManagerName }) => { ); }; + +const getStyles = (theme: GrafanaTheme2) => ({ + section: css` + margin-top: ${theme.spacing(4)}; + `, +}); diff --git a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx index c6703dd8e62..2f39e901256 100644 --- a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx @@ -153,6 +153,7 @@ export const TemplateForm: FC = ({ existing, alertManagerSourceName, conf href={makeAMLink('alerting/notifications', alertManagerSourceName)} variant="secondary" type="button" + fill="outline" > Cancel diff --git a/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx b/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx index 7d2a5179818..0d03cdc3a7f 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx @@ -120,7 +120,8 @@ export function ReceiverForm({ Cancel diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaAlertStatePicker.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaAlertStatePicker.tsx index 81daf80741d..a33f7cb771f 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GrafanaAlertStatePicker.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaAlertStatePicker.tsx @@ -1,16 +1,16 @@ import { SelectableValue } from '@grafana/data'; import { Select } from '@grafana/ui'; import { SelectBaseProps } from '@grafana/ui/src/components/Select/types'; -import { GrafanaAlertState } from 'app/types/unified-alerting-dto'; +import { GrafanaAlertStateDecision } from 'app/types/unified-alerting-dto'; import React, { FC } from 'react'; -type Props = Omit, 'options'>; +type Props = Omit, 'options'>; const options: SelectableValue[] = [ - { value: GrafanaAlertState.Alerting, label: 'Alerting' }, - { value: GrafanaAlertState.NoData, label: 'No Data' }, - { value: GrafanaAlertState.KeepLastState, label: 'Keep Last State' }, - { value: GrafanaAlertState.OK, label: 'OK' }, + { value: GrafanaAlertStateDecision.Alerting, label: 'Alerting' }, + { value: GrafanaAlertStateDecision.NoData, label: 'No Data' }, + { value: GrafanaAlertStateDecision.KeepLastState, label: 'Keep Last State' }, + { value: GrafanaAlertStateDecision.OK, label: 'OK' }, ]; export const GrafanaAlertStatePicker: FC = (props) =>