improve table header DnD (partially #2698 #2262) (#2876)

iterative · Dec 5, 2022 · 903b77b · 903b77b
1 parent a5058c0
commit 903b77b
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 39 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -35,6 +35,20 @@
           "VSC_DEBUG": "true"
         }
       }
+    },
+    {
+      "type": "node",
+      "request": "launch",
+      "name": "Jest Current File (webview)",
+      "program": "${workspaceFolder}/node_modules/.bin/jest",
+      "args": ["${fileBasenameNoExtension}", "--config", "jest.config.js"],
+      "console": "integratedTerminal",
+      "internalConsoleOptions": "neverOpen",
+      "disableOptimisticBPs": true,
+      "windows": {
+        "program": "${workspaceFolder}/node_modules/jest/bin/jest"
+      },
+      "cwd": "${workspaceFolder}/webview"
     }
   ]
 }
diff --git a/webview/src/experiments/components/table/Table.test.tsx b/webview/src/experiments/components/table/Table.test.tsx
@@ -445,7 +445,7 @@ describe('Table', () => {
         headers.indexOf('threshold')
       )
 
-      const [, startingNode] = screen.getAllByText('summary.json')
+      const [startingNode] = screen.getAllByText('summary.json')
       dragAndDrop(
         startingNode,
         getDraggableHeaderFromText('test'),

diff --git a/webview/src/experiments/components/table/header/TableHead.tsx b/webview/src/experiments/components/table/header/TableHead.tsx
@@ -10,7 +10,11 @@ import { Indicators } from '../Indicators'
 import { useColumnOrder } from '../../../hooks/useColumnOrder'
 import { ExperimentsState } from '../../../store'
 import { sendMessage } from '../../../../shared/vscode'
-import { leafColumnIds, reorderColumnIds } from '../../../util/columns'
+import {
+  leafColumnIds,
+  reorderColumnIds,
+  isExperimentColumn
+} from '../../../util/columns'
 import { DragFunction } from '../../../../shared/components/dragDrop/Draggable'
 import { getSelectedForPlotsCount } from '../../../util/rows'
 interface TableHeadProps {
@@ -80,8 +84,9 @@ export const TableHead = ({
 
   const onDragUpdate = (e: DragEvent<HTMLElement>) => {
     findDisplacedHeader(e.currentTarget.id, displacedHeader => {
-      const displaced = leafColumnIds(displacedHeader)
-      dispatch(setDropTarget(displaced[displaced.length - 1]))
+      if (!isExperimentColumn(displacedHeader.id)) {
+        dispatch(setDropTarget(displacedHeader.id))
+      }
     })
   }
 
@@ -95,9 +100,13 @@ export const TableHead = ({
     const fullOrder = fullColumnOrder.current
     const displacer = draggingIds.current
     let newOrder: string[] = []
+    const displacedHeader = allHeaders.find(
+      header => header.id === headerDropTargetId
+    )
 
-    if (fullOrder && displacer) {
-      newOrder = reorderColumnIds(fullOrder, displacer, [headerDropTargetId])
+    if (fullOrder && displacer && displacedHeader) {
+      const leafs = leafColumnIds(displacedHeader)
+      newOrder = reorderColumnIds(fullOrder, displacer, leafs)
 
       setColumnOrder(newOrder)
       sendMessage({

diff --git a/webview/src/experiments/components/table/header/TableHeaderCell.tsx b/webview/src/experiments/components/table/header/TableHeaderCell.tsx
@@ -38,6 +38,7 @@ const calcResizerHeight = (
 
 const getHeaderPropsArgs = (
   column: HeaderGroup<Experiment>,
+  headerDropTargetId: string,
   sortEnabled: boolean,
   sortOrder: SortOrder
 ) => {
@@ -56,7 +57,8 @@ const getHeaderPropsArgs = (
           sortOrder === SortOrder.ASCENDING && !column.parent?.placeholderOf,
         [styles.sortingHeaderCellDesc]:
           sortOrder === SortOrder.DESCENDING && !column.placeholderOf
-      }
+      },
+      headerDropTargetId === column.id && styles.headerCellDropTarget
     ),
     style: {
       position: undefined
@@ -115,7 +117,7 @@ export const TableHeaderCell: React.FC<{
     return getMenuOptions(column, sorts)
   }, [column, sorts])
 
-  const isDraggable = !column.placeholderOf && column.id !== 'id'
+  const isDraggable = !column.placeholderOf && !isExperimentColumn(column.id)
   const isPlaceholder = !!column.placeholderOf
 
   const canResize = column.canResize && !isPlaceholder
@@ -154,7 +156,7 @@ export const TableHeaderCell: React.FC<{
     >
       <div
         {...column.getHeaderProps(
-          getHeaderPropsArgs(column, isSortable, sortOrder)
+          getHeaderPropsArgs(column, headerDropTargetId, isSortable, sortOrder)
         )}
         data-testid={`header-${column.id}`}
         key={column.id}
@@ -171,12 +173,6 @@ export const TableHeaderCell: React.FC<{
         ) : (
           cellContents
         )}
-        <div
-          className={cx(
-            styles.dropTarget,
-            headerDropTargetId === column.id && styles.active
-          )}
-        />
       </div>
     </ContextMenu>
   )

diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss
@@ -69,7 +69,7 @@ $bullet-size: calc(var(--design-unit) * 4px);
   ul[role='menu'] {
     background-color: $header-bg-color;
     padding-left: 2px;
-    margin: 0;
+    margin: 0px 0px 0px 4px;
     border: none;
 
     button {
@@ -86,6 +86,10 @@ $bullet-size: calc(var(--design-unit) * 4px);
 .moveToRight {
   left: auto;
   right: 0;
+
+  ul[role='menu'] {
+    margin: 4px 0px 0px 0px;
+  }
 }
 
 // Concrete selectors
@@ -561,6 +565,14 @@ $bullet-size: calc(var(--design-unit) * 4px);
     &.leafHeader {
       border-bottom: none;
     }
+
+    &.headerCellDropTarget {
+      background: $header-dnd-drop-background;
+      outline-width: 2px;
+      outline-style: dashed;
+      outline-color: $header-dnd-outline;
+      outline-offset: -4px;
+    }
   }
 }
 
@@ -612,24 +624,10 @@ $bullet-size: calc(var(--design-unit) * 4px);
   right: -2px;
 }
 
-.draggingColumn {
-  border: 1px solid $fg-color;
-  font-size: 0.7rem;
-  opacity: 0.7;
-}
-
 .staticColumn {
   transform: translate(0, 0) !important;
 }
 
-.isDroppedColumn {
-  transition-duration: 100;
-}
-
-.dndPlaceholder {
-  display: none;
-}
-
 .columnResizer {
   right: -4px;
   width: 10px;

diff --git a/webview/src/experiments/util/columns.test.ts b/webview/src/experiments/util/columns.test.ts
@@ -15,8 +15,8 @@ describe('reorderColumnIds()', () => {
       'id_1'
     ])
     expect(reorderColumnIds(twoColumnIds, ['id_2'], ['id_1'])).toStrictEqual([
-      'id_1',
-      'id_2'
+      'id_2',
+      'id_1'
     ])
 
     const threeColumnIds = [...twoColumnIds, 'id_3']
@@ -41,6 +41,6 @@ describe('reorderColumnIds()', () => {
     ).toStrictEqual(['id_3', 'id_1', 'id_2'])
     expect(
       reorderColumnIds(threeColumnIds, ['id_2', 'id_3'], ['id_1'])
-    ).toStrictEqual(['id_1', 'id_2', 'id_3'])
+    ).toStrictEqual(['id_2', 'id_3', 'id_1'])
   })
 })
diff --git a/webview/src/experiments/util/columns.ts b/webview/src/experiments/util/columns.ts
@@ -99,8 +99,8 @@ export const reorderColumnIds = (
 
   return [
     ...columnIds.slice(0, displacedIndex),
-    ...displaced,
     ...displacer,
+    ...displaced,
     ...columnIds.slice(displacedIndex + displaced.length, displacerIndex),
     ...columnIds.slice(displacerIndex + displacer.length)
   ]

diff --git a/webview/src/shared/components/iconMenu/IconMenu.tsx b/webview/src/shared/components/iconMenu/IconMenu.tsx
@@ -26,6 +26,8 @@ export const IconMenu: React.FC<IconMenuProps> = ({ items }) => {
     disabled: tooltipDisabled
   })
 
+  const visibleItems = items.filter(({ hidden }) => !hidden)
+
   return (
     <Tooltip
       singleton={tooltipSource}
@@ -46,18 +48,20 @@ export const IconMenu: React.FC<IconMenuProps> = ({ items }) => {
           setTooltipDisabled(false)
         }}
       >
-        <ul className={styles.menu} role="menu">
-          {items
-            .filter(({ hidden }) => !hidden)
-            .map(item => (
+        {visibleItems.length > 0 ? (
+          <ul className={styles.menu} role="menu">
+            {visibleItems.map(item => (
               <IconMenuItem
                 {...item}
                 key={item.tooltip}
                 tooltipTarget={tooltipTarget}
                 menuTarget={menuTarget}
               />
             ))}
-        </ul>
+          </ul>
+        ) : (
+          <></>
+        )}
       </Tooltip>
     </Tooltip>
   )

diff --git a/webview/src/shared/variables.scss b/webview/src/shared/variables.scss
@@ -18,6 +18,8 @@ $row-fg-selected-color: var(--vscode-list-activeSelectionForeground);
 $row-border-selected-color: var(--vscode-list-focusOutline);
 $header-bg-color: $bg-color;
 $header-border-color: $border-color;
+$header-dnd-outline: var(--contrast-active-border);
+$header-dnd-drop-background: var(--vscode-editorGroup-dropBackground);
 $meta-cell-color: var(--vscode-descriptionForeground);
 $header-resizer-color: var(--vscode-sash-hoverBorder);
 $header-drop-target-color: var(--vscode-focusBorder);