fix(ui): handle React 18 batching for "Submit" button on details page. …

…Fixes #13453 (#13593) Signed-off-by: Mason Malone <[email protected]>
argoproj · Oct 1, 2024 · 68adbcc · 68adbcc
1 parent ca6c414
commit 68adbcc
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 43 deletions.
diff --git a/ui/src/app/cluster-workflow-templates/cluster-workflow-template-details.tsx b/ui/src/app/cluster-workflow-templates/cluster-workflow-template-details.tsx
@@ -15,6 +15,7 @@ import {ZeroState} from '../shared/components/zero-state';
 import {Context} from '../shared/context';
 import {historyUrl} from '../shared/history';
 import {services} from '../shared/services';
+import {useEditableObject} from '../shared/use-editable-object';
 import {useQueryParams} from '../shared/use-query-params';
 import * as nsUtils from '../shared/namespaces';
 import {WorkflowDetailsList} from '../workflows/components/workflow-details-list/workflow-details-list';
@@ -36,8 +37,7 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
     const [columns, setColumns] = useState<models.Column[]>([]);
 
     const [error, setError] = useState<Error>();
-    const [template, setTemplate] = useState<ClusterWorkflowTemplate>();
-    const [edited, setEdited] = useState(false);
+    const [template, edited, setTemplate, resetTemplate] = useEditableObject<ClusterWorkflowTemplate>();
 
     useEffect(
         useQueryParams(history, p => {
@@ -47,7 +47,6 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
         [history]
     );
 
-    useEffect(() => setEdited(true), [template]);
     useEffect(() => {
         history.push(historyUrl('cluster-workflow-templates/{name}', {name, sidePanel, tab}));
     }, [name, sidePanel, tab]);
@@ -56,8 +55,7 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
         (async () => {
             try {
                 const newTemplate = await services.clusterWorkflowTemplate.get(name);
-                setTemplate(newTemplate);
-                setEdited(false); // set back to false
+                resetTemplate(newTemplate);
                 setError(null);
             } catch (err) {
                 setError(err);
@@ -106,15 +104,14 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
                             action: () => {
                                 services.clusterWorkflowTemplate
                                     .update(template, name)
-                                    .then(setTemplate)
+                                    .then(resetTemplate)
                                     .then(() =>
                                         notifications.show({
                                             content: 'Updated',
                                             type: NotificationType.Success
                                         })
                                     )
                                     .then(() => setError(null))
-                                    .then(() => setEdited(false))
                                     .catch(setError);
                             }
                         },

diff --git a/ui/src/app/cron-workflows/cron-workflow-details.tsx b/ui/src/app/cron-workflows/cron-workflow-details.tsx
@@ -17,6 +17,7 @@ import {Context} from '../shared/context';
 import {historyUrl} from '../shared/history';
 import {services} from '../shared/services';
 import {useQueryParams} from '../shared/use-query-params';
+import {useEditableObject} from '../shared/use-editable-object';
 import {WidgetGallery} from '../widgets/widget-gallery';
 import {WorkflowDetailsList} from '../workflows/components/workflow-details-list/workflow-details-list';
 import {CronWorkflowEditor} from './cron-workflow-editor';
@@ -35,8 +36,7 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
     const [workflows, setWorkflows] = useState<Workflow[]>([]);
     const [columns, setColumns] = useState<models.Column[]>([]);
 
-    const [cronWorkflow, setCronWorkflow] = useState<CronWorkflow>();
-    const [edited, setEdited] = useState(false);
+    const [cronWorkflow, edited, setCronWorkflow, resetCronWorkflow] = useEditableObject<CronWorkflow>();
     const [error, setError] = useState<Error>();
 
     useEffect(
@@ -63,14 +63,11 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
     useEffect(() => {
         services.cronWorkflows
             .get(name, namespace)
-            .then(setCronWorkflow)
-            .then(() => setEdited(false))
+            .then(resetCronWorkflow)
             .then(() => setError(null))
             .catch(setError);
     }, [namespace, name]);
 
-    useEffect(() => setEdited(true), [cronWorkflow]);
-
     useEffect(() => {
         (async () => {
             const workflowList = await services.workflows.list(namespace, null, [`${models.labels.cronWorkflow}=${name}`], {limit: 50});
@@ -91,8 +88,7 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
                   action: () =>
                       services.cronWorkflows
                           .suspend(name, namespace)
-                          .then(setCronWorkflow)
-                          .then(() => setEdited(false))
+                          .then(resetCronWorkflow)
                           .then(() => setError(null))
                           .catch(setError),
                   disabled: !cronWorkflow || edited
@@ -103,8 +99,7 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
                   action: () =>
                       services.cronWorkflows
                           .resume(name, namespace)
-                          .then(setCronWorkflow)
-                          .then(() => setEdited(false))
+                          .then(resetCronWorkflow)
                           .then(() => setError(null))
                           .catch(setError),
                   disabled: !cronWorkflow || !cronWorkflow.spec.suspend || edited
@@ -142,10 +137,9 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
                                 cronWorkflow.metadata.namespace
                             )
                         )
-                        .then(setCronWorkflow)
+                        .then(resetCronWorkflow)
                         .then(() => notifications.show({content: 'Updated', type: NotificationType.Success}))
                         .then(() => setError(null))
-                        .then(() => setEdited(false))
                         .catch(setError);
                 }
             },

diff --git a/ui/src/app/event-sources/event-source-details.tsx b/ui/src/app/event-sources/event-source-details.tsx
@@ -16,6 +16,7 @@ import {Context} from '../shared/context';
 import {historyUrl} from '../shared/history';
 import {services} from '../shared/services';
 import {useQueryParams} from '../shared/use-query-params';
+import {useEditableObject} from '../shared/use-editable-object';
 import {EventsPanel} from '../workflows/components/events-panel';
 import {EventSourceEditor} from './event-source-editor';
 import {EventSourceLogsViewer} from './event-source-log-viewer';
@@ -52,9 +53,8 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
         [namespace, name, tab, selectedNode]
     );
 
-    const [edited, setEdited] = useState(false);
     const [error, setError] = useState<Error>();
-    const [eventSource, setEventSource] = useState<EventSource>();
+    const [eventSource, edited, setEventSource, resetEventSource] = useEditableObject<EventSource>();
 
     const selected = (() => {
         if (!selectedNode) {
@@ -69,17 +69,14 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
         (async () => {
             try {
                 const newEventSource = await services.eventSource.get(name, namespace);
-                setEventSource(newEventSource);
-                setEdited(false); // set back to false
+                resetEventSource(newEventSource);
                 setError(null);
             } catch (err) {
                 setError(err);
             }
         })();
     }, [name, namespace]);
 
-    useEffect(() => setEdited(true), [eventSource]);
-
     useCollectEvent('openedEventSourceDetails');
 
     return (
@@ -100,14 +97,13 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
                             action: () =>
                                 services.eventSource
                                     .update(eventSource, name, namespace)
-                                    .then(setEventSource)
+                                    .then(resetEventSource)
                                     .then(() =>
                                         notifications.show({
                                             content: 'Updated',
                                             type: NotificationType.Success
                                         })
                                     )
-                                    .then(() => setEdited(false))
                                     .then(() => setError(null))
                                     .catch(setError)
                         },

diff --git a/ui/src/app/sensors/sensor-details.tsx b/ui/src/app/sensors/sensor-details.tsx
@@ -15,6 +15,7 @@ import {Context} from '../shared/context';
 import {historyUrl} from '../shared/history';
 import {services} from '../shared/services';
 import {useQueryParams} from '../shared/use-query-params';
+import {useEditableObject} from '../shared/use-editable-object';
 import {SensorEditor} from './sensor-editor';
 import {SensorSidePanel} from './sensor-side-panel';
 
@@ -29,8 +30,7 @@ export function SensorDetails({match, location, history}: RouteComponentProps<an
     const [name] = useState(match.params.name);
     const [tab, setTab] = useState<string>(queryParams.get('tab'));
 
-    const [sensor, setSensor] = useState<Sensor>();
-    const [edited, setEdited] = useState(false);
+    const [sensor, edited, setSensor, resetSensor] = useEditableObject<Sensor>();
     const [selectedLogNode, setSelectedLogNode] = useState<Node>(queryParams.get('selectedLogNode'));
     const [error, setError] = useState<Error>();
 
@@ -58,14 +58,11 @@ export function SensorDetails({match, location, history}: RouteComponentProps<an
     useEffect(() => {
         services.sensor
             .get(name, namespace)
-            .then(setSensor)
-            .then(() => setEdited(false))
+            .then(resetSensor)
             .then(() => setError(null))
             .catch(setError);
     }, [namespace, name]);
 
-    useEffect(() => setEdited(true), [sensor]);
-
     useCollectEvent('openedSensorDetails');
 
     const selected = (() => {
@@ -94,9 +91,8 @@ export function SensorDetails({match, location, history}: RouteComponentProps<an
                             action: () =>
                                 services.sensor
                                     .update(sensor, namespace)
-                                    .then(setSensor)
+                                    .then(resetSensor)
                                     .then(() => notifications.show({content: 'Updated', type: NotificationType.Success}))
-                                    .then(() => setEdited(false))
                                     .then(() => setError(null))
                                     .catch(setError)
                         },

diff --git a/ui/src/app/shared/use-editable-object.ts b/ui/src/app/shared/use-editable-object.ts
@@ -0,0 +1,21 @@
+import {useState} from 'react';
+
+/**
+ * useEditableObject is a React hook to manage the state of object that can be edited and updated.
+ * Uses ref comparisons to determine whether the resource has been edited.
+ */
+export function useEditableObject<T>(initial?: T): [T, boolean, React.Dispatch<T>, (value: T) => void] {
+    const [value, setValue] = useState<T>(initial);
+    const [initialValue, setInitialValue] = useState<T>(initial);
+
+    // Note: This is a pure reference comparison instead of a deep comparison for performance
+    // reasons, since <ObjectEditor> changes are latency-sensitive.
+    const edited = value !== initialValue;
+
+    function resetValue(value: T) {
+        setValue(value);
+        setInitialValue(value);
+    }
+
+    return [value, edited, setValue, resetValue];
+}
diff --git a/ui/src/app/workflow-templates/workflow-template-details.tsx b/ui/src/app/workflow-templates/workflow-template-details.tsx
@@ -10,6 +10,7 @@ import {WorkflowTemplate, Workflow} from '../../models';
 import {uiUrl} from '../shared/base';
 import {ErrorNotice} from '../shared/components/error-notice';
 import {Loading} from '../shared/components/loading';
+import {useEditableObject} from '../shared/use-editable-object';
 import {useCollectEvent} from '../shared/use-collect-event';
 import {ZeroState} from '../shared/components/zero-state';
 import {Context} from '../shared/context';
@@ -34,11 +35,8 @@ export function WorkflowTemplateDetails({history, location, match}: RouteCompone
     const [workflows, setWorkflows] = useState<Workflow[]>([]);
     const [columns, setColumns] = useState<models.Column[]>([]);
 
-    const [template, setTemplate] = useState<WorkflowTemplate>();
+    const [template, edited, setTemplate, resetTemplate] = useEditableObject<WorkflowTemplate>();
     const [error, setError] = useState<Error>();
-    const [edited, setEdited] = useState(false);
-
-    useEffect(() => setEdited(true), [template]);
 
     useEffect(
         useQueryParams(history, p => {
@@ -64,8 +62,7 @@ export function WorkflowTemplateDetails({history, location, match}: RouteCompone
     useEffect(() => {
         services.workflowTemplate
             .get(name, namespace)
-            .then(setTemplate)
-            .then(() => setEdited(false)) // set back to false
+            .then(resetTemplate)
             .then(() => setError(null))
             .catch(setError);
     }, [name, namespace]);
@@ -106,9 +103,8 @@ export function WorkflowTemplateDetails({history, location, match}: RouteCompone
                             action: () =>
                                 services.workflowTemplate
                                     .update(template, name, namespace)
-                                    .then(setTemplate)
+                                    .then(resetTemplate)
                                     .then(() => notifications.show({content: 'Updated', type: NotificationType.Success}))
-                                    .then(() => setEdited(false))
                                     .then(() => setError(null))
                                     .catch(setError)
                         },