From 89fadd59db4aceb42f604e0873429559802038e7 Mon Sep 17 00:00:00 2001 From: Peter Pan Date: Sun, 23 May 2021 14:05:47 +0800 Subject: [PATCH] fix: confusion of column order in hyper-parameters page (#966) --- .../ParallelCoordinatesGraph/Component.tsx | 31 +++++++++++--- .../ParallelCoordinatesView.tsx | 8 +++- .../HyperParameterPage/TableView/Table.tsx | 7 +--- .../TableView/TableView.tsx | 42 ++++++------------- .../hooks/useIndicatorOrder.ts | 35 ++++++++++++++++ .../src/resource/hyper-parameter/index.ts | 1 + 6 files changed, 83 insertions(+), 41 deletions(-) create mode 100644 frontend/packages/core/src/resource/hyper-parameter/hooks/useIndicatorOrder.ts diff --git a/frontend/packages/core/src/components/HyperParameterPage/ParallelCoordinatesView/ParallelCoordinatesGraph/Component.tsx b/frontend/packages/core/src/components/HyperParameterPage/ParallelCoordinatesView/ParallelCoordinatesGraph/Component.tsx index 9dd97548..3693c593 100644 --- a/frontend/packages/core/src/components/HyperParameterPage/ParallelCoordinatesView/ParallelCoordinatesGraph/Component.tsx +++ b/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 = ({ indicators, data, colors, + order, onHover, onSelect, + onChangeOrder, className }) => { const container = useRef(null); const graph = useRef(); 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 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 { - 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 ( diff --git a/frontend/packages/core/src/components/HyperParameterPage/ParallelCoordinatesView/ParallelCoordinatesView.tsx b/frontend/packages/core/src/components/HyperParameterPage/ParallelCoordinatesView/ParallelCoordinatesView.tsx index 5bffe596..709b18fb 100644 --- a/frontend/packages/core/src/components/HyperParameterPage/ParallelCoordinatesView/ParallelCoordinatesView.tsx +++ b/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 = ({indicators, list, data}) => { @@ -48,6 +50,8 @@ const ParallelCoordinatesView: FunctionComponent = const [colors, setColors] = useState([]); + const [order, setOrder] = useIndicatorOrder(COLUMN_ORDER_STORAGE_KEY, indicators); + return ( <> @@ -58,8 +62,10 @@ const ParallelCoordinatesView: FunctionComponent = list={list} data={data} colors={colors} + order={order} onHover={onHover} onSelect={onSelect} + onChangeOrder={setOrder} /> diff --git a/frontend/packages/core/src/components/HyperParameterPage/TableView/Table.tsx b/frontend/packages/core/src/components/HyperParameterPage/TableView/Table.tsx index 7254de13..c845968f 100644 --- a/frontend/packages/core/src/components/HyperParameterPage/TableView/Table.tsx +++ b/frontend/packages/core/src/components/HyperParameterPage/TableView/Table.tsx @@ -95,7 +95,7 @@ const TableViewTable: FunctionComponent = ({ setColumnOrder, state, totalColumnsWidth, - columns: tableColumns, + visibleColumns, allColumns } = useTable( { @@ -131,10 +131,7 @@ const TableViewTable: FunctionComponent = ({ 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]); diff --git a/frontend/packages/core/src/components/HyperParameterPage/TableView/TableView.tsx b/frontend/packages/core/src/components/HyperParameterPage/TableView/TableView.tsx index 1170340a..e87ef70d 100644 --- a/frontend/packages/core/src/components/HyperParameterPage/TableView/TableView.tsx +++ b/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 = ({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(DEFAULT_ORDER_INDICATOR); + const [columnOrder, setColumnOrder] = useState(DEFAULT_ORDER_INDICATOR); const orderDirectionList = useMemo( () => @@ -88,35 +87,20 @@ const TableView: FunctionComponent = ({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( - 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 ( {t('hyper-parameter:order-by')} - + {columnOrder !== DEFAULT_ORDER_INDICATOR ? ( <> {t('hyper-parameter:order-direction')}