From 60b91bac62db14cf4629f068be5558487d9c6ed8 Mon Sep 17 00:00:00 2001 From: Mason Malone <651224+MasonM@users.noreply.github.com> Date: Sat, 16 Nov 2024 21:35:12 -0800 Subject: [PATCH] fix(ui): improve editor performance and fix Submit button. Fixes #1382 The `` component used by all the edit pages encapsulates the logic for serializing/deserializing the object being edited, but it's extremely inefficient. For example, if you're editing a `WorkflowTemplate`, every single keystroke will cause `parse()` to be called twice and `stringify()` to be called 4 times. For large documents, this serializatino/deserialization overhead adds up quickly. Additionally, the "Submit" button is sometimes incorrectly grayed out, e.g. after clicking the "Update" button. This is happening because `useEditableObject()` is using reference comparisons on the object being edited, since it doesn't have access to the serialized string version encapsulated by ``. This decouples `` from serialization/deseralization and lifts the state involved up to the `useEditableObject()` hook, as discussed in https://github.com/argoproj/argo-workflows/pull/13593#discussion_r1781972661. Doing so eliminates unnecessary serialization/deseralization: now, if you're editing a `WorkflowTemplate`, each keystroke will only call `parse()` once and won't call `stringify()` at all. This also fixes issues with the Submit button not reflecting the current page state, since `useEditableObject()` now uses string comparisons instead of ref comparisons. Signed-off-by: Mason Malone <651224+MasonM@users.noreply.github.com> --- .../cluster-workflow-template-creator.tsx | 7 +- .../cluster-workflow-template-details.tsx | 15 ++- .../cluster-workflow-template-editor.tsx | 58 ---------- .../cron-workflows/cron-workflow-creator.tsx | 5 +- .../cron-workflows/cron-workflow-details.tsx | 13 ++- .../cron-workflows/cron-workflow-editor.tsx | 22 +++- ui/src/event-flow/event-flow-page.tsx | 4 +- ui/src/event-sources/event-source-creator.tsx | 5 +- ui/src/event-sources/event-source-details.tsx | 13 ++- ui/src/event-sources/event-source-editor.tsx | 22 +++- ui/src/sensors/sensor-creator.tsx | 5 +- ui/src/sensors/sensor-details.tsx | 17 ++- ui/src/sensors/sensor-editor.tsx | 15 ++- ui/src/shared/components/object-editor.tsx | 44 +++----- ui/src/shared/components/object-parser.ts | 6 +- .../resource-editor/resource-editor.tsx | 69 ------------ .../components/resource-editor/resource.scss | 20 ---- ui/src/shared/use-editable-object.test.ts | 105 ++++++++++++++++++ ui/src/shared/use-editable-object.ts | 89 ++++++++++++--- .../workflow-event-bindings.tsx | 4 +- .../workflow-template-creator.tsx | 5 +- .../workflow-template-details.tsx | 18 ++- .../workflow-template-editor.tsx | 22 +++- .../workflows/components/workflow-creator.tsx | 5 +- .../workflow-resource-panel.tsx | 4 +- .../workflows/components/workflow-editor.tsx | 28 ++++- .../workflow-yaml-viewer.tsx | 10 +- 27 files changed, 386 insertions(+), 244 deletions(-) delete mode 100644 ui/src/cluster-workflow-templates/cluster-workflow-template-editor.tsx delete mode 100644 ui/src/shared/components/resource-editor/resource-editor.tsx delete mode 100644 ui/src/shared/components/resource-editor/resource.scss create mode 100644 ui/src/shared/use-editable-object.test.ts diff --git a/ui/src/cluster-workflow-templates/cluster-workflow-template-creator.tsx b/ui/src/cluster-workflow-templates/cluster-workflow-template-creator.tsx index 1bd88a07f137..df3eb397294d 100644 --- a/ui/src/cluster-workflow-templates/cluster-workflow-template-creator.tsx +++ b/ui/src/cluster-workflow-templates/cluster-workflow-template-creator.tsx @@ -8,10 +8,11 @@ import {UploadButton} from '../shared/components/upload-button'; import {exampleClusterWorkflowTemplate} from '../shared/examples'; import {ClusterWorkflowTemplate} from '../shared/models'; import {services} from '../shared/services'; -import {ClusterWorkflowTemplateEditor} from './cluster-workflow-template-editor'; +import {useEditableObject} from '../shared/use-editable-object'; +import {WorkflowTemplateEditor} from '../workflow-templates/workflow-template-editor'; export function ClusterWorkflowTemplateCreator({onCreate}: {onCreate: (workflow: ClusterWorkflowTemplate) => void}) { - const [template, setTemplate] = useState(exampleClusterWorkflowTemplate()); + const {object: template, setObject: setTemplate, serialization, lang, setLang} = useEditableObject(exampleClusterWorkflowTemplate()); const [error, setError] = useState(); return ( <> @@ -31,7 +32,7 @@ export function ClusterWorkflowTemplateCreator({onCreate}: {onCreate: (workflow: - +
.
diff --git a/ui/src/cluster-workflow-templates/cluster-workflow-template-details.tsx b/ui/src/cluster-workflow-templates/cluster-workflow-template-details.tsx index 9a3b6a96904f..8930097cdd5e 100644 --- a/ui/src/cluster-workflow-templates/cluster-workflow-template-details.tsx +++ b/ui/src/cluster-workflow-templates/cluster-workflow-template-details.tsx @@ -18,9 +18,9 @@ import {services} from '../shared/services'; import {useCollectEvent} from '../shared/use-collect-event'; import {useEditableObject} from '../shared/use-editable-object'; import {useQueryParams} from '../shared/use-query-params'; +import {WorkflowTemplateEditor} from '../workflow-templates/workflow-template-editor'; import {SubmitWorkflowPanel} from '../workflows/components/submit-workflow-panel'; import {WorkflowDetailsList} from '../workflows/components/workflow-details-list/workflow-details-list'; -import {ClusterWorkflowTemplateEditor} from './cluster-workflow-template-editor'; import '../workflows/components/workflow-details/workflow-details.scss'; @@ -37,7 +37,7 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route const [columns, setColumns] = useState([]); const [error, setError] = useState(); - const [template, edited, setTemplate, resetTemplate] = useEditableObject(); + const {object: template, setObject: setTemplate, resetObject: resetTemplate, serialization, edited, lang, setLang} = useEditableObject(); useEffect( useQueryParams(history, p => { @@ -138,7 +138,16 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route {!template ? ( ) : ( - + )} {template && ( diff --git a/ui/src/cluster-workflow-templates/cluster-workflow-template-editor.tsx b/ui/src/cluster-workflow-templates/cluster-workflow-template-editor.tsx deleted file mode 100644 index a0241ffea71e..000000000000 --- a/ui/src/cluster-workflow-templates/cluster-workflow-template-editor.tsx +++ /dev/null @@ -1,58 +0,0 @@ -import {Tabs} from 'argo-ui/src/components/tabs/tabs'; -import * as React from 'react'; - -import {LabelsAndAnnotationsEditor} from '../shared/components/editors/labels-and-annotations-editor'; -import {MetadataEditor} from '../shared/components/editors/metadata-editor'; -import {WorkflowParametersEditor} from '../shared/components/editors/workflow-parameters-editor'; -import {ObjectEditor} from '../shared/components/object-editor'; -import {WorkflowTemplate} from '../shared/models'; - -export function ClusterWorkflowTemplateEditor({ - onChange, - template, - onError, - onTabSelected, - selectedTabKey -}: { - template: WorkflowTemplate; - onChange: (template: WorkflowTemplate) => void; - onError: (error: Error) => void; - onTabSelected?: (tab: string) => void; - selectedTabKey?: string; -}) { - return ( - onChange({...x})} /> - }, - { - key: 'spec', - title: 'Spec', - content: onChange({...template, spec})} onError={onError} /> - }, - { - key: 'metadata', - title: 'MetaData', - content: onChange({...template, metadata})} /> - }, - { - key: 'workflow-metadata', - title: 'Workflow MetaData', - content: ( - onChange({...template, spec: {...template.spec, workflowMetadata}})} - /> - ) - } - ]} - /> - ); -} diff --git a/ui/src/cron-workflows/cron-workflow-creator.tsx b/ui/src/cron-workflows/cron-workflow-creator.tsx index 095eda7ddeca..bb09fb75fa0d 100644 --- a/ui/src/cron-workflows/cron-workflow-creator.tsx +++ b/ui/src/cron-workflows/cron-workflow-creator.tsx @@ -9,10 +9,11 @@ import {exampleCronWorkflow} from '../shared/examples'; import {CronWorkflow} from '../shared/models'; import * as nsUtils from '../shared/namespaces'; import {services} from '../shared/services'; +import {useEditableObject} from '../shared/use-editable-object'; import {CronWorkflowEditor} from './cron-workflow-editor'; export function CronWorkflowCreator({onCreate, namespace}: {namespace: string; onCreate: (cronWorkflow: CronWorkflow) => void}) { - const [cronWorkflow, setCronWorkflow] = useState(exampleCronWorkflow(nsUtils.getNamespaceWithDefault(namespace))); + const {object: cronWorkflow, setObject: setCronWorkflow, serialization, lang, setLang} = useEditableObject(exampleCronWorkflow(nsUtils.getNamespaceWithDefault(namespace))); const [error, setError] = useState(); return ( <> @@ -32,7 +33,7 @@ export function CronWorkflowCreator({onCreate, namespace}: {namespace: string; o - +

.

diff --git a/ui/src/cron-workflows/cron-workflow-details.tsx b/ui/src/cron-workflows/cron-workflow-details.tsx index ca7eaf1f2ef2..566ccf766a4c 100644 --- a/ui/src/cron-workflows/cron-workflow-details.tsx +++ b/ui/src/cron-workflows/cron-workflow-details.tsx @@ -36,7 +36,7 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr const [workflows, setWorkflows] = useState([]); const [columns, setColumns] = useState([]); - const [cronWorkflow, edited, setCronWorkflow, resetCronWorkflow] = useEditableObject(); + const {object: cronWorkflow, setObject: setCronWorkflow, resetObject: resetCronWorkflow, serialization, edited, lang, setLang} = useEditableObject(); const [error, setError] = useState(); useEffect( @@ -207,7 +207,16 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr {!cronWorkflow ? ( ) : ( - + )} setSidePanel(null)}> {sidePanel === 'share' && } diff --git a/ui/src/cron-workflows/cron-workflow-editor.tsx b/ui/src/cron-workflows/cron-workflow-editor.tsx index e453157e34e0..fcad11038366 100644 --- a/ui/src/cron-workflows/cron-workflow-editor.tsx +++ b/ui/src/cron-workflows/cron-workflow-editor.tsx @@ -5,6 +5,7 @@ import {LabelsAndAnnotationsEditor} from '../shared/components/editors/labels-an import {MetadataEditor} from '../shared/components/editors/metadata-editor'; import {WorkflowParametersEditor} from '../shared/components/editors/workflow-parameters-editor'; import {ObjectEditor} from '../shared/components/object-editor'; +import type {Lang} from '../shared/components/object-parser'; import {CronWorkflow} from '../shared/models'; import {CronWorkflowSpecEditor} from './cron-workflow-spec-editior'; import {CronWorkflowStatusViewer} from './cron-workflow-status-viewer'; @@ -14,10 +15,16 @@ export function CronWorkflowEditor({ onTabSelected, onError, onChange, - cronWorkflow + onLangChange, + cronWorkflow, + serialization, + lang }: { cronWorkflow: CronWorkflow; - onChange: (cronWorkflow: CronWorkflow) => void; + serialization: string; + lang: Lang; + onChange: (cronWorkflow: string | CronWorkflow) => void; + onLangChange: (lang: Lang) => void; onError: (error: Error) => void; onTabSelected?: (tab: string) => void; selectedTabKey?: string; @@ -41,7 +48,16 @@ export function CronWorkflowEditor({ { key: 'manifest', title: 'Manifest', - content: onChange({...x})} /> + content: ( + + ) }, { key: 'cron', diff --git a/ui/src/event-flow/event-flow-page.tsx b/ui/src/event-flow/event-flow-page.tsx index 95c74fe8a191..bdbb07e9cf29 100644 --- a/ui/src/event-flow/event-flow-page.tsx +++ b/ui/src/event-flow/event-flow-page.tsx @@ -15,7 +15,7 @@ import {GraphPanel} from '../shared/components/graph/graph-panel'; import {Node} from '../shared/components/graph/types'; import {Links} from '../shared/components/links'; import {NamespaceFilter} from '../shared/components/namespace-filter'; -import {ResourceEditor} from '../shared/components/resource-editor/resource-editor'; +import {SerializingObjectEditor} from '../shared/components/object-editor'; import {ZeroState} from '../shared/components/zero-state'; import {Context} from '../shared/context'; import {Footnote} from '../shared/footnote'; @@ -317,7 +317,7 @@ export function EventFlowPage({history, location, match}: RouteComponentProps + content: }, { title: 'LOGS', diff --git a/ui/src/event-sources/event-source-creator.tsx b/ui/src/event-sources/event-source-creator.tsx index aff598faf78a..6fc89e4c2580 100644 --- a/ui/src/event-sources/event-source-creator.tsx +++ b/ui/src/event-sources/event-source-creator.tsx @@ -8,10 +8,11 @@ import {exampleEventSource} from '../shared/examples'; import {EventSource} from '../shared/models'; import * as nsUtils from '../shared/namespaces'; import {services} from '../shared/services'; +import {useEditableObject} from '../shared/use-editable-object'; import {EventSourceEditor} from './event-source-editor'; export function EventSourceCreator({onCreate, namespace}: {namespace: string; onCreate: (eventSource: EventSource) => void}) { - const [eventSource, setEventSource] = useState(exampleEventSource(nsUtils.getNamespaceWithDefault(namespace))); + const {object: eventSource, setObject: setEventSource, serialization, lang, setLang} = useEditableObject(exampleEventSource(nsUtils.getNamespaceWithDefault(namespace))); const [error, setError] = useState(); return ( <> @@ -31,7 +32,7 @@ export function EventSourceCreator({onCreate, namespace}: {namespace: string; on - +

Example event sources diff --git a/ui/src/event-sources/event-source-details.tsx b/ui/src/event-sources/event-source-details.tsx index a31d5ba66451..0b6cbc1a17a0 100644 --- a/ui/src/event-sources/event-source-details.tsx +++ b/ui/src/event-sources/event-source-details.tsx @@ -54,7 +54,7 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro ); const [error, setError] = useState(); - const [eventSource, edited, setEventSource, resetEventSource] = useEditableObject(); + const {object: eventSource, setObject: setEventSource, resetObject: resetEventSource, serialization, edited, lang, setLang} = useEditableObject(); const selected = (() => { if (!selectedNode) { @@ -139,7 +139,16 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro {!eventSource ? ( ) : ( - + )} setSelectedNode(null)}> diff --git a/ui/src/event-sources/event-source-editor.tsx b/ui/src/event-sources/event-source-editor.tsx index a5310d3d2bb5..f05ea6c4801d 100644 --- a/ui/src/event-sources/event-source-editor.tsx +++ b/ui/src/event-sources/event-source-editor.tsx @@ -3,16 +3,23 @@ import * as React from 'react'; import {MetadataEditor} from '../shared/components/editors/metadata-editor'; import {ObjectEditor} from '../shared/components/object-editor'; +import type {Lang} from '../shared/components/object-parser'; import {EventSource} from '../shared/models'; export function EventSourceEditor({ onChange, + onLangChange, onTabSelected, selectedTabKey, - eventSource + eventSource, + serialization, + lang }: { eventSource: EventSource; - onChange: (template: EventSource) => void; + serialization: string; + lang: Lang; + onChange: (template: string | EventSource) => void; + onLangChange: (lang: Lang) => void; onError: (error: Error) => void; onTabSelected?: (tab: string) => void; selectedTabKey?: string; @@ -27,7 +34,16 @@ export function EventSourceEditor({ { key: 'manifest', title: 'Manifest', - content: onChange({...x})} /> + content: ( + + ) }, { diff --git a/ui/src/sensors/sensor-creator.tsx b/ui/src/sensors/sensor-creator.tsx index 25d2a0950e7b..8de396a92d86 100644 --- a/ui/src/sensors/sensor-creator.tsx +++ b/ui/src/sensors/sensor-creator.tsx @@ -8,10 +8,11 @@ import {exampleSensor} from '../shared/examples'; import {Sensor} from '../shared/models'; import * as nsUtils from '../shared/namespaces'; import {services} from '../shared/services'; +import {useEditableObject} from '../shared/use-editable-object'; import {SensorEditor} from './sensor-editor'; export function SensorCreator({namespace, onCreate}: {namespace: string; onCreate: (sensor: Sensor) => void}) { - const [sensor, setSensor] = useState(exampleSensor(nsUtils.getNamespaceWithDefault(namespace))); + const {object: sensor, setObject: setSensor, serialization, lang, setLang} = useEditableObject(exampleSensor(nsUtils.getNamespaceWithDefault(namespace))); const [error, setError] = useState(); return ( <> @@ -31,7 +32,7 @@ export function SensorCreator({namespace, onCreate}: {namespace: string; onCreat - +

Example sensors diff --git a/ui/src/sensors/sensor-details.tsx b/ui/src/sensors/sensor-details.tsx index e4388bb760a9..e8919dc113ee 100644 --- a/ui/src/sensors/sensor-details.tsx +++ b/ui/src/sensors/sensor-details.tsx @@ -30,7 +30,7 @@ export function SensorDetails({match, location, history}: RouteComponentProps(queryParams.get('tab')); - const [sensor, edited, setSensor, resetSensor] = useEditableObject(); + const {object: sensor, setObject: setSensor, resetObject: resetSensor, serialization, edited, lang, setLang} = useEditableObject(); const [selectedLogNode, setSelectedLogNode] = useState(queryParams.get('selectedLogNode')); const [error, setError] = useState(); @@ -125,7 +125,20 @@ export function SensorDetails({match, location, history}: RouteComponentProps <> - {!sensor ? : } + {!sensor ? ( + + ) : ( + + )} {!!selectedLogNode && ( void; + serialization: string; + lang: Lang; + onChange: (template: string | Sensor) => void; + onLangChange: (lang: Lang) => void; onError: (error: Error) => void; onTabSelected?: (tab: string) => void; selectedTabKey?: string; @@ -27,7 +34,9 @@ export function SensorEditor({ { key: 'manifest', title: 'Manifest', - content: onChange({...x})} /> + content: ( + + ) }, { key: 'metadata', diff --git a/ui/src/shared/components/object-editor.tsx b/ui/src/shared/components/object-editor.tsx index ea79be418e83..c9c3cb37bbd5 100644 --- a/ui/src/shared/components/object-editor.tsx +++ b/ui/src/shared/components/object-editor.tsx @@ -3,44 +3,31 @@ import {useEffect, useRef, useState} from 'react'; import MonacoEditor from 'react-monaco-editor'; import {uiUrl} from '../base'; -import {ScopedLocalStorage} from '../scoped-local-storage'; +import {useEditableObject} from '../use-editable-object'; import {Button} from './button'; -import {parse, stringify} from './object-parser'; +import type {Lang} from './object-parser'; import {PhaseIcon} from './phase-icon'; import {SuspenseMonacoEditor} from './suspense-monaco-editor'; interface Props { type?: string; value: T; - buttons?: React.ReactNode; - onChange?: (value: T) => void; + lang: Lang; + text: string; + onLangChange: (lang: Lang) => void; + onChange?: (value: string) => void; } -const defaultLang = 'yaml'; - -export function ObjectEditor({type, value, buttons, onChange}: Props) { - const storage = new ScopedLocalStorage('object-editor'); +export function ObjectEditor({type, value, text, lang, onChange, onLangChange}: Props) { const [error, setError] = useState(); - const [lang, setLang] = useState(storage.getItem('lang', defaultLang)); - const [text, setText] = useState(stringify(value, lang)); const editor = useRef(null); - useEffect(() => storage.setItem('lang', lang, defaultLang), [lang]); - useEffect(() => setText(stringify(value, lang)), [value]); - useEffect(() => setText(stringify(parse(text), lang)), [lang]); useEffect(() => { - if (!editor.current) { + if (!editor.current || text === editor.current.editor.getValue()) { return; } - - // we ONLY want to change the text, if the normalized version has changed, this prevents white-space changes - // from resulting in a significant change - const editorText = stringify(parse(editor.current.editor.getValue()), lang); - const editorLang = editor.current.editor.getValue().startsWith('{') ? 'json' : 'yaml'; - if (text !== editorText || lang !== editorLang) { - editor.current.editor.setValue(stringify(parse(text), lang)); - } - }, [editor, text, lang]); + editor.current.editor.setValue(text); + }, [editor, text]); useEffect(() => { if (!type || lang !== 'json') { @@ -83,7 +70,7 @@ export function ObjectEditor({type, value, buttons, onChange}: Props) { return ( <>

({type, value, buttons, onChange}: Props) { onChange={v => { if (onChange) { try { - onChange(parse(v)); + onChange(v); setError(null); } catch (e) { setError(e); @@ -153,3 +139,9 @@ export function ObjectEditor({type, value, buttons, onChange}: Props) { ); } + +/** Wrapper for ObjectEditor that automatically handles serializing/deserializing the object using useEditableObject() */ +export function SerializingObjectEditor({type, value}: {type?: string; value: T}) { + const {object, setObject, serialization, lang, setLang} = useEditableObject(value); + return ; +} diff --git a/ui/src/shared/components/object-parser.ts b/ui/src/shared/components/object-parser.ts index 7b4800abf660..d6ec1fde74ce 100644 --- a/ui/src/shared/components/object-parser.ts +++ b/ui/src/shared/components/object-parser.ts @@ -1,5 +1,7 @@ import YAML from 'yaml'; +export type Lang = 'json' | 'yaml'; + export function parse(value: string): T { if (value.startsWith('{')) { return JSON.parse(value); @@ -12,6 +14,6 @@ export function parse(value: string): T { }) as T; } -export function stringify(value: T, type: string) { - return type === 'yaml' ? YAML.stringify(value, {aliasDuplicateObjects: false}) : JSON.stringify(value, null, ' '); +export function stringify(value: T, lang: Lang) { + return lang === 'yaml' ? YAML.stringify(value, {aliasDuplicateObjects: false}) : JSON.stringify(value, null, ' '); } diff --git a/ui/src/shared/components/resource-editor/resource-editor.tsx b/ui/src/shared/components/resource-editor/resource-editor.tsx deleted file mode 100644 index 52f7132c067a..000000000000 --- a/ui/src/shared/components/resource-editor/resource-editor.tsx +++ /dev/null @@ -1,69 +0,0 @@ -import * as kubernetes from 'argo-ui/src/models/kubernetes'; -import * as React from 'react'; -import {useState} from 'react'; - -import {Button} from '../button'; -import {ErrorNotice} from '../error-notice'; -import {ObjectEditor} from '../object-editor'; -import {UploadButton} from '../upload-button'; - -interface Props { - kind?: string; - upload?: boolean; - namespace?: string; - title?: string; - value: T; - editing?: boolean; - onSubmit?: (value: T) => Promise; -} - -export function ResourceEditor(props: Props) { - const [editing, setEditing] = useState(props.editing); - const [value, setValue] = useState(props.value); - const [error, setError] = useState(); - - async function submit() { - try { - if (value.metadata && !value.metadata.namespace && props.namespace) { - value.metadata.namespace = props.namespace; - } - await props.onSubmit(value); - setError(null); - } catch (newError) { - setError(newError); - } - } - - return ( - <> - {props.title &&

{props.title}

} - - - {editing ? ( - <> - {props.upload && onUpload={setValue} onError={setError} />} - {props.onSubmit && ( - - )} - - ) : ( - props.onSubmit && ( - - ) - )} - - } - onChange={setValue} - /> - - ); -} diff --git a/ui/src/shared/components/resource-editor/resource.scss b/ui/src/shared/components/resource-editor/resource.scss deleted file mode 100644 index 71ffef0c5a57..000000000000 --- a/ui/src/shared/components/resource-editor/resource.scss +++ /dev/null @@ -1,20 +0,0 @@ -@import 'node_modules/argo-ui/src/styles/config'; - -.resource { - padding-top: 1em; - white-space: pre; - color: $argo-color-gray-7; - width: 100%; - min-height: 300px; -} - -.resource-editor-panel { - &__editor { - border: 1px solid $argo-color-gray-3; - border-radius: 5px; - margin-top: 1em; - padding-bottom: 1em; - overflow: hidden; - } -} - diff --git a/ui/src/shared/use-editable-object.test.ts b/ui/src/shared/use-editable-object.test.ts new file mode 100644 index 000000000000..11acd670927a --- /dev/null +++ b/ui/src/shared/use-editable-object.test.ts @@ -0,0 +1,105 @@ +import {createInitialState, reducer} from './use-editable-object'; + +describe('createInitialState', () => { + test('without object', () => { + expect(createInitialState()).toEqual({ + object: undefined, + serialization: null, + lang: 'yaml', + initialSerialization: null, + edited: false + }); + }); + + test('with object', () => { + expect(createInitialState({a: 1})).toEqual({ + object: {a: 1}, + serialization: 'a: 1\n', + lang: 'yaml', + initialSerialization: 'a: 1\n', + edited: false + }); + }); +}); + +describe('reducer', () => { + const testState = { + object: {a: 1}, + serialization: 'a: 1\n', + lang: 'yaml', + initialSerialization: 'a: 1\n', + edited: false + } as const; + + test('setLang unedited', () => { + const newState = reducer(testState, {type: 'setLang', payload: 'json'}); + expect(newState).toEqual({ + object: {a: 1}, + serialization: '{\n "a": 1\n}', + lang: 'json', + initialSerialization: '{\n "a": 1\n}', + edited: false + }); + }); + + test('setLang edited', () => { + const newState = reducer( + { + ...testState, + edited: true + }, + {type: 'setLang', payload: 'json'} + ); + expect(newState).toEqual({ + object: {a: 1}, + serialization: '{\n "a": 1\n}', + lang: 'json', + initialSerialization: 'a: 1\n', + edited: true + }); + }); + + test('setObject with string', () => { + const newState = reducer(testState, {type: 'setObject', payload: 'a: 2'}); + expect(newState).toEqual({ + object: {a: 2}, + serialization: 'a: 2', + lang: 'yaml', + initialSerialization: 'a: 1\n', + edited: true + }); + }); + + test('setObject with object', () => { + const newState = reducer(testState, {type: 'setObject', payload: {a: 2}}); + expect(newState).toEqual({ + object: {a: 2}, + serialization: 'a: 2\n', + lang: 'yaml', + initialSerialization: 'a: 1\n', + edited: true + }); + }); + + test('resetObject with string', () => { + const newState = reducer(testState, {type: 'resetObject', payload: 'a: 2'}); + expect(newState).toEqual({ + object: {a: 2}, + serialization: 'a: 2', + lang: 'yaml', + initialSerialization: 'a: 2', + edited: false + }); + }); + + test('resetObject with object', () => { + const newState = reducer(testState, {type: 'resetObject', payload: {a: 2}}); + expect(newState).toEqual({ + object: {a: 2}, + serialization: 'a: 2\n', + lang: 'yaml', + initialSerialization: 'a: 2\n', + edited: false + }); + }); +}); diff --git a/ui/src/shared/use-editable-object.ts b/ui/src/shared/use-editable-object.ts index bb18287af8a8..a04d3174b4ca 100644 --- a/ui/src/shared/use-editable-object.ts +++ b/ui/src/shared/use-editable-object.ts @@ -1,21 +1,80 @@ -import {useState} from 'react'; +import {useReducer} 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(initial?: T): [T, boolean, React.Dispatch, (value: T) => void] { - const [value, setValue] = useState(initial); - const [initialValue, setInitialValue] = useState(initial); +import {Lang, parse, stringify} from '../shared/components/object-parser'; +import {ScopedLocalStorage} from '../shared/scoped-local-storage'; + +type Action = {type: 'setLang'; payload: Lang} | {type: 'setObject'; payload: string | T} | {type: 'resetObject'; payload: string | T}; + +interface State { + /** The parsed form of the object, kept in sync with "serialization" */ + object: T; + /** The stringified form of the object, kept in sync with "object" */ + serialization: string; + /** The serialization language used (YAML or JSON) */ + lang: Lang; + /** The initial stringified form of the object. Used to check if it was edited */ + initialSerialization: string; + /** Whether any changes have been made */ + edited: boolean; +} - // Note: This is a pure reference comparison instead of a deep comparison for performance - // reasons, since changes are latency-sensitive. - const edited = value !== initialValue; +const defaultLang = 'yaml'; +const storage = new ScopedLocalStorage('object-editor'); - function resetValue(value: T) { - setValue(value); - setInitialValue(value); +export function reducer(state: State, action: Action) { + const newState = {...state}; + switch (action.type) { + case 'resetObject': + case 'setObject': + if (typeof action.payload === 'string') { + newState.object = parse(action.payload); + newState.serialization = action.payload; + } else { + newState.object = action.payload; + newState.serialization = stringify(action.payload, newState.lang); + } + if (action.type === 'resetObject') { + newState.initialSerialization = newState.serialization; + } + newState.edited = newState.initialSerialization !== newState.serialization; + return newState; + case 'setLang': + newState.lang = action.payload; + storage.setItem('lang', newState.lang, defaultLang); + newState.serialization = stringify(newState.object, newState.lang); + if (!newState.edited) { + newState.initialSerialization = newState.serialization; + } + return newState; } +} + +export function createInitialState(object?: T): State { + const lang = storage.getItem('lang', defaultLang); + const serialization = object ? stringify(object, lang) : null; + return { + object, + serialization, + lang, + initialSerialization: serialization, + edited: false + }; +} - return [value, edited, setValue, resetValue]; +/** + * useEditableObject is a React hook to manage the state of an object that can be serialized and edited, encapsulating the logic to + * parse/stringify the object as necessary. + */ +export function useEditableObject(object?: T): State & { + setObject: (value: string | T) => void; + resetObject: (value: string | T) => void; + setLang: (lang: Lang) => void; +} { + const [state, dispatch] = useReducer(reducer, object, createInitialState); + return { + ...state, + setObject: value => dispatch({type: 'setObject', payload: value}), + resetObject: value => dispatch({type: 'resetObject', payload: value}), + setLang: value => dispatch({type: 'setLang', payload: value}) + }; } diff --git a/ui/src/workflow-event-bindings/workflow-event-bindings.tsx b/ui/src/workflow-event-bindings/workflow-event-bindings.tsx index be0ca33793d2..b3556c41bfca 100644 --- a/ui/src/workflow-event-bindings/workflow-event-bindings.tsx +++ b/ui/src/workflow-event-bindings/workflow-event-bindings.tsx @@ -11,7 +11,7 @@ import {GraphPanel} from '../shared/components/graph/graph-panel'; import {Graph} from '../shared/components/graph/types'; import {Loading} from '../shared/components/loading'; import {NamespaceFilter} from '../shared/components/namespace-filter'; -import {ResourceEditor} from '../shared/components/resource-editor/resource-editor'; +import {SerializingObjectEditor} from '../shared/components/object-editor'; import {ZeroState} from '../shared/components/zero-state'; import {Context} from '../shared/context'; import {Footnote} from '../shared/footnote'; @@ -147,7 +147,7 @@ export function WorkflowEventBindings({match, location, history}: RouteComponent {introductionText} {learnMore}. setSelectedWorkflowEventBinding(null)}> - {selected && } + {selected && } )} diff --git a/ui/src/workflow-templates/workflow-template-creator.tsx b/ui/src/workflow-templates/workflow-template-creator.tsx index 0e8c33001999..34a3f7cecdd7 100644 --- a/ui/src/workflow-templates/workflow-template-creator.tsx +++ b/ui/src/workflow-templates/workflow-template-creator.tsx @@ -9,10 +9,11 @@ import {exampleWorkflowTemplate} from '../shared/examples'; import {WorkflowTemplate} from '../shared/models'; import * as nsUtils from '../shared/namespaces'; import {services} from '../shared/services'; +import {useEditableObject} from '../shared/use-editable-object'; import {WorkflowTemplateEditor} from './workflow-template-editor'; export function WorkflowTemplateCreator({namespace, onCreate}: {namespace: string; onCreate: (workflow: WorkflowTemplate) => void}) { - const [template, setTemplate] = useState(exampleWorkflowTemplate(nsUtils.getNamespaceWithDefault(namespace))); + const {object: template, setObject: setTemplate, serialization, lang, setLang} = useEditableObject(exampleWorkflowTemplate(nsUtils.getNamespaceWithDefault(namespace))); const [error, setError] = useState(); return ( <> @@ -32,7 +33,7 @@ export function WorkflowTemplateCreator({namespace, onCreate}: {namespace: strin
- +

.

diff --git a/ui/src/workflow-templates/workflow-template-details.tsx b/ui/src/workflow-templates/workflow-template-details.tsx index 8a8d869c3050..cdf3bd0778a2 100644 --- a/ui/src/workflow-templates/workflow-template-details.tsx +++ b/ui/src/workflow-templates/workflow-template-details.tsx @@ -34,8 +34,7 @@ export function WorkflowTemplateDetails({history, location, match}: RouteCompone const [tab, setTab] = useState(queryParams.get('tab')); const [workflows, setWorkflows] = useState([]); const [columns, setColumns] = useState([]); - - const [template, edited, setTemplate, resetTemplate] = useEditableObject(); + const {object: template, setObject: setTemplate, resetObject: resetTemplate, serialization, edited, lang, setLang} = useEditableObject(); const [error, setError] = useState(); useEffect( @@ -133,7 +132,20 @@ export function WorkflowTemplateDetails({history, location, match}: RouteCompone }}> <> - {!template ? : } + {!template ? ( + + ) : ( + + )} {template && ( setSidePanel(null)} isMiddle={sidePanel === 'submit'}> diff --git a/ui/src/workflow-templates/workflow-template-editor.tsx b/ui/src/workflow-templates/workflow-template-editor.tsx index c273df8ccc18..83a23e64450d 100644 --- a/ui/src/workflow-templates/workflow-template-editor.tsx +++ b/ui/src/workflow-templates/workflow-template-editor.tsx @@ -5,19 +5,26 @@ import {LabelsAndAnnotationsEditor} from '../shared/components/editors/labels-an import {MetadataEditor} from '../shared/components/editors/metadata-editor'; import {WorkflowParametersEditor} from '../shared/components/editors/workflow-parameters-editor'; import {ObjectEditor} from '../shared/components/object-editor'; +import type {Lang} from '../shared/components/object-parser'; import {WorkflowTemplate} from '../shared/models'; export function WorkflowTemplateEditor({ onChange, + onLangChange, onError, onTabSelected, selectedTabKey, - template + template, + serialization, + lang }: { template: WorkflowTemplate; - onChange: (template: WorkflowTemplate) => void; + serialization: string; + lang: Lang; + onChange: (template: string | WorkflowTemplate) => void; onError: (error: Error) => void; onTabSelected?: (tab: string) => void; + onLangChange: (lang: Lang) => void; selectedTabKey?: string; }) { return ( @@ -30,7 +37,16 @@ export function WorkflowTemplateEditor({ { key: 'manifest', title: 'Manifest', - content: onChange({...x})} /> + content: ( + + ) }, { key: 'spec', diff --git a/ui/src/workflows/components/workflow-creator.tsx b/ui/src/workflows/components/workflow-creator.tsx index a5e6c6e0e319..5be458fb82ce 100644 --- a/ui/src/workflows/components/workflow-creator.tsx +++ b/ui/src/workflows/components/workflow-creator.tsx @@ -10,6 +10,7 @@ import {exampleWorkflow} from '../../shared/examples'; import {Workflow, WorkflowTemplate} from '../../shared/models'; import * as nsUtils from '../../shared/namespaces'; import {services} from '../../shared/services'; +import {useEditableObject} from '../../shared/use-editable-object'; import {SubmitWorkflowPanel} from './submit-workflow-panel'; import {WorkflowEditor} from './workflow-editor'; @@ -19,7 +20,7 @@ export function WorkflowCreator({namespace, onCreate}: {namespace: string; onCre const [workflowTemplates, setWorkflowTemplates] = useState(); const [workflowTemplate, setWorkflowTemplate] = useState(); const [stage, setStage] = useState('choose-method'); - const [workflow, setWorkflow] = useState(); + const {object: workflow, setObject: setWorkflow, serialization, lang, setLang} = useEditableObject(); const [error, setError] = useState(); useEffect(() => { @@ -116,7 +117,7 @@ export function WorkflowCreator({namespace, onCreate}: {namespace: string; onCre - +
.
diff --git a/ui/src/workflows/components/workflow-details/workflow-resource-panel.tsx b/ui/src/workflows/components/workflow-details/workflow-resource-panel.tsx index 49193471bbe8..c7bddd44d6a1 100644 --- a/ui/src/workflows/components/workflow-details/workflow-resource-panel.tsx +++ b/ui/src/workflows/components/workflow-details/workflow-resource-panel.tsx @@ -1,10 +1,10 @@ import * as React from 'react'; -import {ObjectEditor} from '../../../shared/components/object-editor'; +import {SerializingObjectEditor} from '../../../shared/components/object-editor'; import {Workflow} from '../../../shared/models'; export const WorkflowResourcePanel = (props: {workflow: Workflow}) => (
- +
); diff --git a/ui/src/workflows/components/workflow-editor.tsx b/ui/src/workflows/components/workflow-editor.tsx index 45cab53bb3cd..9f0524335f35 100644 --- a/ui/src/workflows/components/workflow-editor.tsx +++ b/ui/src/workflows/components/workflow-editor.tsx @@ -4,6 +4,7 @@ import * as React from 'react'; import {MetadataEditor} from '../../shared/components/editors/metadata-editor'; import {WorkflowParametersEditor} from '../../shared/components/editors/workflow-parameters-editor'; import {ObjectEditor} from '../../shared/components/object-editor'; +import type {Lang} from '../../shared/components/object-parser'; import {Workflow} from '../../shared/models'; export function WorkflowEditor({ @@ -11,12 +12,18 @@ export function WorkflowEditor({ onTabSelected, onError, onChange, - template + onLangChange, + workflow, + serialization, + lang }: { - template: Workflow; - onChange: (template: Workflow) => void; + workflow: Workflow; + serialization: string; + lang: Lang; + onChange: (workflow: string | Workflow) => void; onError: (error: Error) => void; onTabSelected?: (tab: string) => void; + onLangChange: (lang: Lang) => void; selectedTabKey?: string; }) { return ( @@ -29,17 +36,26 @@ export function WorkflowEditor({ { key: 'manifest', title: 'Manifest', - content: onChange({...x})} /> + content: ( + + ) }, { key: 'parameters', title: 'Parameters', - content: onChange({...template, spec})} onError={onError} /> + content: onChange({...workflow, spec})} onError={onError} /> }, { key: 'metadata', title: 'MetaData', - content: onChange({...template, metadata})} /> + content: onChange({...workflow, metadata})} /> } ]} /> diff --git a/ui/src/workflows/components/workflow-yaml-viewer/workflow-yaml-viewer.tsx b/ui/src/workflows/components/workflow-yaml-viewer/workflow-yaml-viewer.tsx index dd4f0cdc5400..be52b8cb8223 100644 --- a/ui/src/workflows/components/workflow-yaml-viewer/workflow-yaml-viewer.tsx +++ b/ui/src/workflows/components/workflow-yaml-viewer/workflow-yaml-viewer.tsx @@ -1,7 +1,7 @@ import {SlideContents} from 'argo-ui/src/components/slide-contents/slide-contents'; import * as React from 'react'; -import {ObjectEditor} from '../../../shared/components/object-editor'; +import {SerializingObjectEditor} from '../../../shared/components/object-editor'; import * as models from '../../../shared/models'; import {getResolvedTemplates} from '../../../shared/template-resolution'; @@ -25,7 +25,7 @@ export function WorkflowYamlViewer(props: WorkflowYamlViewerProps) { contents.push(

{normalizeNodeName(props.selectedNode.displayName || props.selectedNode.name)}

- +
); } @@ -35,7 +35,7 @@ export function WorkflowYamlViewer(props: WorkflowYamlViewerProps) { contents.push(

{props.selectedNode.name}

- +
); } @@ -47,7 +47,7 @@ export function WorkflowYamlViewer(props: WorkflowYamlViewerProps) { } + contents={} className='workflow-yaml-section' /> ); @@ -59,7 +59,7 @@ export function WorkflowYamlViewer(props: WorkflowYamlViewerProps) { } + contents={} className='workflow-yaml-section' /> );