From 670e687245da795f49512f57d6cca2f9c8a93432 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Thu, 29 Sep 2022 14:49:24 +0200 Subject: [PATCH] Improving rendering of `TimeRangeDropdown` component. (#13554) * Improving rendering of `TimeRangeDropdown` component. * Memoize initial form values. --- .../views/components/TimeRangeValidation.ts | 12 ++- ...eRangeDropdown.relativeTimeRange.test.tsx} | 41 ++++----- .../date-time-picker/TimeRangeDropdown.tsx | 86 ++++++++++++------- 3 files changed, 76 insertions(+), 63 deletions(-) rename graylog2-web-interface/src/views/components/searchbar/date-time-picker/{TimerangeDropdown.relativeTimeRange.test.tsx => TimeRangeDropdown.relativeTimeRange.test.tsx} (78%) diff --git a/graylog2-web-interface/src/views/components/TimeRangeValidation.ts b/graylog2-web-interface/src/views/components/TimeRangeValidation.ts index bbc6e24731f1..baad1173a3ec 100644 --- a/graylog2-web-interface/src/views/components/TimeRangeValidation.ts +++ b/graylog2-web-interface/src/views/components/TimeRangeValidation.ts @@ -78,7 +78,7 @@ const validateAbsoluteTimeRange = (timeRange: AbsoluteTimeRange, limitDuration: }; const validateRelativeTimeRangeWithEnd = (timeRange: RelativeTimeRangeWithEnd, limitDuration: number) => { - let errors = {}; + let errors: { from?: string, to?: string } = {}; if (limitDuration > 0) { if (timeRange.from > limitDuration || !timeRange.from) { @@ -116,21 +116,19 @@ const validateKeywordTimeRange = (timeRange: KeywordTimeRange, limitDuration: nu }; const validateTimeRange = (timeRange: TimeRange | NoTimeRangeOverride, limitDuration: number, formatTime: (dateTime: DateTime, format: string) => string) => { - let errors = {}; - if (isTypeKeyword(timeRange)) { - errors = { ...errors, ...validateKeywordTimeRange(timeRange, limitDuration, formatTime) }; + return validateKeywordTimeRange(timeRange, limitDuration, formatTime); } if (isTypeRelativeWithEnd(timeRange)) { - errors = { ...errors, ...validateRelativeTimeRangeWithEnd(timeRange, limitDuration) }; + return validateRelativeTimeRangeWithEnd(timeRange, limitDuration); } if (isTypeAbsolute(timeRange)) { - errors = { ...errors, ...validateAbsoluteTimeRange(timeRange, limitDuration, formatTime) }; + return validateAbsoluteTimeRange(timeRange, limitDuration, formatTime); } - return errors; + return {}; }; export default validateTimeRange; diff --git a/graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimerangeDropdown.relativeTimeRange.test.tsx b/graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimeRangeDropdown.relativeTimeRange.test.tsx similarity index 78% rename from graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimerangeDropdown.relativeTimeRange.test.tsx rename to graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimeRangeDropdown.relativeTimeRange.test.tsx index 0235caf116ad..a31031342530 100644 --- a/graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimerangeDropdown.relativeTimeRange.test.tsx +++ b/graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimeRangeDropdown.relativeTimeRange.test.tsx @@ -50,14 +50,14 @@ describe('TimeRangeDropdown relative time range', () => { limitDuration: 259200, } as const; - const TimeRangeDropdown = (allProps: TimeRangeDropdownProps) => ( - + const TimeRangeDropdown = (allProps: Partial) => ( + ); const getSubmitButton = () => screen.getByRole('button', { name: /Update time range/i }); it('display warning when emptying from range value input', async () => { - render(); + render(); const fromRangeValueInput = screen.getByTitle('Set the from value'); @@ -70,15 +70,12 @@ describe('TimeRangeDropdown relative time range', () => { }); it('display warning when emptying to range value input', async () => { - const props = { - ...defaultProps, - currentTimeRange: { - type: 'relative', - from: 300, - to: 240, - }, + const currentTimeRange = { + type: 'relative', + from: 300, + to: 240, }; - render(); + render(); const toRangeValueInput = screen.getByTitle('Set the to value'); const submitButton = getSubmitButton(); @@ -92,24 +89,20 @@ describe('TimeRangeDropdown relative time range', () => { it('allow emptying from and to ranges and typing in completely new values', async () => { const setCurrentTimeRangeStub = jest.fn(); - const props = { - ...defaultProps, - currentTimeRange: { - type: 'relative', - from: 300, - to: 240, - }, - setCurrentTimeRange: setCurrentTimeRangeStub, + const currentTimeRange = { + type: 'relative', + from: 300, + to: 240, }; - render(); + render(); - const fromRangeValueInput = screen.getByTitle('Set the from value'); + const fromRangeValueInput = await screen.findByTitle('Set the from value'); const toRangeValueInput = screen.getByTitle('Set the to value'); const submitButton = getSubmitButton(); - userEvent.type(fromRangeValueInput, '{backspace}7'); - userEvent.type(toRangeValueInput, '{backspace}6'); - userEvent.click(submitButton); + await userEvent.type(fromRangeValueInput, '{backspace}7'); + await userEvent.type(toRangeValueInput, '{backspace}6'); + await userEvent.click(submitButton); await waitFor(() => expect(setCurrentTimeRangeStub).toHaveBeenCalledTimes(1)); diff --git a/graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimeRangeDropdown.tsx b/graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimeRangeDropdown.tsx index 76dab404553b..63d7e4370dbb 100644 --- a/graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimeRangeDropdown.tsx +++ b/graylog2-web-interface/src/views/components/searchbar/date-time-picker/TimeRangeDropdown.tsx @@ -15,7 +15,7 @@ * . */ import * as React from 'react'; -import { useCallback, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import { Form, Formik } from 'formik'; import styled, { css } from 'styled-components'; import moment from 'moment'; @@ -164,15 +164,12 @@ const timeRangeTypeTabs = ({ activeTab, limitDuration, setValidatingKeyword, tab }); const dateTimeValidate = (nextTimeRange, limitDuration, formatTime: (dateTime: DateTime, format: string) => string) => { - let errors = {}; const timeRange = normalizeIfClassifiedRelativeTimeRange(nextTimeRange); const timeRangeErrors = validateTimeRange(timeRange, limitDuration, formatTime); - if (Object.keys(timeRangeErrors).length !== 0) { - errors = { ...errors, nextTimeRange: timeRangeErrors }; - } - - return errors; + return Object.keys(timeRangeErrors).length !== 0 + ? { nextTimeRange: timeRangeErrors } + : {}; }; const onInitializingNextTimeRange = (currentTimeRange: SearchBarFormValues['timerange'] | NoTimeRangeOverride) => { @@ -183,6 +180,41 @@ const onInitializingNextTimeRange = (currentTimeRange: SearchBarFormValues['time return currentTimeRange; }; +type TimeRangeTabsProps = { + handleActiveTab: (nextTab: AbsoluteTimeRange['type'] | RelativeTimeRange['type'] | KeywordTimeRange['type']) => void, + currentTimeRange: NoTimeRangeOverride | TimeRange, + limitDuration: number, + validTypes: Array<'absolute' | 'relative' | 'keyword'>, + setValidatingKeyword: (validating: boolean) => void, +}; + +const TimeRangeTabs = ({ handleActiveTab, currentTimeRange, limitDuration, validTypes, setValidatingKeyword }: TimeRangeTabsProps) => { + const [activeTab, setActiveTab] = useState('type' in currentTimeRange ? currentTimeRange.type : undefined); + + const onSelect = useCallback((nextTab: AbsoluteTimeRange['type'] | RelativeTimeRange['type'] | KeywordTimeRange['type']) => { + handleActiveTab(nextTab); + setActiveTab(nextTab); + }, [handleActiveTab]); + + const tabs = useMemo(() => timeRangeTypeTabs({ + activeTab, + limitDuration, + setValidatingKeyword, + tabs: validTypes, + }), [activeTab, limitDuration, setValidatingKeyword, validTypes]); + + return ( + + {tabs} + {!activeTab && ()} + + ); +}; + const TimeRangeDropdown = ({ noOverride, toggleDropdownShow, @@ -194,25 +226,24 @@ const TimeRangeDropdown = ({ }: TimeRangeDropdownProps) => { const { formatTime, userTimezone } = useUserDateTime(); const [validatingKeyword, setValidatingKeyword] = useState(false); - const [activeTab, setActiveTab] = useState('type' in currentTimeRange ? currentTimeRange.type : undefined); const positionIsBottom = position === 'bottom'; - const defaultRanges = createDefaultRanges(formatTime); + const defaultRanges = useMemo(() => createDefaultRanges(formatTime), [formatTime]); - const handleNoOverride = () => { + const handleNoOverride = useCallback(() => { setCurrentTimeRange({}); toggleDropdownShow(); - }; + }, [setCurrentTimeRange, toggleDropdownShow]); const handleCancel = useCallback(() => { toggleDropdownShow(); }, [toggleDropdownShow]); - const handleSubmit = ({ nextTimeRange }: { nextTimeRange: TimeRangeDropDownFormValues['nextTimeRange'] }) => { + const handleSubmit = useCallback(({ nextTimeRange }: { nextTimeRange: TimeRangeDropDownFormValues['nextTimeRange'] }) => { setCurrentTimeRange(normalizeIfAllMessagesRange(normalizeIfClassifiedRelativeTimeRange(nextTimeRange))); toggleDropdownShow(); - }; + }, [setCurrentTimeRange, toggleDropdownShow]); const title = ( @@ -226,6 +257,9 @@ const TimeRangeDropdown = ({ ); + const _validateTimeRange = useCallback(({ nextTimeRange }) => dateTimeValidate(nextTimeRange, limitDuration, formatTime), [formatTime, limitDuration]); + const initialTimeRange = useMemo(() => ({ nextTimeRange: onInitializingNextTimeRange(currentTimeRange) }), [currentTimeRange]); + return ( - initialValues={{ nextTimeRange: onInitializingNextTimeRange(currentTimeRange) }} - validate={({ nextTimeRange }) => dateTimeValidate(nextTimeRange, limitDuration, formatTime)} + initialValues={initialTimeRange} + validate={_validateTimeRange} onSubmit={handleSubmit} validateOnMount> {(({ values: { nextTimeRange }, isValid, setFieldValue, submitForm }) => { @@ -246,8 +280,6 @@ const TimeRangeDropdown = ({ } else { setFieldValue('nextTimeRange', defaultRanges[nextTab]); } - - setActiveTab(nextTab); }; return ( @@ -257,21 +289,11 @@ const TimeRangeDropdown = ({ - - {timeRangeTypeTabs({ - activeTab, - limitDuration, - setValidatingKeyword, - tabs: validTypes, - })} - - {!activeTab && ()} - - +