fix: confusion of column order in hyper-parameters page (#966)

frontend/packages/core/src/components/HyperParameterPage/ParallelCoordinatesView/ParallelCoordinatesGraph/Component.tsx

@@ -109,28 +109,35 @@ const StyledScaleMethodSelect = styled(ScaleMethodSelect)`
     position: relative;
 `;
 
+const sortIndicators = (indicators: Indicator[], order?: string[]) =>
+    order ? indicators.sort(({name: nameA}, {name: nameB}) => order.indexOf(nameA) - order.indexOf(nameB)) : indicators;
+
 type ParallelCoordinatesProps = ViewData & {
     colors: string[];
+    order?: string[];
     onHover?: (index: number | null) => unknown;
     onSelect?: (index: number | null) => unknown;
+    onChangeOrder?: (order: string[]) => unknown;
 };
 
 const ParallelCoordinates: FunctionComponent<ParallelCoordinatesProps & WithStyled> = ({
     indicators,
     data,
     colors,
+    order,
     onHover,
     onSelect,
+    onChangeOrder,
     className
 }) => {
     const container = useRef<HTMLDivElement>(null);
     const graph = useRef<Graph>();
     const [columnWidth, setColumnWidth] = useState(0);
 
-    const [indicatorsOrder, setIndicatorsOrder] = useState(indicators.map(({name}) => name));
+    const [indicatorsOrder, setIndicatorsOrder] = useState(sortIndicators(indicators, order).map(({name}) => name));
     useEffect(() => {
-        setIndicatorsOrder(indicators.map(({name}) => name));
-    }, [indicators]);
+        setIndicatorsOrder(sortIndicators(indicators, order).map(({name}) => name));
+    }, [indicators, order]);
 
     const orderedIndicators = useMemo(
         () =>
@@ -176,7 +183,18 @@ const ParallelCoordinates: FunctionComponent<ParallelCoordinatesProps & WithStyl
             setIndicatorsOrder(order);
         });
         return () => graph.current?.dispose();
-    }, []);
+    }, [onChangeOrder]);
+
+    const changeOrder = useCallback((order: string[]) => onChangeOrder?.(order), [onChangeOrder]);
+    useEffect(() => {
+        const g = graph.current;
+        if (g) {
+            g.on('dragged', changeOrder);
+            return () => {
+                g.off('dragged', changeOrder);
+            };
+        }
+    }, [changeOrder]);
 
     useEffect(() => {
         const c = container.current;
@@ -215,9 +233,10 @@ const ParallelCoordinates: FunctionComponent<ParallelCoordinatesProps & WithStyl
     }, [colors]);
 
     useEffect(() => {
-        graph.current?.render(indicators, data);
+        const orderedIndicators = sortIndicators(indicators, order);
+        graph.current?.render(orderedIndicators, data);
         setColumnWidth(graph.current?.columnWidth ?? 0);
-    }, [indicators, data]);
+    }, [indicators, data, order]);
 
     return (
         <Container className={className}>

frontend/packages/core/src/components/HyperParameterPage/ParallelCoordinatesView/ParallelCoordinatesView.tsx

@@ -15,6 +15,7 @@
  */
 
 import React, {FunctionComponent, useState} from 'react';
+import {useGraph, useIndicatorOrder} from '~/resource/hyper-parameter';
 
 import ColorMap from '~/components/HyperParameterPage/ColorMap';
 import ParallelCoordinatesGraph from './ParallelCoordinatesGraph';
@@ -23,7 +24,6 @@ import View from '~/components/HyperParameterPage/View';
 import type {ViewData} from '~/resource/hyper-parameter';
 import {rem} from '~/utils/style';
 import styled from 'styled-components';
-import {useGraph} from '~/resource/hyper-parameter';
 
 const ParallelCoordinatesContainer = styled.div`
     width: 100%;
@@ -41,6 +41,8 @@ const ParallelCoordinatesContainer = styled.div`
     }
 `;
 
+const COLUMN_ORDER_STORAGE_KEY = 'hyper-parameter-parallel-coordinates-view-column-order';
+
 type ParallelCoordinatesViewProps = ViewData;
 
 const ParallelCoordinatesView: FunctionComponent<ParallelCoordinatesViewProps> = ({indicators, list, data}) => {
@@ -48,6 +50,8 @@ const ParallelCoordinatesView: FunctionComponent<ParallelCoordinatesViewProps> =
 
     const [colors, setColors] = useState<string[]>([]);
 
+    const [order, setOrder] = useIndicatorOrder(COLUMN_ORDER_STORAGE_KEY, indicators);
+
     return (
         <>
             <View>
@@ -58,8 +62,10 @@ const ParallelCoordinatesView: FunctionComponent<ParallelCoordinatesViewProps> =
                         list={list}
                         data={data}
                         colors={colors}
+                        order={order}
                         onHover={onHover}
                         onSelect={onSelect}
+                        onChangeOrder={setOrder}
                     />
                     <ColorMap className="color-map" indicators={indicators} data={data} onChange={setColors} />
                 </ParallelCoordinatesContainer>

frontend/packages/core/src/components/HyperParameterPage/TableView/Table.tsx

@@ -95,7 +95,7 @@ const TableViewTable: FunctionComponent<TableViewTableProps> = ({
         setColumnOrder,
         state,
         totalColumnsWidth,
-        columns: tableColumns,
+        visibleColumns,
         allColumns
     } = useTable(
         {
@@ -131,10 +131,7 @@ const TableViewTable: FunctionComponent<TableViewTableProps> = ({
     const startDrag = useCallback((id: string) => setDraggingColumnId(id), []);
     const stopDrag = useCallback(() => setDraggingColumnId(null), []);
     const changeDropSide = useCallback((id: string, side: 'before' | 'after') => setDroppableColumn([id, side]), []);
-    const orderedColumnIds = useMemo(
-        () => (state.columnOrder.length ? state.columnOrder : tableColumns.map(c => c.id)),
-        [state.columnOrder, tableColumns]
-    );
+    const orderedColumnIds = useMemo(() => visibleColumns.map(c => c.id), [visibleColumns]);
     useEffect(() => {
         onOrderChange?.(orderedColumnIds.filter(id => id !== 'name'));
     }, [onOrderChange, orderedColumnIds]);

frontend/packages/core/src/components/HyperParameterPage/TableView/TableView.tsx

@@ -14,19 +14,18 @@
  * limitations under the License.
  */
 
-import {DEFAULT_ORDER_INDICATOR, OrderDirection} from '~/resource/hyper-parameter';
-import React, {FunctionComponent, useCallback, useMemo, useState} from 'react';
+import {DEFAULT_ORDER_INDICATOR, OrderDirection, useIndicatorOrder} from '~/resource/hyper-parameter';
+import React, {FunctionComponent, useMemo, useState} from 'react';
 
 import Select from '~/components/Select';
 import Table from './Table';
 import View from '~/components/HyperParameterPage/View';
 import type {ViewData} from '~/resource/hyper-parameter';
 import {rem} from '~/utils/style';
-import {safeSplit} from '~/utils';
 import styled from 'styled-components';
 import {useTranslation} from 'react-i18next';
 
-const TABLE_ORDER_STORAGE_KEY = 'hyper-parameter-table-view-table-order';
+const COLUMN_ORDER_STORAGE_KEY = 'hyper-parameter-table-view-column-order';
 
 const Wrapper = styled(View)`
     display: flex;
@@ -67,14 +66,14 @@ const TableView: FunctionComponent<TableViewProps> = ({indicators, list, data})
 
     const indicatorNameList = useMemo(() => indicators.map(({name}) => name), [indicators]);
 
-    const indicatorOrderList = useMemo(
+    const columnOrderList = useMemo(
         () => [
             {value: DEFAULT_ORDER_INDICATOR, label: t('hyper-parameter.order-default')},
             ...indicatorNameList.map(value => ({value, label: value}))
         ],
         [indicatorNameList, t]
     );
-    const [indicatorOrder, setIndicatorOrder] = useState<string | symbol>(DEFAULT_ORDER_INDICATOR);
+    const [columnOrder, setColumnOrder] = useState<string | symbol>(DEFAULT_ORDER_INDICATOR);
 
     const orderDirectionList = useMemo(
         () =>
@@ -88,35 +87,20 @@ const TableView: FunctionComponent<TableViewProps> = ({indicators, list, data})
 
     const sortBy = useMemo(
         () =>
-            indicatorOrder === DEFAULT_ORDER_INDICATOR
+            columnOrder === DEFAULT_ORDER_INDICATOR
                 ? []
-                : [{id: indicatorOrder as string, desc: orderDirection === OrderDirection.DESCENDING}],
-        [orderDirection, indicatorOrder]
+                : [{id: columnOrder as string, desc: orderDirection === OrderDirection.DESCENDING}],
+        [orderDirection, columnOrder]
     );
 
-    const [columnOrder, setColumnOrder] = useState<string[]>(
-        safeSplit(window.sessionStorage.getItem(TABLE_ORDER_STORAGE_KEY) ?? '', ',')
-    );
-    const changeOrder = useCallback(
-        (order: string[]) => {
-            const filterOrder = order.filter(o => indicatorNameList.includes(o));
-            setColumnOrder(filterOrder);
-            window.sessionStorage.setItem(TABLE_ORDER_STORAGE_KEY, filterOrder.join(','));
-        },
-        [indicatorNameList]
-    );
+    const [indicatorOrder, setIndicatorOrder] = useIndicatorOrder(COLUMN_ORDER_STORAGE_KEY, indicators);
 
     return (
         <Wrapper>
             <OrderSection>
                 <span>{t('hyper-parameter:order-by')}</span>
-                <Select
-                    className="order-select"
-                    list={indicatorOrderList}
-                    value={indicatorOrder}
-                    onChange={setIndicatorOrder}
-                />
-                {indicatorOrder !== DEFAULT_ORDER_INDICATOR ? (
+                <Select className="order-select" list={columnOrderList} value={columnOrder} onChange={setColumnOrder} />
+                {columnOrder !== DEFAULT_ORDER_INDICATOR ? (
                     <>
                         <span>{t('hyper-parameter:order-direction')}</span>
                         <Select
@@ -135,8 +119,8 @@ const TableView: FunctionComponent<TableViewProps> = ({indicators, list, data})
                     data={data}
                     sortBy={sortBy}
                     expand
-                    columnOrder={columnOrder}
-                    onOrderChange={changeOrder}
+                    columnOrder={indicatorOrder}
+                    onOrderChange={setIndicatorOrder}
                 />
             </TableSection>
         </Wrapper>

frontend/packages/core/src/resource/hyper-parameter/hooks/useIndicatorOrder.ts

+/**
+ * Copyright 2020 Baidu Inc. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {useCallback, useMemo, useState} from 'react';
+
+import type {Indicator} from '../types';
+import {safeSplit} from '~/utils';
+
+export default function useIndicatorOrder(key: string, indicators: Indicator[]) {
+    const indicatorNameList = useMemo(() => indicators.map(i => i.name), [indicators]);
+    const [order, setOrder] = useState<string[]>(safeSplit(window.sessionStorage.getItem(key) ?? '', ','));
+    const changeOrder = useCallback(
+        (order: string[]) => {
+            const filterOrder = order.filter(o => indicatorNameList.includes(o));
+            setOrder(filterOrder);
+            window.sessionStorage.setItem(key, filterOrder.join(','));
+        },
+        [indicatorNameList, key]
+    );
+
+    return [order, changeOrder] as const;
+}

frontend/packages/core/src/resource/hyper-parameter/index.ts

@@ -37,6 +37,7 @@ export {format, formatIndicators, getColorScale, COLOR_MAP} from './format';
 export {filter} from './filter';
 export {calculateRelativeTime, chartData} from './metric';
 export {default as useGraph} from './hooks/useGraph';
+export {default as useIndicatorOrder} from './hooks/useIndicatorOrder';
 
 export const DEFAULT_ORDER_INDICATOR = Symbol('DEFAULT_ORDER_INDICATOR');
 export const DND_TYPE = Symbol('DND_TYPE');