Refactor setting of experiment table column order via drag and drop (#4479)

Author: mattseddon

webview/src/experiments/components/Experiments.tsx

@@ -145,10 +145,6 @@ export const ExperimentsTable: React.FC = () => {
     setColumns(getColumns(columnsData))
   }, [columnsData])
 
-  useEffect(() => {
-    setColumnOrder(columnOrderData)
-  }, [columnOrderData])
-
   const getRowId = useCallback(
     (experiment: Commit, relativeIndex: number, parent?: TableRow<Commit>) =>
       parent ? [parent.id, experiment.id].join('.') : String(relativeIndex),
@@ -166,6 +162,7 @@ export const ExperimentsTable: React.FC = () => {
     getExpandedRowModel: getExpandedRowModel(),
     getRowId,
     getSubRows: row => row.subRows,
+    onColumnOrderChange: setColumnOrder,
     onColumnSizingChange: setColumnSizing,
     onExpandedChange: setExpanded,
     state: {
@@ -188,7 +185,7 @@ export const ExperimentsTable: React.FC = () => {
   }
   return (
     <RowSelectionProvider>
-      <Table instance={instance} onColumnOrderChange={setColumnOrder} />
+      <Table instance={instance} />
       {!hasConfig && <AddStage />}
     </RowSelectionProvider>
   )

webview/src/experiments/components/table/Table.tsx

@@ -1,20 +1,12 @@
 import React, { useRef, useState, CSSProperties, useContext } from 'react'
-import { ColumnOrderState } from '@tanstack/react-table'
 import cx from 'classnames'
 import styles from './styles.module.scss'
 import { TableHead } from './header/TableHead'
 import { RowSelectionContext } from './RowSelectionContext'
 import { TableContent } from './body/TableContent'
 import { InstanceProp } from '../../util/interfaces'
 
-interface TableProps extends InstanceProp {
-  onColumnOrderChange: (order: ColumnOrderState) => void
-}
-
-export const Table: React.FC<TableProps> = ({
-  instance,
-  onColumnOrderChange
-}) => {
+export const Table: React.FC<InstanceProp> = ({ instance }) => {
   const { clearSelectedRows } = useContext(RowSelectionContext)
   const [expColumnNeedsShadow, setExpColumnNeedsShadow] = useState(false)
   const [tableHeadHeight, setTableHeadHeight] = useState(55)
@@ -44,7 +36,6 @@ export const Table: React.FC<TableProps> = ({
           root={tableRef.current}
           setExpColumnNeedsShadow={setExpColumnNeedsShadow}
           setTableHeadHeight={setTableHeadHeight}
-          onOrderChange={onColumnOrderChange}
         />
         <TableContent
           instance={instance}

webview/src/experiments/components/table/header/TableHead.tsx

@@ -1,7 +1,7 @@
 import { Experiment } from 'dvc/src/experiments/webview/contract'
 import React, { DragEvent, useRef, useEffect } from 'react'
 import { useSelector, useDispatch } from 'react-redux'
-import { Table, Header, ColumnOrderState } from '@tanstack/react-table'
+import { Table, Header } from '@tanstack/react-table'
 import { MergedHeaderGroups } from './MergeHeaderGroups'
 import { setDropTarget } from '../../../state/headerDropTargetSlice'
 import { ExperimentsState } from '../../../store'
@@ -17,15 +17,13 @@ import { reorderColumns } from '../../../util/messages'
 interface TableHeadProps {
   instance: Table<Experiment>
   root: HTMLElement | null
-  onOrderChange: (order: ColumnOrderState) => void
   setExpColumnNeedsShadow: (needsShadow: boolean) => void
   setTableHeadHeight: (height: number) => void
 }
 
 export const TableHead = ({
   instance,
   root,
-  onOrderChange,
   setExpColumnNeedsShadow,
   setTableHeadHeight
 }: TableHeadProps) => {
@@ -110,7 +108,6 @@ export const TableHead = ({
       setColumnOrder(newOrder)
       reorderColumns(newOrder)
       onDragEnd()
-      onOrderChange(newOrder)
     }
   }
 

webview/src/experiments/util/columns.ts

@@ -3,44 +3,78 @@ import { Header } from '@tanstack/react-table'
 
 export const isFirstLevelHeader = (id: string) => id.split(':').length - 1 === 1
 
-export const reorderColumnIds = (
-  columnIds: string[],
-  displacer: string[],
+type OrderAccumulator = { newOrder: string[]; firstDroppedFound: boolean }
+
+const addDroppedAndDisplaced = (
+  acc: OrderAccumulator,
+  dropped: string[],
   displaced: string[]
 ) => {
-  if (columnIds.length === 0 || displacer[0] === displaced[0]) {
-    return columnIds
+  const droppedStartedLeftOfDisplaced = acc.firstDroppedFound
+  const order = droppedStartedLeftOfDisplaced
+    ? [...displaced, ...dropped]
+    : [...dropped, ...displaced]
+  acc.newOrder.push(...order)
+}
+
+const collectId = (
+  acc: OrderAccumulator,
+  columnId: string,
+  dropped: string[],
+  displaced: string[]
+) => {
+  if (columnId === displaced[0]) {
+    addDroppedAndDisplaced(acc, dropped, displaced)
+    return
+  }
+  if (displaced.includes(columnId)) {
+    return
   }
+  if (dropped.includes(columnId)) {
+    acc.firstDroppedFound = true
+    return
+  }
+
+  acc.newOrder.push(columnId)
+}
 
-  const displacerIndex = columnIds.indexOf(displacer[0])
-  const displacedIndex = columnIds.indexOf(displaced[0])
-
-  if (displacerIndex < displacedIndex) {
-    return [
-      ...columnIds.slice(0, displacerIndex),
-      ...columnIds.slice(displacerIndex + displacer.length, displacedIndex),
-      ...displaced,
-      ...displacer,
-      ...columnIds.slice(displacedIndex + displaced.length)
-    ]
+const collectNewOrder = (
+  columnIds: string[],
+  dropped: string[],
+  displaced: string[]
+): string[] => {
+  const acc: OrderAccumulator = {
+    firstDroppedFound: false,
+    newOrder: []
   }
+  for (const columnId of columnIds) {
+    collectId(acc, columnId, dropped, displaced)
+  }
+  return acc.newOrder
+}
 
-  return [
-    ...columnIds.slice(0, displacedIndex),
-    ...displacer,
-    ...displaced,
-    ...columnIds.slice(displacedIndex + displaced.length, displacerIndex),
-    ...columnIds.slice(displacerIndex + displacer.length)
-  ]
+export const reorderColumnIds = (
+  columnIds: string[],
+  dropped: string[],
+  displaced: string[]
+) => {
+  if (columnIds.length === 0 || dropped[0] === displaced[0]) {
+    return columnIds
+  }
+  return collectNewOrder(columnIds, dropped, displaced)
 }
 
 export const leafColumnIds = (
   header: Header<Experiment, unknown>
 ): string[] => {
-  return header
-    .getLeafHeaders()
-    .filter(h => h.subHeaders.length === 0)
-    .map(h => h.column.id)
+  const leafColumnIds: string[] = []
+  for (const headers of header.getLeafHeaders()) {
+    if (headers.subHeaders.length > 0) {
+      continue
+    }
+    leafColumnIds.push(headers.column.id)
+  }
+  return leafColumnIds
 }
 
 export const EXPERIMENT_COLUMN_ID = 'id'