From c6cb248cf0a23bbaadf53a79dee22133e1ed4445 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Sat, 13 Apr 2024 17:57:45 -0400 Subject: [PATCH] fix(ui): use router navigation instead of page load after submit - `document.location.href` causes the browser to load a new page which is a full page load - `navigation.goto` should always be used when we're routing within the single-page app (SPA) - this only changes the internal route so only the next component needs to render, not the entire page - (and user-facing `history`, same as changing the `location`) - fix `name` and `namespace` in Workflow Details to actually change when the URL changes - they were previously set as `state` despite not actually being `state`, meaning they only ever received the initial URL and no further changes - in particular, this is required for a resubmit to work, as it re-routes to the same component, but with a different URL - some other tiny code style optimizations in surrounding code Signed-off-by: Anton Gilgur --- .../components/resubmit-workflow-panel.tsx | 6 ++-- .../components/retry-workflow-panel.tsx | 6 ++-- .../components/submit-workflow-panel.tsx | 6 ++-- .../workflow-details/workflow-details.tsx | 29 ++++++------------- 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/ui/src/app/workflows/components/resubmit-workflow-panel.tsx b/ui/src/app/workflows/components/resubmit-workflow-panel.tsx index 423b618d47f2..67842bde9470 100644 --- a/ui/src/app/workflows/components/resubmit-workflow-panel.tsx +++ b/ui/src/app/workflows/components/resubmit-workflow-panel.tsx @@ -1,6 +1,7 @@ import {Checkbox} from 'argo-ui'; -import React, {useState} from 'react'; +import React, {useContext, useState} from 'react'; import {Parameter, ResubmitOpts, Workflow} from '../../../models'; +import {Context} from '../../shared/context'; import {uiUrl} from '../../shared/base'; import {ErrorNotice} from '../../shared/components/error-notice'; import {ParametersInput} from '../../shared/components/parameters-input'; @@ -13,6 +14,7 @@ interface Props { } export function ResubmitWorkflowPanel(props: Props) { + const {navigation} = useContext(Context); const [overrideParameters, setOverrideParameters] = useState(false); const [workflowParameters, setWorkflowParameters] = useState(JSON.parse(JSON.stringify(props.workflow.spec.arguments.parameters || []))); const [memoized, setMemoized] = useState(false); @@ -33,7 +35,7 @@ export function ResubmitWorkflowPanel(props: Props) { const submitted = props.isArchived ? await services.workflows.resubmitArchived(props.workflow.metadata.uid, props.workflow.metadata.namespace, opts) : await services.workflows.resubmit(props.workflow.metadata.name, props.workflow.metadata.namespace, opts); - document.location.href = uiUrl(`workflows/${submitted.metadata.namespace}/${submitted.metadata.name}`); + navigation.goto(uiUrl(`workflows/${submitted.metadata.namespace}/${submitted.metadata.name}`)); } catch (err) { setError(err); setIsSubmitting(false); diff --git a/ui/src/app/workflows/components/retry-workflow-panel.tsx b/ui/src/app/workflows/components/retry-workflow-panel.tsx index 1a7508d4f73c..51ea2c518a1b 100644 --- a/ui/src/app/workflows/components/retry-workflow-panel.tsx +++ b/ui/src/app/workflows/components/retry-workflow-panel.tsx @@ -1,6 +1,7 @@ import {Checkbox} from 'argo-ui'; -import React, {useState} from 'react'; +import React, {useContext, useState} from 'react'; import {Parameter, RetryOpts, Workflow} from '../../../models'; +import {Context} from '../../shared/context'; import {uiUrl} from '../../shared/base'; import {ErrorNotice} from '../../shared/components/error-notice'; import {ParametersInput} from '../../shared/components/parameters-input'; @@ -14,6 +15,7 @@ interface Props { } export function RetryWorkflowPanel(props: Props) { + const {navigation} = useContext(Context); const [overrideParameters, setOverrideParameters] = useState(false); const [restartSuccessful, setRestartSuccessful] = useState(false); const [workflowParameters, setWorkflowParameters] = useState(JSON.parse(JSON.stringify(props.workflow.spec.arguments.parameters || []))); @@ -37,7 +39,7 @@ export function RetryWorkflowPanel(props: Props) { props.isArchived && !props.isWorkflowInCluster ? await services.workflows.retryArchived(props.workflow.metadata.uid, props.workflow.metadata.namespace, opts) : await services.workflows.retry(props.workflow.metadata.name, props.workflow.metadata.namespace, opts); - document.location.href = uiUrl(`workflows/${submitted.metadata.namespace}/${submitted.metadata.name}`); + navigation.goto(uiUrl(`workflows/${submitted.metadata.namespace}/${submitted.metadata.name}#`)); // add # at the end to reset query params to close panel } catch (err) { setError(err); setIsSubmitting(false); diff --git a/ui/src/app/workflows/components/submit-workflow-panel.tsx b/ui/src/app/workflows/components/submit-workflow-panel.tsx index 290179dc4807..c2605d99993b 100644 --- a/ui/src/app/workflows/components/submit-workflow-panel.tsx +++ b/ui/src/app/workflows/components/submit-workflow-panel.tsx @@ -1,6 +1,7 @@ import {Select} from 'argo-ui'; -import React, {useMemo, useState} from 'react'; +import React, {useContext, useMemo, useState} from 'react'; import {Parameter, Template} from '../../../models'; +import {Context} from '../../shared/context'; import {uiUrl} from '../../shared/base'; import {ErrorNotice} from '../../shared/components/error-notice'; import {ParametersInput} from '../../shared/components/parameters-input'; @@ -26,6 +27,7 @@ const defaultTemplate: Template = { }; export function SubmitWorkflowPanel(props: Props) { + const {navigation} = useContext(Context); const [entrypoint, setEntrypoint] = useState(workflowEntrypoint); const [parameters, setParameters] = useState([]); const [workflowParameters, setWorkflowParameters] = useState(JSON.parse(JSON.stringify(props.workflowParameters))); @@ -55,7 +57,7 @@ export function SubmitWorkflowPanel(props: Props) { ], labels: labels.join(',') }); - document.location.href = uiUrl(`workflows/${submitted.metadata.namespace}/${submitted.metadata.name}`); + navigation.goto(uiUrl(`workflows/${submitted.metadata.namespace}/${submitted.metadata.name}`)); } catch (err) { setError(err); setIsSubmitting(false); diff --git a/ui/src/app/workflows/components/workflow-details/workflow-details.tsx b/ui/src/app/workflows/components/workflow-details/workflow-details.tsx index 60fe906ecc54..5d919a5ed646 100644 --- a/ui/src/app/workflows/components/workflow-details/workflow-details.tsx +++ b/ui/src/app/workflows/components/workflow-details/workflow-details.tsx @@ -94,9 +94,9 @@ export function WorkflowDetails({history, location, match}: RouteComponentProps< // boiler-plate const {navigation, popup} = useContext(Context); const queryParams = new URLSearchParams(location.search); + const namespace = match.params.namespace; + const name = match.params.name; - const [namespace] = useState(match.params.namespace); - const [name, setName] = useState(match.params.name); const [tab, setTab] = useState(queryParams.get('tab') || 'workflow'); const [uid, setUid] = useState(queryParams.get('uid') || ''); const [nodeId, setNodeId] = useState(queryParams.get('nodeId')); @@ -107,8 +107,8 @@ export function WorkflowDetails({history, location, match}: RouteComponentProps< const [workflow, setWorkflow] = useState(); const [links, setLinks] = useState(); const [error, setError] = useState(); - const selectedNode = workflow && workflow.status && workflow.status.nodes && workflow.status.nodes[nodeId]; - const selectedArtifact = workflow && workflow.status && findArtifact(workflow.status, nodeId); + const selectedNode = workflow?.status?.nodes?.[nodeId]; + const selectedArtifact = workflow?.status && findArtifact(workflow.status, nodeId); const [selectedTemplateArtifactRepo, setSelectedTemplateArtifactRepo] = useState(); const isSidePanelExpanded = !!(selectedNode || selectedArtifact); const isSidePanelAnimating = useTransition(isSidePanelExpanded, ANIMATION_MS + ANIMATION_BUFFER_MS); @@ -197,9 +197,7 @@ export function WorkflowDetails({history, location, match}: RouteComponentProps< popup .confirm('Confirm', () => ) .then(async yes => { - if (!yes) { - return; - } + if (!yes) return; const allPromises = []; if (isWorkflowInCluster(workflow)) { @@ -224,16 +222,9 @@ export function WorkflowDetails({history, location, match}: RouteComponentProps< setSidePanel('retry'); } else { popup.confirm('Confirm', `Are you sure you want to ${workflowOperation.title.toLowerCase()} this workflow?`).then(yes => { - if (!yes) { - return; - } - - workflowOperation - .action(workflow) - .then((wf: Workflow) => { - setName(wf.metadata.name); - }) - .catch(setError); + if (!yes) return; + + workflowOperation.action(workflow).catch(setError); }); } } @@ -481,9 +472,7 @@ export function WorkflowDetails({history, location, match}: RouteComponentProps< function renderResumePopup() { return popup.confirm('Confirm', renderSuspendNodeOptions).then(yes => { - if (!yes) { - return; - } + if (!yes) return; updateOutputParametersForNodeIfRequired().then(resumeNode).catch(setError); });