From 3263cfa4a4a5552d39b99c13258497242884de62 Mon Sep 17 00:00:00 2001 From: tetora1053 Date: Wed, 27 Sep 2023 22:03:59 +0900 Subject: [PATCH 1/5] refactor(ui): InputFilter component from class to functional Signed-off-by: tetora1053 --- ui/src/app/shared/components/input-filter.tsx | 108 ++++++++---------- 1 file changed, 50 insertions(+), 58 deletions(-) diff --git a/ui/src/app/shared/components/input-filter.tsx b/ui/src/app/shared/components/input-filter.tsx index b723915f2133..95f57f09ce75 100644 --- a/ui/src/app/shared/components/input-filter.tsx +++ b/ui/src/app/shared/components/input-filter.tsx @@ -1,5 +1,5 @@ import {Autocomplete} from 'argo-ui'; -import * as React from 'react'; +import React, {useState} from 'react'; interface InputProps { value: string; @@ -8,68 +8,60 @@ interface InputProps { onChange: (input: string) => void; } -interface InputState { - value: string; - localCache: string[]; - error?: Error; -} +export function InputFilter(props: InputProps) { + const [value, setValue] = useState(props.value); + const [localCache, setLocalCache] = useState((localStorage.getItem(props.name + '_inputs') || '').split(',').filter(item => item !== '')); -export class InputFilter extends React.Component { - constructor(props: Readonly) { - super(props); - this.state = { - value: props.value, - localCache: (localStorage.getItem(this.props.name + '_inputs') || '').split(',').filter(value => value !== '') - }; + function setValueAndCache(newValue: string) { + setLocalCache(state => { + const updatedCache = [...state]; + if (!updatedCache.includes(newValue)) { + updatedCache.unshift(newValue); + } + while (updatedCache.length > 5) { + updatedCache.pop(); + } + localStorage.setItem(props.name + '_inputs', updatedCache.join(',')); + return updatedCache; + }); } - public render() { + function renderInput(inputProps: React.HTMLProps) { return ( - <> - this.setState({value})} - onSelect={value => { - this.setState({value}); - this.props.onChange(value); - }} - renderInput={inputProps => ( - { - if (event.keyCode === 13) { - this.setValueAndCache(event.currentTarget.value); - this.props.onChange(this.state.value); - } - }} - className='argo-field' - placeholder={this.props.placeholder} - /> - )} - /> - { - this.setState({value: ''}); - this.props.onChange(''); - }}> - - - + { + if (event.keyCode === 13) { + setValue(event.currentTarget.value); + setValueAndCache(event.currentTarget.value); + props.onChange(value); + } + }} + className='argo-field' + placeholder={props.placeholder} + /> ); } - private setValueAndCache(value: string) { - this.setState(state => { - const localCache = state.localCache; - if (!state.localCache.includes(value)) { - localCache.unshift(value); - } - while (localCache.length > 5) { - localCache.pop(); - } - localStorage.setItem(this.props.name + '_inputs', localCache.join(',')); - return {value, localCache}; - }); - } + return ( + <> + setValue(newValue)} + onSelect={newValue => { + setValue(newValue); + props.onChange(newValue); + }} + renderInput={renderInput} + /> + { + setValue(''); + props.onChange(''); + }}> + + + + ); } From 7adb9e96d28dec43115180bccfade21fa2cb9d43 Mon Sep 17 00:00:00 2001 From: tetora1053 Date: Fri, 29 Sep 2023 00:14:19 +0900 Subject: [PATCH 2/5] refactor(ui): WorkflowTimeline component from class to functional Signed-off-by: tetora1053 --- .../workflow-timeline/workflow-timeline.tsx | 212 +++++++++--------- 1 file changed, 104 insertions(+), 108 deletions(-) diff --git a/ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx b/ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx index 744ea202554c..75dd23a8d6a0 100644 --- a/ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx +++ b/ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx @@ -1,6 +1,6 @@ import classNames from 'classnames'; import moment from 'moment'; -import * as React from 'react'; +import React, {useEffect, useRef, useState} from 'react'; import {fromEvent, interval, Subscription} from 'rxjs'; import * as models from '../../../../models'; @@ -18,128 +18,124 @@ interface WorkflowTimelineProps { nodeClicked?: (node: models.NodeStatus) => any; } -interface WorkflowTimelineState { - parentWidth: number; - now: moment.Moment; -} +export function WorkflowTimeline(props: WorkflowTimelineProps) { + let resizeSubscription: Subscription; + let refreshSubscription: Subscription; -export class WorkflowTimeline extends React.Component { - private container: HTMLElement; - private resizeSubscription: Subscription; - private refreshSubscription: Subscription; + const [parentWidth, setParentWidth] = useState(0); + const [now, setNow] = useState(moment()); - constructor(props: WorkflowTimelineProps) { - super(props); - this.state = {parentWidth: 0, now: moment()}; - this.ensureRunningWorkflowRefreshing(props.workflow); - } + const containerRef = useRef(null); - public componentDidMount() { - this.resizeSubscription = fromEvent(window, 'resize').subscribe(() => this.updateWidth()); - this.updateWidth(); - } + useEffect(() => { + resizeSubscription = fromEvent(window, 'resize').subscribe(updateWidth); + updateWidth(); - public componentWillReceiveProps(nextProps: WorkflowTimelineProps) { - this.ensureRunningWorkflowRefreshing(nextProps.workflow); - } + return () => { + if (resizeSubscription) { + resizeSubscription.unsubscribe(); + } + if (refreshSubscription) { + refreshSubscription.unsubscribe(); + } + }; + }, []); - public componentWillUnmount() { - if (this.resizeSubscription) { - this.resizeSubscription.unsubscribe(); - this.resizeSubscription = null; - } - if (this.refreshSubscription) { - this.refreshSubscription.unsubscribe(); - this.refreshSubscription = null; + useEffect(() => { + ensureRunningWorkflowRefreshing(props.workflow); + }, [props.workflow]); + + function updateWidth() { + if (containerRef.current) { + setParentWidth((containerRef.current.offsetParent || window.document.body).clientWidth - NODE_NAME_WIDTH); } } - public render() { - if (!this.props.workflow.status.nodes) { - return

No nodes

; - } - const nodes = Object.keys(this.props.workflow.status.nodes) - .map(id => { - const node = this.props.workflow.status.nodes[id]; - node.finishedAt = node.finishedAt || this.state.now.format(); - node.startedAt = node.startedAt || this.state.now.format(); - return node; - }) - .filter(node => node.startedAt && node.type === 'Pod') - .sort((first, second) => { - const diff = moment(first.startedAt).diff(second.startedAt); - // If node started almost at the same time then sort by end time - if (diff <= 2) { - return moment(first.finishedAt).diff(second.finishedAt); - } - return diff; + function ensureRunningWorkflowRefreshing(workflow: models.Workflow) { + const completedPhases = [models.NODE_PHASE.ERROR, models.NODE_PHASE.SUCCEEDED, models.NODE_PHASE.SKIPPED, models.NODE_PHASE.OMITTED, models.NODE_PHASE.FAILED]; + const isCompleted = workflow && workflow.status && completedPhases.indexOf(workflow.status.phase) > -1; + if (!refreshSubscription && !isCompleted) { + refreshSubscription = interval(1000).subscribe(() => { + setNow(moment()); }); - if (nodes.length === 0) { - return
; + } else if (refreshSubscription && isCompleted) { + refreshSubscription.unsubscribe(); + refreshSubscription = null; } - const timelineStart = moment(nodes[0].startedAt).valueOf(); - const timelineEnd = nodes.map(node => moment(node.finishedAt).valueOf()).reduce((first, second) => Math.max(first, second), moment(timelineStart).valueOf()); - - const timeToLeft = (time: number) => ((time - timelineStart) / (timelineEnd - timelineStart)) * Math.max(this.state.parentWidth, MIN_WIDTH) + NODE_NAME_WIDTH; - const groups = nodes.map(node => ({ - startedAt: moment(node.startedAt).valueOf(), - finishedAt: moment(node.finishedAt).valueOf(), - nodes: [ - Object.assign({}, node, { - left: timeToLeft(moment(node.startedAt).valueOf()), - width: timeToLeft(moment(node.finishedAt).valueOf()) - timeToLeft(moment(node.startedAt).valueOf()) - }) - ] - })); - for (let i = groups.length - 1; i >= 1; i--) { - const cur = groups[i]; - const next = groups[i - 1]; - if (moment(cur.startedAt).diff(next.finishedAt, 'milliseconds') < 0 && moment(next.startedAt).diff(cur.startedAt, 'milliseconds') < ROUND_START_DIFF_MS) { - next.nodes = next.nodes.concat(cur.nodes); - next.finishedAt = nodes.map(node => moment(node.finishedAt).valueOf()).reduce((first, second) => Math.max(first, second), next.finishedAt.valueOf()); - groups.splice(i, 1); + } + + if (!props.workflow.status.nodes) { + return

No nodes

; + } + + const nodes = Object.keys(props.workflow.status.nodes) + .map(id => { + const node = props.workflow.status.nodes[id]; + node.finishedAt = node.finishedAt || now.format(); + node.startedAt = node.startedAt || now.format(); + return node; + }) + .filter(node => node.startedAt && node.type === 'Pod') + .sort((first, second) => { + const diff = moment(first.startedAt).diff(second.startedAt); + if (diff <= 2) { + return moment(first.finishedAt).diff(second.finishedAt); } - } - return ( -
(this.container = container)} style={{width: Math.max(this.state.parentWidth, MIN_WIDTH) + NODE_NAME_WIDTH}}> -
-
- {groups.map(group => [ -
- {moment(group.startedAt).format('hh:mm')} -
, - ...group.nodes.map(node => ( -
this.props.nodeClicked && this.props.nodeClicked(node)}> -
- {Utils.shortNodeName(node)} -
-
-
- )) - ])} -
- ); + return diff; + }); + + if (nodes.length === 0) { + return
; } - public updateWidth() { - if (this.container) { - this.setState({parentWidth: (this.container.offsetParent || window.document.body).clientWidth - NODE_NAME_WIDTH}); - } + const timelineStart = moment(nodes[0].startedAt).valueOf(); + const timelineEnd = nodes.map(node => moment(node.finishedAt).valueOf()).reduce((first, second) => Math.max(first, second), moment(timelineStart).valueOf()); + + function timeToLeft(time: number) { + return ((time - timelineStart) / (timelineEnd - timelineStart)) * Math.max(parentWidth, MIN_WIDTH) + NODE_NAME_WIDTH; } - private ensureRunningWorkflowRefreshing(workflow: models.Workflow) { - const completedPhases = [models.NODE_PHASE.ERROR, models.NODE_PHASE.SUCCEEDED, models.NODE_PHASE.SKIPPED, models.NODE_PHASE.OMITTED, models.NODE_PHASE.FAILED]; - const isCompleted = workflow && workflow.status && completedPhases.indexOf(workflow.status.phase) > -1; - if (!this.refreshSubscription && !isCompleted) { - this.refreshSubscription = interval(1000).subscribe(() => { - this.setState({now: moment()}); - }); - } else if (this.refreshSubscription && isCompleted) { - this.refreshSubscription.unsubscribe(); - this.refreshSubscription = null; + const groups = nodes.map(node => ({ + startedAt: moment(node.startedAt).valueOf(), + finishedAt: moment(node.finishedAt).valueOf(), + nodes: [ + Object.assign({}, node, { + left: timeToLeft(moment(node.startedAt).valueOf()), + width: timeToLeft(moment(node.finishedAt).valueOf()) - timeToLeft(moment(node.startedAt).valueOf()) + }) + ] + })); + + for (let i = groups.length - 1; i >= 1; i--) { + const cur = groups[i]; + const next = groups[i - 1]; + if (moment(cur.startedAt).diff(next.finishedAt, 'milliseconds') < 0 && moment(next.startedAt).diff(cur.startedAt, 'milliseconds') < ROUND_START_DIFF_MS) { + next.nodes = next.nodes.concat(cur.nodes); + next.finishedAt = nodes.map(node => moment(node.finishedAt).valueOf()).reduce((first, second) => Math.max(first, second), next.finishedAt.valueOf()); + groups.splice(i, 1); } } + + return ( +
+
+
+ {groups.map(group => [ +
+ {moment(group.startedAt).format('hh:mm')} +
, + ...group.nodes.map(node => ( +
props.nodeClicked && props.nodeClicked(node)}> +
+ {Utils.shortNodeName(node)} +
+
+
+ )) + ])} +
+ ); } From dd5b64d8eefe98e1cefbbc95e1d979477c397000 Mon Sep 17 00:00:00 2001 From: tetora1053 Date: Fri, 29 Sep 2023 01:21:37 +0900 Subject: [PATCH 3/5] fix: remove redundant type annotations Signed-off-by: tetora1053 --- ui/src/app/shared/components/input-filter.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/src/app/shared/components/input-filter.tsx b/ui/src/app/shared/components/input-filter.tsx index 95f57f09ce75..9b132111a43f 100644 --- a/ui/src/app/shared/components/input-filter.tsx +++ b/ui/src/app/shared/components/input-filter.tsx @@ -9,8 +9,8 @@ interface InputProps { } export function InputFilter(props: InputProps) { - const [value, setValue] = useState(props.value); - const [localCache, setLocalCache] = useState((localStorage.getItem(props.name + '_inputs') || '').split(',').filter(item => item !== '')); + const [value, setValue] = useState(props.value); + const [localCache, setLocalCache] = useState((localStorage.getItem(props.name + '_inputs') || '').split(',').filter(item => item !== '')); function setValueAndCache(newValue: string) { setLocalCache(state => { @@ -24,6 +24,7 @@ export function InputFilter(props: InputProps) { localStorage.setItem(props.name + '_inputs', updatedCache.join(',')); return updatedCache; }); + setValue(newValue); } function renderInput(inputProps: React.HTMLProps) { @@ -32,7 +33,6 @@ export function InputFilter(props: InputProps) { {...inputProps} onKeyUp={event => { if (event.keyCode === 13) { - setValue(event.currentTarget.value); setValueAndCache(event.currentTarget.value); props.onChange(value); } From 9b7b7c7f2b1be879b0d06ab2087f0136688ae720 Mon Sep 17 00:00:00 2001 From: tetora1053 Date: Fri, 29 Sep 2023 02:18:11 +0900 Subject: [PATCH 4/5] refactor: variable name Signed-off-by: tetora1053 --- ui/src/app/shared/components/input-filter.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/src/app/shared/components/input-filter.tsx b/ui/src/app/shared/components/input-filter.tsx index 9b132111a43f..e870a7f97efb 100644 --- a/ui/src/app/shared/components/input-filter.tsx +++ b/ui/src/app/shared/components/input-filter.tsx @@ -13,8 +13,8 @@ export function InputFilter(props: InputProps) { const [localCache, setLocalCache] = useState((localStorage.getItem(props.name + '_inputs') || '').split(',').filter(item => item !== '')); function setValueAndCache(newValue: string) { - setLocalCache(state => { - const updatedCache = [...state]; + setLocalCache(currentCache => { + const updatedCache = [...currentCache]; if (!updatedCache.includes(newValue)) { updatedCache.unshift(newValue); } From f2f6973d7573446c8f2f9af3c784a08e2bfa43df Mon Sep 17 00:00:00 2001 From: tetora1053 Date: Fri, 29 Sep 2023 23:54:08 +0900 Subject: [PATCH 5/5] fix: optimize code Signed-off-by: tetora1053 --- .../workflow-timeline/workflow-timeline.tsx | 41 ++++++++----------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx b/ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx index 75dd23a8d6a0..7edbf9cf50cf 100644 --- a/ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx +++ b/ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx @@ -11,6 +11,7 @@ require('./workflow-timeline.scss'); const ROUND_START_DIFF_MS = 1000; const NODE_NAME_WIDTH = 250; const MIN_WIDTH = 800; +const COMPLETED_PHASES = [models.NODE_PHASE.ERROR, models.NODE_PHASE.SUCCEEDED, models.NODE_PHASE.SKIPPED, models.NODE_PHASE.OMITTED, models.NODE_PHASE.FAILED]; interface WorkflowTimelineProps { workflow: models.Workflow; @@ -19,30 +20,33 @@ interface WorkflowTimelineProps { } export function WorkflowTimeline(props: WorkflowTimelineProps) { - let resizeSubscription: Subscription; - let refreshSubscription: Subscription; - const [parentWidth, setParentWidth] = useState(0); const [now, setNow] = useState(moment()); - const containerRef = useRef(null); + const containerRef = useRef(null); + const resizeSubscription = useRef(null); + const refreshSubscription = useRef(null); useEffect(() => { - resizeSubscription = fromEvent(window, 'resize').subscribe(updateWidth); + resizeSubscription.current = fromEvent(window, 'resize').subscribe(updateWidth); updateWidth(); return () => { - if (resizeSubscription) { - resizeSubscription.unsubscribe(); - } - if (refreshSubscription) { - refreshSubscription.unsubscribe(); - } + resizeSubscription.current?.unsubscribe(); + refreshSubscription.current?.unsubscribe(); }; }, []); useEffect(() => { - ensureRunningWorkflowRefreshing(props.workflow); + const isCompleted = props.workflow?.status && COMPLETED_PHASES.includes(props.workflow.status.phase); + if (!refreshSubscription.current && !isCompleted) { + refreshSubscription.current = interval(1000).subscribe(() => { + setNow(moment()); + }); + } else if (refreshSubscription.current && isCompleted) { + refreshSubscription.current.unsubscribe(); + refreshSubscription.current = null; + } }, [props.workflow]); function updateWidth() { @@ -51,19 +55,6 @@ export function WorkflowTimeline(props: WorkflowTimelineProps) { } } - function ensureRunningWorkflowRefreshing(workflow: models.Workflow) { - const completedPhases = [models.NODE_PHASE.ERROR, models.NODE_PHASE.SUCCEEDED, models.NODE_PHASE.SKIPPED, models.NODE_PHASE.OMITTED, models.NODE_PHASE.FAILED]; - const isCompleted = workflow && workflow.status && completedPhases.indexOf(workflow.status.phase) > -1; - if (!refreshSubscription && !isCompleted) { - refreshSubscription = interval(1000).subscribe(() => { - setNow(moment()); - }); - } else if (refreshSubscription && isCompleted) { - refreshSubscription.unsubscribe(); - refreshSubscription = null; - } - } - if (!props.workflow.status.nodes) { return

No nodes

; }