[WebUI] fix issue#2530: duration and intermediate results pictures do…

…n't update (#2531)
microsoft · Jun 12, 2020 · 54496c2 · 54496c2
1 parent 89fa23c
commit 54496c2
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 70 deletions.
diff --git a/src/webui/src/components/Modals/CustomizedTrial.tsx b/src/webui/src/components/Modals/CustomizedTrial.tsx
@@ -151,14 +151,16 @@ class Customize extends React.Component<CustomizeProps, CustomizeState> {
         }
     }
 
-    componentWillReceiveProps(nextProps: CustomizeProps): void {
-        const { copyTrialId } = nextProps;
-        if (copyTrialId !== undefined && TRIALS.getTrial(copyTrialId) !== undefined) {
-            const originCopyTrialPara = TRIALS.getTrial(copyTrialId).description.parameters;
-            this.setState(() => ({ copyTrialParameter: originCopyTrialPara }));
+    componentDidUpdate(prevProps: CustomizeProps): void {
+        if (this.props.copyTrialId !== prevProps.copyTrialId) {
+            const { copyTrialId } = this.props;
+            if (copyTrialId !== undefined && TRIALS.getTrial(copyTrialId) !== undefined) {
+                const originCopyTrialPara = TRIALS.getTrial(copyTrialId).description.parameters;
+                this.setState(() => ({ copyTrialParameter: originCopyTrialPara }));
+            }
         }
     }
- 
+
     render(): React.ReactNode {
         const { closeCustomizeModal, visible } = this.props;
         const { isShowSubmitSucceed, isShowSubmitFailed, isShowWarning, customID, copyTrialParameter } = this.state;

diff --git a/src/webui/src/components/TrialsDetail.tsx b/src/webui/src/components/TrialsDetail.tsx
@@ -39,7 +39,7 @@ class TrialsDetail extends React.Component<TrialsDetailProps, TrialDetailState>
         super(props);
         this.state = {
             tablePageSize: 20,
-            whichGraph: '1',
+            whichGraph: 'Default metric',
             searchType: 'Id',
             // eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/explicit-function-return-type
             searchFilter: trial => true
@@ -81,8 +81,8 @@ class TrialsDetail extends React.Component<TrialsDetailProps, TrialDetailState>
         }
     }
 
-    handleWhichTabs = (activeKey: string): void => {
-        this.setState({ whichGraph: activeKey });
+    handleWhichTabs = (item: any): void => {
+        this.setState({whichGraph: item.props.headerText});
     }
 
     updateSearchFilterType = (event: React.FormEvent<HTMLDivElement>, item: IDropdownOption | undefined): void => {
@@ -109,19 +109,19 @@ class TrialsDetail extends React.Component<TrialsDetailProps, TrialDetailState>
         return (
             <div>
                 <div className="trial" id="tabsty">
-                    <Pivot defaultSelectedKey={"0"} className="detial-title">
+                    <Pivot defaultSelectedKey={"0"} className="detial-title" onLinkClick={this.handleWhichTabs} selectedKey={whichGraph}>
                         {/* <PivotItem tab={this.titleOfacc} key="1"> doesn't work*/}
-                        <PivotItem headerText="Default metric" itemIcon="HomeGroup" key="1">
+                        <PivotItem headerText="Default metric" itemIcon="HomeGroup" key="Default metric">
                             <Stack className="graph">
                                 <DefaultPoint
                                     trialIds={trialIds}
-                                    visible={whichGraph === '1'}
+                                    visible={whichGraph === 'Default metric'}
                                     trialsUpdateBroadcast={this.props.trialsUpdateBroadcast}
                                 />
                             </Stack>
                         </PivotItem>
                         {/* <PivotItem tab={this.titleOfhyper} key="2"> */}
-                        <PivotItem headerText="Hyper-parameter" itemIcon="Equalizer" key="2">
+                        <PivotItem headerText="Hyper-parameter" itemIcon="Equalizer" key="Hyper-parameter">
                             <Stack className="graph">
                                 <Para
                                     dataSource={source}
@@ -131,11 +131,11 @@ class TrialsDetail extends React.Component<TrialsDetailProps, TrialDetailState>
                             </Stack>
                         </PivotItem>
                         {/* <PivotItem tab={this.titleOfDuration} key="3"> */}
-                        <PivotItem headerText="Duration" itemIcon="BarChartHorizontal" key="3">
+                        <PivotItem headerText="Duration" itemIcon="BarChartHorizontal" key="Duration">
                             <Duration source={source} whichGraph={whichGraph} />
                         </PivotItem>
                         {/* <PivotItem tab={this.titleOfIntermediate} key="4"> */}
-                        <PivotItem headerText="Intermediate result" itemIcon="StackedLineChart" key="4">
+                        <PivotItem headerText="Intermediate result" itemIcon="StackedLineChart" key="Intermediate result">
                             {/* *why this graph has small footprint? */}
                             <Intermediate source={source} whichGraph={whichGraph} />
                         </PivotItem>

diff --git a/src/webui/src/components/overview/SuccessTable.tsx b/src/webui/src/components/overview/SuccessTable.tsx
@@ -126,9 +126,11 @@ class SuccessTable extends React.Component<SuccessTableProps, SuccessTableState>
         }
     ];
 
-    componentWillReceiveProps(nextProps: SuccessTableProps): void {
-        const { trialIds } = nextProps;
-        this.setState(() => ({ source: TRIALS.table(trialIds) }));
+    componentDidUpdate(prevProps: SuccessTableProps): void {
+        if(this.props.trialIds !== prevProps.trialIds){
+            const { trialIds } = this.props;
+            this.setState(() => ({ source: TRIALS.table(trialIds) }));
+        }
     }
 
     render(): React.ReactNode {

diff --git a/src/webui/src/components/trial-detail/Duration.tsx b/src/webui/src/components/trial-detail/Duration.tsx
@@ -167,36 +167,13 @@ class Duration extends React.Component<DurationProps, DurationState> {
         this.drawDurationGraph(source);
     }
 
-    componentWillReceiveProps(nextProps: DurationProps): void {
-        const { whichGraph, source } = nextProps;
-        if (whichGraph === '3') {
-            this.drawDurationGraph(source);
-        }
-    }
-
-    shouldComponentUpdate(nextProps: DurationProps): boolean {
-
-        const { whichGraph, source } = nextProps;
-        if (whichGraph === '3') {
-            const beforeSource = this.props.source;
-            if (whichGraph !== this.props.whichGraph) {
-                return true;
-            }
-
-            if (source.length !== beforeSource.length) {
-                return true;
-            }
-
-            if (beforeSource[beforeSource.length - 1] !== undefined) {
-                if (source[source.length - 1].duration !== beforeSource[beforeSource.length - 1].duration) {
-                    return true;
-                }
-                if (source[source.length - 1].status !== beforeSource[beforeSource.length - 1].status) {
-                    return true;
-                }
+    componentDidUpdate(prevProps: DurationProps): void {
+        // add this if to prevent endless loop
+        if (this.props.source !== prevProps.source) {
+            if (this.props.whichGraph === 'Duration') {
+                this.drawDurationGraph(this.props.source);
             }
         }
-        return false;
     }
 
     render(): React.ReactNode {

diff --git a/src/webui/src/components/trial-detail/Intermediate.tsx b/src/webui/src/components/trial-detail/Intermediate.tsx
@@ -212,21 +212,23 @@ class Intermediate extends React.Component<IntermediateProps, IntermediateState>
         this.drawIntermediate(source);
     }
 
-    componentWillReceiveProps(nextProps: IntermediateProps, nextState: IntermediateState): void {
-        const { isFilter, filterSource } = nextState;
-        const { whichGraph, source } = nextProps;
+    componentDidUpdate(prevProps: IntermediateProps, prevState: any): void {
+        if (this.props.source !== prevProps.source || this.state.isFilter !== prevState.isFilter) {
+            const { isFilter, filterSource } = this.state;
+            const { whichGraph, source } = this.props;
 
-        if (whichGraph === '4') {
-            if (isFilter === true) {
-                const pointVal = this.pointInput !== null ? this.pointInput.value : '';
-                const minVal = this.minValInput !== null ? this.minValInput.value : '';
-                if (pointVal === '' && minVal === '') {
-                    this.drawIntermediate(source);
+            if (whichGraph === 'Intermediate result') {
+                if (isFilter === true) {
+                    const pointVal = this.pointInput !== null ? this.pointInput.value : '';
+                    const minVal = this.minValInput !== null ? this.minValInput.value : '';
+                    if (pointVal === '' && minVal === '') {
+                        this.drawIntermediate(source);
+                    } else {
+                        this.drawIntermediate(filterSource);
+                    }
                 } else {
-                    this.drawIntermediate(filterSource);
+                    this.drawIntermediate(source);
                 }
-            } else {
-                this.drawIntermediate(source);
             }
         }
     }

diff --git a/src/webui/src/components/trial-detail/Para.tsx b/src/webui/src/components/trial-detail/Para.tsx
@@ -591,10 +591,12 @@ class Para extends React.Component<ParaProps, ParaState> {
         this.reInit();
     }
 
-    componentWillReceiveProps(nextProps: ParaProps): void {
-        const { dataSource, expSearchSpace, whichGraph } = nextProps;
-        if (whichGraph === '2') {
-            this.hyperParaPic(dataSource, expSearchSpace);
+    componentDidUpdate(prevProps: ParaProps): void {
+        if(this.props.dataSource !== prevProps.dataSource) {
+            const { dataSource, expSearchSpace, whichGraph } = this.props;
+            if (whichGraph === 'Hyper-parameter') {
+                this.hyperParaPic(dataSource, expSearchSpace);
+            }
         }
     }
 

diff --git a/src/webui/src/components/trial-detail/TableList.tsx b/src/webui/src/components/trial-detail/TableList.tsx
@@ -569,15 +569,17 @@ class TableList extends React.Component<TableListProps, TableListState> {
         window.addEventListener('resize', this.onWindowResize);
     }
 
-    UNSAFE_componentWillReceiveProps(nextProps: TableListProps): void {
-        const { columnList, tableSource } = nextProps;
-        this.setState({
-            tableSourceForSort: tableSource,
-            tableColumns: this.initTableColumnList(columnList),
-            allColumnList: this.getAllColumnKeys()
-        });
-
+    componentDidUpdate(prevProps: TableListProps): void {
+        if (this.props.columnList !== prevProps.columnList || this.props.tableSource !== prevProps.tableSource) {
+            const { columnList, tableSource } = this.props;
+            this.setState({
+                tableSourceForSort: tableSource,
+                tableColumns: this.initTableColumnList(columnList),
+                allColumnList: this.getAllColumnKeys()
+            });
+        }
     }
+
     render(): React.ReactNode {
         const { intermediateKey, modalIntermediateWidth, modalIntermediateHeight,
             tableColumns, allColumnList, isShowColumn, modalVisible,