From da6236d89a67025ac0d67861237dc736e550b767 Mon Sep 17 00:00:00 2001 From: Marcus Andersson Date: Thu, 27 May 2021 15:58:58 +0200 Subject: [PATCH] Alerting: improved error message and change styling of relative button. (#34730) * improved error message and change styling of relative button. * minor adjustments. * adding clock icon. --- .../RelativeTimeRangePicker.tsx | 119 ++++++++++++------ .../RelativeTimeRangePicker/utils.test.ts | 12 +- .../RelativeTimeRangePicker/utils.ts | 20 ++- .../TimeRangePicker/TimeRangeForm.tsx | 2 +- 4 files changed, 107 insertions(+), 46 deletions(-) diff --git a/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/RelativeTimeRangePicker.tsx b/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/RelativeTimeRangePicker.tsx index 5eff96bbb79b..0357d4e0bbbb 100644 --- a/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/RelativeTimeRangePicker.tsx +++ b/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/RelativeTimeRangePicker.tsx @@ -1,18 +1,22 @@ import React, { FormEvent, ReactElement, useCallback, useState } from 'react'; -import { css } from '@emotion/css'; +import { css, cx } from '@emotion/css'; import { RelativeTimeRange, GrafanaTheme2, TimeOption } from '@grafana/data'; -import { Tooltip } from '../../Tooltip/Tooltip'; import { useStyles2 } from '../../../themes'; -import { Button, ButtonGroup, ToolbarButton } from '../../Button'; +import { Button } from '../../Button'; import { ClickOutsideWrapper } from '../../ClickOutsideWrapper/ClickOutsideWrapper'; import { TimeRangeList } from '../TimeRangePicker/TimeRangeList'; import { quickOptions } from '../rangeOptions'; import CustomScrollbar from '../../CustomScrollbar/CustomScrollbar'; import { TimePickerTitle } from '../TimeRangePicker/TimePickerTitle'; -import { isRangeValid, isRelativeFormat, mapOptionToRelativeTimeRange, mapRelativeTimeRangeToOption } from './utils'; +import { + isRangeValid, + isRelativeFormat, + mapOptionToRelativeTimeRange, + mapRelativeTimeRangeToOption, + RangeValidation, +} from './utils'; import { Field } from '../../Forms/Field'; -import { Input } from '../../Input/Input'; -import { InputState } from '../TimeRangePicker/TimeRangeForm'; +import { getInputStyles, Input } from '../../Input/Input'; import { Icon } from '../../Icon/Icon'; /** @@ -23,7 +27,11 @@ export interface RelativeTimeRangePickerProps { onChange: (timeRange: RelativeTimeRange) => void; } -const errorMessage = 'Value not in relative time format.'; +type InputState = { + value: string; + validation: RangeValidation; +}; + const validOptions = quickOptions.filter((o) => isRelativeFormat(o.from)); /** @@ -34,10 +42,10 @@ export function RelativeTimeRangePicker(props: RelativeTimeRangePickerProps): Re const [isOpen, setIsOpen] = useState(false); const onClose = useCallback(() => setIsOpen(false), []); const timeOption = mapRelativeTimeRangeToOption(timeRange); - const [from, setFrom] = useState({ value: timeOption.from, invalid: !isRangeValid(timeOption.from) }); - const [to, setTo] = useState({ value: timeOption.to, invalid: !isRangeValid(timeOption.to) }); + const [from, setFrom] = useState({ value: timeOption.from, validation: isRangeValid(timeOption.from) }); + const [to, setTo] = useState({ value: timeOption.to, validation: isRangeValid(timeOption.to) }); - const styles = useStyles2(getStyles(from.invalid, to.invalid)); + const styles = useStyles2(getStyles(from.validation.errorMessage, to.validation.errorMessage)); const onChangeTimeOption = (option: TimeOption) => { const relativeTimeRange = mapOptionToRelativeTimeRange(option); @@ -51,7 +59,7 @@ export function RelativeTimeRangePicker(props: RelativeTimeRangePickerProps): Re }; const onOpen = useCallback( - (event: FormEvent) => { + (event: FormEvent) => { event.stopPropagation(); event.preventDefault(); setIsOpen(!isOpen); @@ -62,7 +70,7 @@ export function RelativeTimeRangePicker(props: RelativeTimeRangePickerProps): Re const onApply = (event: FormEvent) => { event.preventDefault(); - if (to.invalid || from.invalid) { + if (!to.validation.isValid || !from.validation.isValid) { return; } @@ -81,14 +89,18 @@ export function RelativeTimeRangePicker(props: RelativeTimeRangePickerProps): Re }; return ( - - - - - {timeOption.from} to {timeOption.to} - - - +
+
+ + + + + {timeOption.from} to {timeOption.to} + + + + +
{isOpen && (
@@ -112,18 +124,18 @@ export function RelativeTimeRangePicker(props: RelativeTimeRangePickerProps): Re .
- + event.stopPropagation()} - onBlur={() => setFrom({ ...from, invalid: !isRangeValid(from.value) })} + onBlur={() => setFrom({ ...from, validation: isRangeValid(from.value) })} onChange={(event) => setFrom({ ...from, value: event.currentTarget.value })} value={from.value} /> - + event.stopPropagation()} - onBlur={() => setTo({ ...to, invalid: !isRangeValid(to.value) })} + onBlur={() => setTo({ ...to, validation: isRangeValid(to.value) })} onChange={(event) => setTo({ ...to, value: event.currentTarget.value })} value={to.value} /> @@ -136,33 +148,54 @@ export function RelativeTimeRangePicker(props: RelativeTimeRangePickerProps): Re )} - + ); } -const getStyles = (fromInvalid: boolean, toInvalid: boolean) => (theme: GrafanaTheme2) => { - let bodyHeight = 250; - const errorHeight = theme.spacing.gridSize * 4; - - if (fromInvalid && toInvalid) { - bodyHeight += errorHeight * 2; - } else if (fromInvalid || toInvalid) { - bodyHeight += errorHeight; - } +const getStyles = (fromError?: string, toError?: string) => (theme: GrafanaTheme2) => { + const inputStyles = getInputStyles({ theme, invalid: false }); + const bodyMinimumHeight = 250; + const bodyHeight = bodyMinimumHeight + calculateErrorHeight(theme, fromError) + calculateErrorHeight(theme, toError); return { container: css` - position: relative; display: flex; - vertical-align: middle; + position: relative; `, + pickerInput: cx( + inputStyles.input, + inputStyles.wrapper, + css` + display: flex; + align-items: center; + justify-content: space-between; + cursor: pointer; + padding-right: 0; + padding-left: 0; + line-height: ${theme.v1.spacing.formInputHeight - 2}px; + ` + ), + caretIcon: cx( + inputStyles.suffix, + css` + position: relative; + margin-left: ${theme.v1.spacing.xs}; + ` + ), + clockIcon: cx( + inputStyles.prefix, + css` + position: relative; + margin-right: ${theme.v1.spacing.xs}; + ` + ), content: css` background: ${theme.colors.background.primary}; box-shadow: ${theme.shadows.z3}; position: absolute; z-index: ${theme.zIndex.dropdown}; width: 500px; - top: 116%; + top: 100%; border-radius: 2px; border: 1px solid ${theme.colors.border.weak}; left: 0; @@ -189,3 +222,15 @@ const getStyles = (fromInvalid: boolean, toInvalid: boolean) => (theme: GrafanaT `, }; }; + +function calculateErrorHeight(theme: GrafanaTheme2, errorMessage?: string): number { + if (!errorMessage) { + return 0; + } + + if (errorMessage.length > 34) { + return theme.spacing.gridSize * 6.5; + } + + return theme.spacing.gridSize * 4; +} diff --git a/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/utils.test.ts b/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/utils.test.ts index 0e68333d145f..fce263505e05 100644 --- a/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/utils.test.ts +++ b/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/utils.test.ts @@ -90,19 +90,23 @@ describe('utils', () => { describe('isRangeValid', () => { it('should consider now as a valid relative format', () => { - expect(isRangeValid('now')).toBe(true); + const result = isRangeValid('now'); + expect(result.isValid).toBe(true); }); it('should consider now-90d as a valid relative format', () => { - expect(isRangeValid('now-90d')).toBe(true); + const result = isRangeValid('now-90d'); + expect(result.isValid).toBe(true); }); it('should consider now-90000000d as an invalid relative format', () => { - expect(isRangeValid('now-90000000d')).toBe(false); + const result = isRangeValid('now-90000000d'); + expect(result.isValid).toBe(false); }); it('should consider now-11111111111s as an invalid relative format', () => { - expect(isRangeValid('now-11111111111s')).toBe(false); + const result = isRangeValid('now-11111111111s'); + expect(result.isValid).toBe(false); }); }); }); diff --git a/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/utils.ts b/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/utils.ts index 73cd02acc9b4..cc4d321d8866 100644 --- a/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/utils.ts +++ b/packages/grafana-ui/src/components/TimePicker/RelativeTimeRangePicker/utils.ts @@ -16,17 +16,29 @@ export const mapRelativeTimeRangeToOption = (range: RelativeTimeRange): TimeOpti return { from, to, display: `${from} to ${to}` }; }; -export const isRangeValid = (relative: string, now = Date.now()): boolean => { +export type RangeValidation = { + isValid: boolean; + errorMessage?: string; +}; + +export const isRangeValid = (relative: string, now = Date.now()): RangeValidation => { if (!isRelativeFormat(relative)) { - return false; + return { + isValid: false, + errorMessage: 'Value not in relative time format.', + }; } const seconds = relativeToSeconds(relative); if (seconds > Math.ceil(now / 1000)) { - return false; + return { + isValid: false, + errorMessage: 'Can not enter value prior to January 1, 1970.', + }; } - return true; + + return { isValid: true }; }; export const isRelativeFormat = (format: string): boolean => { diff --git a/packages/grafana-ui/src/components/TimePicker/TimeRangePicker/TimeRangeForm.tsx b/packages/grafana-ui/src/components/TimePicker/TimeRangePicker/TimeRangeForm.tsx index cbbede690542..b80409107ec3 100644 --- a/packages/grafana-ui/src/components/TimePicker/TimeRangePicker/TimeRangeForm.tsx +++ b/packages/grafana-ui/src/components/TimePicker/TimeRangePicker/TimeRangeForm.tsx @@ -25,7 +25,7 @@ interface Props { isReversed?: boolean; } -export interface InputState { +interface InputState { value: string; invalid: boolean; }