Improving rendering of TimeRangeDropdown component. (#13554)

* Improving rendering of `TimeRangeDropdown` component. * Memoize initial form values.
Graylog2 · Sep 29, 2022 · 670e687 · 670e687
1 parent 09451a2
commit 670e687
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 63 deletions.
diff --git 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/...erangeDropdown.relativeTimeRange.test.tsx → ...eRangeDropdown.relativeTimeRange.test.tsx b/...erangeDropdown.relativeTimeRange.test.tsx → ...eRangeDropdown.relativeTimeRange.test.tsx
@@ -50,14 +50,14 @@ describe('TimeRangeDropdown relative time range', () => {
     limitDuration: 259200,
   } as const;
 
-  const TimeRangeDropdown = (allProps: TimeRangeDropdownProps) => (
-    <OriginalTimeRangeDropDown {...allProps} />
+  const TimeRangeDropdown = (allProps: Partial<TimeRangeDropdownProps>) => (
+    <OriginalTimeRangeDropDown {...defaultProps} {...allProps} />
   );
 
   const getSubmitButton = () => screen.getByRole('button', { name: /Update time range/i });
 
   it('display warning when emptying from range value input', async () => {
-    render(<TimeRangeDropdown {...defaultProps} />);
+    render(<TimeRangeDropdown />);
 
     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(<TimeRangeDropdown {...props} />);
+    render(<TimeRangeDropdown currentTimeRange={currentTimeRange} />);
 
     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(<TimeRangeDropdown {...props} />);
+    render(<TimeRangeDropdown currentTimeRange={currentTimeRange} setCurrentTimeRange={setCurrentTimeRangeStub} />);
 
-    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
@@ -15,7 +15,7 @@
  * <http://www.mongodb.com/licensing/server-side-public-license>.
  */
 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 (
+    <StyledTabs id="dateTimeTypes"
+                defaultActiveKey={availableTimeRangeTypes[0].type}
+                activeKey={activeTab ?? -1}
+                onSelect={onSelect}
+                animation={false}>
+      {tabs}
+      {!activeTab && (<TabDisabledTimeRange />)}
+    </StyledTabs>
+  );
+};
+
 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 = (
     <PopoverTitle>
@@ -226,6 +257,9 @@ const TimeRangeDropdown = ({
     </PopoverTitle>
   );
 
+  const _validateTimeRange = useCallback(({ nextTimeRange }) => dateTimeValidate(nextTimeRange, limitDuration, formatTime), [formatTime, limitDuration]);
+  const initialTimeRange = useMemo(() => ({ nextTimeRange: onInitializingNextTimeRange(currentTimeRange) }), [currentTimeRange]);
+
   return (
     <StyledPopover id="timerange-type"
                    data-testid="timerange-type"
@@ -235,8 +269,8 @@ const TimeRangeDropdown = ({
                    arrowOffsetTop={positionIsBottom ? undefined : 25}
                    arrowOffsetLeft={positionIsBottom ? 34 : -11}
                    title={title}>
-      <Formik<TimeRangeDropDownFormValues> initialValues={{ nextTimeRange: onInitializingNextTimeRange(currentTimeRange) }}
-                                           validate={({ nextTimeRange }) => dateTimeValidate(nextTimeRange, limitDuration, formatTime)}
+      <Formik<TimeRangeDropDownFormValues> 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 = ({
                   <Col md={12}>
                     <TimeRangeLivePreview timerange={normalizeIfClassifiedRelativeTimeRange(nextTimeRange)} />
 
-                    <StyledTabs id="dateTimeTypes"
-                                defaultActiveKey={availableTimeRangeTypes[0].type}
-                                activeKey={activeTab ?? -1}
-                                onSelect={handleActiveTab}
-                                animation={false}>
-                      {timeRangeTypeTabs({
-                        activeTab,
-                        limitDuration,
-                        setValidatingKeyword,
-                        tabs: validTypes,
-                      })}
-
-                      {!activeTab && (<TabDisabledTimeRange />)}
-
-                    </StyledTabs>
+                    <TimeRangeTabs currentTimeRange={currentTimeRange}
+                                   handleActiveTab={handleActiveTab}
+                                   limitDuration={limitDuration}
+                                   validTypes={validTypes}
+                                   setValidatingKeyword={setValidatingKeyword} />
                   </Col>
                 </Row>