table dnd cleanup, reset drop zone on leave (#2965)

Author: shcheklein

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

@@ -30,7 +30,7 @@ import {
   getHeaders,
   tableData as sortingTableDataFixture
 } from '../../../test/sort'
-import { dragAndDrop } from '../../../test/dragDrop'
+import { dragAndDrop, dragEnter, dragLeave } from '../../../test/dragDrop'
 import { DragEnterDirection } from '../../../shared/components/dragDrop/util'
 import { experimentsReducers } from '../../store'
 import { customQueries } from '../../../test/queries'
@@ -463,5 +463,29 @@ describe('Table', () => {
         headers.indexOf('test')
       )
     })
+
+    it('should remove the drop zone when hovering out a target column header', async () => {
+      const { getDraggableHeaderFromText } = renderExperimentsTable({
+        ...tableDataFixture
+      })
+
+      await getHeaders()
+
+      const startingNode = screen.getByText('process')
+      const targetNode = getDraggableHeaderFromText('loss')
+      const header = screen.getByTestId('header-metrics:summary.json:loss')
+
+      dragEnter(startingNode, targetNode.id, DragEnterDirection.AUTO)
+
+      expect(
+        header?.classList.contains(styles.headerCellDropTarget)
+      ).toBeTruthy()
+
+      dragLeave(targetNode)
+
+      expect(
+        header?.classList.contains(styles.headerCellDropTarget)
+      ).toBeFalsy()
+    })
   })
 })

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

@@ -10,20 +10,22 @@ export const MergedHeaderGroups: React.FC<{
   headerGroup: HeaderGroup<Experiment>
   columns: HeaderGroup<Experiment>[]
   orderedColumns: Column[]
-  onDragUpdate: DragFunction
+  onDragEnter: DragFunction
   onDragStart: DragFunction
   onDragEnd: DragFunction
   onDrop: DragFunction
+  onDragLeave: DragFunction
   setExpColumnNeedsShadow: (needsShadow: boolean) => void
   root: HTMLElement | null
 }> = ({
   headerGroup,
   columns,
   orderedColumns,
-  onDragUpdate,
+  onDragEnter,
   onDragEnd,
   onDragStart,
   onDrop,
+  onDragLeave,
   root,
   setExpColumnNeedsShadow
 }) => {
@@ -40,10 +42,11 @@ export const MergedHeaderGroups: React.FC<{
           orderedColumns={orderedColumns}
           column={column}
           columns={columns}
-          onDragEnter={onDragUpdate}
+          onDragEnter={onDragEnter}
           onDragStart={onDragStart}
           onDragEnd={onDragEnd}
           onDrop={onDrop}
+          onDragLeave={onDragLeave}
           root={root}
         />
       ))}

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

@@ -82,7 +82,7 @@ export const TableHead = ({
     displacedHeader && cb(displacedHeader)
   }
 
-  const onDragUpdate = (e: DragEvent<HTMLElement>) => {
+  const onDragEnter = (e: DragEvent<HTMLElement>) => {
     findDisplacedHeader(e.currentTarget.id, displacedHeader => {
       if (!isExperimentColumn(displacedHeader.id)) {
         dispatch(setDropTarget(displacedHeader.id))
@@ -96,6 +96,12 @@ export const TableHead = ({
     dispatch(setDropTarget(''))
   }
 
+  const onDragLeave = () => {
+    // note: for this to work it's important to have `pointer-events: none;`
+    // on text children to avoid duplicate dragEnter and dragLeave events fired
+    dispatch(setDropTarget(''))
+  }
+
   const onDrop = () => {
     const fullOrder = fullColumnOrder.current
     const displacer = draggingIds.current
@@ -130,9 +136,10 @@ export const TableHead = ({
           headerGroup={headerGroup}
           columns={allHeaders}
           onDragStart={onDragStart}
-          onDragUpdate={onDragUpdate}
+          onDragEnter={onDragEnter}
           onDragEnd={onDragEnd}
           onDrop={onDrop}
+          onDragLeave={onDragLeave}
           root={root}
           setExpColumnNeedsShadow={setExpColumnNeedsShadow}
         />

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

@@ -14,6 +14,7 @@ interface TableHeaderProps {
   onDragEnd: DragFunction
   onDragStart: DragFunction
   onDrop: DragFunction
+  onDragLeave: DragFunction
   setExpColumnNeedsShadow: (needsShadow: boolean) => void
   root: HTMLElement | null
 }
@@ -26,6 +27,7 @@ export const TableHeader: React.FC<TableHeaderProps> = ({
   onDragEnd,
   onDragStart,
   onDrop,
+  onDragLeave,
   root,
   setExpColumnNeedsShadow
 }) => {
@@ -43,6 +45,7 @@ export const TableHeader: React.FC<TableHeaderProps> = ({
       onDragEnd={onDragEnd}
       onDragStart={onDragStart}
       onDrop={onDrop}
+      onDragLeave={onDragLeave}
       root={root}
       setExpColumnNeedsShadow={setExpColumnNeedsShadow}
     />

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

@@ -93,6 +93,7 @@ export const TableHeaderCell: React.FC<{
   onDragEnd: DragFunction
   onDragStart: DragFunction
   onDrop: DragFunction
+  onDragLeave: DragFunction
   setExpColumnNeedsShadow: (needsShadow: boolean) => void
   root: HTMLElement | null
 }> = ({
@@ -104,6 +105,7 @@ export const TableHeaderCell: React.FC<{
   onDragEnd,
   onDragStart,
   onDrop,
+  onDragLeave,
   root,
   setExpColumnNeedsShadow
 }) => {
@@ -140,6 +142,7 @@ export const TableHeaderCell: React.FC<{
       onDragStart={onDragStart}
       onDragEnd={onDragEnd}
       onDrop={onDrop}
+      onDragLeave={onDragLeave}
       canResize={canResize}
       setMenuSuppressed={setMenuSuppressed}
       resizerHeight={resizerHeight}

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

@@ -35,9 +35,16 @@ export const ColumnDragHandle: React.FC<{
   onDragStart: DragFunction
   onDrop: DragFunction
   onDragEnd: DragFunction
-}> = ({ disabled, column, onDragEnter, onDragStart, onDragEnd, onDrop }) => {
-  const DropTarget = <span>{column?.name}</span>
-
+  onDragLeave: DragFunction
+}> = ({
+  disabled,
+  column,
+  onDragEnter,
+  onDragStart,
+  onDragEnd,
+  onDrop,
+  onDragLeave
+}) => {
   return (
     <span
       data-testid="rendered-header"
@@ -49,11 +56,11 @@ export const ColumnDragHandle: React.FC<{
         id={column.id}
         disabled={disabled}
         group={'experiment-table'}
-        dropTarget={DropTarget}
         onDragEnter={onDragEnter}
         onDragStart={onDragStart}
         onDragEnd={onDragEnd}
         onDrop={onDrop}
+        onDragLeave={onDragLeave}
       >
         <span>{column?.render('Header')}</span>
       </Draggable>
@@ -72,6 +79,7 @@ export const TableHeaderCellContents: React.FC<{
   onDragEnd: DragFunction
   onDragStart: DragFunction
   onDrop: DragFunction
+  onDragLeave: DragFunction
   canResize: boolean
   setMenuSuppressed: (menuSuppressed: boolean) => void
   resizerHeight: string
@@ -86,6 +94,7 @@ export const TableHeaderCellContents: React.FC<{
   onDragEnd,
   onDragStart,
   onDrop,
+  onDragLeave,
   canResize,
   setMenuSuppressed,
   resizerHeight
@@ -117,6 +126,7 @@ export const TableHeaderCellContents: React.FC<{
         onDragStart={onDragStart}
         onDrop={onDrop}
         onDragEnd={onDragEnd}
+        onDragLeave={onDragLeave}
       />
       {canResize && (
         <div

webview/src/experiments/components/table/styles.module.scss

@@ -349,6 +349,11 @@ $bullet-size: calc(var(--design-unit) * 4px);
       @extend %truncateLeftParent;
       direction: rtl;
       opacity: 0.6;
+
+      span {
+        // to prevent extra dragLeave and dragEnter fired
+        pointer-events: none;
+      }
     }
     .cellContents {
       @extend %truncateLeftChild;

webview/src/shared/components/dragDrop/Draggable.tsx

@@ -1,7 +1,6 @@
 import React, { DragEvent } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
 import { setGroup } from './dragDropSlice'
-import { DropTarget } from './DropTarget'
 import { ExperimentsState } from '../../../experiments/store'
 
 export type DragFunction = (e: DragEvent<HTMLElement>) => void
@@ -11,29 +10,29 @@ export interface DraggableProps {
   group: string
   disabled: boolean
   children: JSX.Element
-  dropTarget: JSX.Element
   onDrop: DragFunction
   onDragStart: DragFunction
   onDragEnter: DragFunction
   onDragEnd: DragFunction
+  onDragLeave: DragFunction
 }
 
 export const Draggable: React.FC<DraggableProps> = ({
   id,
   group,
   children,
   disabled,
-  dropTarget,
   onDrop,
   onDragEnter,
   onDragStart,
-  onDragEnd
+  onDragEnd,
+  onDragLeave
 }) => {
   const groupState = useSelector(
     (state: ExperimentsState) => state.dragAndDrop.groups[group] || {}
   )
   const dispatch = useDispatch()
-  const { draggedOverId, draggedId } = groupState
+  const { draggedId } = groupState
 
   const modifyGroup = (id: string) => {
     dispatch(
@@ -60,9 +59,7 @@ export const Draggable: React.FC<DraggableProps> = ({
   const handleDragEnter = (e: DragEvent<HTMLElement>) => {
     if (draggedId) {
       const { id } = e.currentTarget
-
-      if (id !== draggedId && id !== draggedOverId) {
-        modifyGroup(id)
+      if (id !== draggedId) {
         onDragEnter(e)
       }
     }
@@ -77,33 +74,19 @@ export const Draggable: React.FC<DraggableProps> = ({
       setGroup({
         group: {
           ...groupState,
-          draggedId: undefined,
-          draggedOverId: undefined
+          draggedId: undefined
         },
         id: group
       })
     )
     onDragEnd(e)
   }
 
-  if (dropTarget && id === draggedOverId) {
-    return (
-      <DropTarget
-        onDragOver={handleDragOver}
-        onDragEnd={onDragEnd}
-        onDrop={onDrop}
-        id={id}
-        key="drop-target"
-      >
-        {dropTarget}
-      </DropTarget>
-    )
-  }
-
   return (
     <children.type
       {...children.props}
       id={id}
+      onDragLeave={onDragLeave}
       onDragStart={handleDragStart}
       onDragEnd={handleDragEnd}
       onDragOver={handleDragOver}