Split drag and drop container into different files (#4880)

* Split drag and drop container in multiple files * Fix undefined * Remove mutation of item * Fix tests
iterative · Oct 26, 2023 · 43f014b · 43f014b
1 parent 9dc008c
commit 43f014b
Show file tree

Hide file tree

Showing 7 changed files with 169 additions and 88 deletions.
diff --git a/webview/src/plots/components/comparisonTable/ComparisonTable.test.tsx b/webview/src/plots/components/comparisonTable/ComparisonTable.test.tsx
@@ -28,7 +28,7 @@ import { webviewInitialState } from '../webviewSlice'
 import { getThemeValue, hexToRGB, ThemeProperty } from '../../../util/styles'
 import * as EventCurrentTargetDistances from '../../../shared/components/dragDrop/currentTarget'
 
-const getHeaders = () => screen.getAllByRole('columnheader')
+const getHeaders = (): HTMLElement[] => screen.getAllByRole('columnheader')
 
 jest.mock('../../../shared/api')
 jest.mock('../../../shared/components/dragDrop/currentTarget', () => {
@@ -489,7 +489,7 @@ describe('ComparisonTable', () => {
 
       const [, draggedHeader] = getHeaders()
 
-      expect(draggedHeader.isSameNode(startingNode)).toBe(true)
+      expect(draggedHeader.isEqualNode(startingNode)).toBe(true)
     })
 
     it('should wrap the drop target with the header we are dragging over', () => {
@@ -503,26 +503,25 @@ describe('ComparisonTable', () => {
 
       // eslint-disable-next-line testing-library/no-node-access
       expect(headerWrapper.childElementCount).toBe(2)
-      expect(headerWrapper.contains(endingNode)).toBe(true)
+      expect(
+        // eslint-disable-next-line testing-library/no-node-access
+        Object.values(headerWrapper.children)
+          .map(child => child.id)
+          .includes(endingNode.id)
+      ).toBe(true)
     })
 
     it('should not change the order when dropping a header in its own spot', () => {
       renderTable()
 
-      const [startingAndEndingNode, secondEndingNode] = getHeaders()
+      const [startingNode] = getHeaders()
 
-      dragAndDrop(
-        startingAndEndingNode,
-        startingAndEndingNode,
-        DragEnterDirection.RIGHT
-      )
+      dragAndDrop(startingNode, startingNode, DragEnterDirection.RIGHT)
       expect(mockPostMessage).not.toHaveBeenCalled()
 
-      dragAndDrop(
-        startingAndEndingNode,
-        secondEndingNode,
-        DragEnterDirection.RIGHT
-      )
+      const [start, end] = getHeaders()
+
+      dragAndDrop(start, end, DragEnterDirection.RIGHT)
       expect(mockPostMessage).toHaveBeenCalled()
     })
 

diff --git a/webview/src/plots/components/templatePlots/TemplatePlots.tsx b/webview/src/plots/components/templatePlots/TemplatePlots.tsx
@@ -12,7 +12,7 @@ import styles from '../styles.module.scss'
 import { shouldUseVirtualizedGrid } from '../util'
 import { PlotsState } from '../../store'
 import { setDraggedOverGroup } from '../../../shared/components/dragDrop/dragDropSlice'
-import { isSameGroup } from '../../../shared/components/dragDrop/DragDropContainer'
+import { isSameGroup } from '../../../shared/components/dragDrop/util'
 import { changeOrderWithDraggedInfo } from '../../../util/array'
 import { LoadingSection, sectionIsLoading } from '../LoadingSection'
 import { reorderTemplatePlots } from '../../util/messages'

diff --git a/webview/src/shared/components/dragDrop/DragDropContainer.tsx b/webview/src/shared/components/dragDrop/DragDropContainer.tsx
@@ -9,36 +9,33 @@ import React, {
   useLayoutEffect
 } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
-import { DragEnterDirection, getDragEnterDirection } from './util'
+import {
+  DragEnterDirection,
+  getDragEnterDirection,
+  isEnteringAfter,
+  isExactGroup,
+  isSameGroup
+} from './util'
 import { changeRef, setDraggedOverGroup } from './dragDropSlice'
 import styles from './styles.module.scss'
 import { DropTarget } from './DropTarget'
-import { getIDIndex, getIDWithoutIndex } from '../../../util/ids'
+import { DragDropItemWithTarget } from './DragDropItemWithTarget'
+import { DragDropItem } from './DragDropItem'
+import { getIDIndex } from '../../../util/ids'
 import { Any } from '../../../util/objects'
 import { PlotsState } from '../../../plots/store'
 import { getStyleProperty } from '../../../util/styles'
 import { idToNode } from '../../../util/helpers'
 import { useDeferedDragLeave } from '../../hooks/useDeferedDragLeave'
 
-const AFTER_DIRECTIONS = new Set([
-  DragEnterDirection.RIGHT,
-  DragEnterDirection.BOTTOM
-])
-
 const orderIdxTune = (direction: DragEnterDirection, isAfter: boolean) => {
-  if (AFTER_DIRECTIONS.has(direction)) {
+  if (isEnteringAfter(direction)) {
     return isAfter ? 0 : 1
   }
 
   return isAfter ? -1 : 0
 }
 
-export const isSameGroup = (group1?: string, group2?: string) =>
-  getIDWithoutIndex(group1) === getIDWithoutIndex(group2)
-
-const isExactGroup = (group1?: string, group1Alt?: string, group2?: string) =>
-  group1 === group2 || group1Alt === group2
-
 const setStyle = (elem: HTMLElement, style: CSSProperties, reset?: boolean) => {
   for (const [rule, value] of Object.entries(style)) {
     elem.style[getStyleProperty(rule)] = reset ? '' : value
@@ -199,7 +196,6 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
     const dragged = newOrder[draggedIndex]
     newOrder.splice(draggedIndex, 1)
     newOrder.splice(droppedIndex, 0, dragged)
-
     setOrder(newOrder)
     dispatch(changeRef(undefined))
 
@@ -266,26 +262,6 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
     deferedDragLeave()
   }
 
-  const buildItem = (id: string, draggable: JSX.Element) => (
-    <draggable.type
-      key={draggable.key}
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      ref={(draggable as any).ref}
-      {...draggable.props}
-      onDragStart={handleDragStart}
-      onDragEnd={cleanup}
-      onDragOver={handleDragOver}
-      onDragEnter={handleDragEnter}
-      onDrop={handleOnDrop}
-      onDragLeave={handleDragLeave}
-      draggable={!disabledDropIds.includes(id)}
-      style={
-        (!shouldShowOnDrag && id === draggedId && { display: 'none' }) ||
-        draggable.props.style
-      }
-    />
-  )
-
   const getDropTargetClassNames = (isEnteringRight: boolean) =>
     shouldShowOnDrag
       ? cx(styles.dropTargetWhenShowingOnDrag, {
@@ -300,7 +276,7 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
     wrapper: JSX.Element
   ) => (
     <DropTarget
-      key="drop-target"
+      key={`drop-target-${id}`}
       onDragOver={handleDragOver}
       onDragLeave={handleDragLeave}
       onDrop={handleOnDrop}
@@ -312,50 +288,63 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
     </DropTarget>
   )
 
-  const createItemWithDropTarget = (id: string, item: JSX.Element) => {
-    const isEnteringAfter = AFTER_DIRECTIONS.has(direction)
-    const target = isExactGroup(draggedOverGroup, draggedRef?.group, group)
-      ? getTarget(
-          id,
-          isEnteringAfter,
-          shouldShowOnDrag ? <div /> : <item.type />
-        )
-      : null
-
-    const itemWithTag = shouldShowOnDrag ? (
-      <div key="item" {...item.props} />
-    ) : (
-      item
-    )
-    const block = isEnteringAfter
-      ? [itemWithTag, target]
-      : [target, itemWithTag]
-
-    return shouldShowOnDrag ? (
-      <item.type key={item.key} className={styles.newBlockWhenShowingOnDrag}>
-        {block}
-      </item.type>
-    ) : (
-      block
-    )
-  }
+  const wrappedItems = items
+    .map(draggable => {
+      const id = draggable.props.id
+      const isDraggedOver =
+        id === draggedOverId && (hoveringSomething || !parentDraggedOver)
+
+      const item = (
+        <DragDropItem
+          key={draggable.key}
+          cleanup={cleanup}
+          disabledDropIds={disabledDropIds}
+          draggable={draggable}
+          id={id}
+          onDragEnter={handleDragEnter}
+          onDragLeave={handleDragLeave}
+          onDragOver={handleDragOver}
+          onDragStart={handleDragStart}
+          onDrop={handleOnDrop}
+          draggedId={draggedId}
+          shouldShowOnDrag={shouldShowOnDrag}
+          isDiv={isDraggedOver && shouldShowOnDrag}
+        />
+      )
 
-  const wrappedItems = items.flatMap(draggable => {
-    const id = draggable?.props?.id
-    const item = id && buildItem(id, draggable)
+      if (isDraggedOver) {
+        const isAfter = isEnteringAfter(direction)
+        const target = isExactGroup(draggedOverGroup, draggedRef?.group, group)
+          ? getTarget(
+              id,
+              isAfter,
+              shouldShowOnDrag ? <div /> : <draggable.type />
+            )
+          : null
+
+        return (
+          <DragDropItemWithTarget
+            key={draggable.key}
+            isAfter={isAfter}
+            dropTarget={target}
+            shouldShowOnDrag={shouldShowOnDrag}
+            draggable={draggable}
+          >
+            {item}
+          </DragDropItemWithTarget>
+        )
+      }
 
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-return
-    return id === draggedOverId && (hoveringSomething || !parentDraggedOver)
-      ? createItemWithDropTarget(id, item)
-      : item
-  })
+      return item
+    })
+    .filter(Boolean) as JSX.Element[]
 
   if (
     isSameGroup(draggedRef?.group, group) &&
     !hoveringSomething &&
     parentDraggedOver
   ) {
-    const lastItem = wrappedItems[wrappedItems.length - 1]
+    const lastItem = items[items.length - 1]
     wrappedItems.push(getTarget(lastItem.props.id, false, <lastItem.type />))
   }
 

diff --git a/webview/src/shared/components/dragDrop/DragDropItem.tsx b/webview/src/shared/components/dragDrop/DragDropItem.tsx
@@ -0,0 +1,51 @@
+import React, { DragEvent } from 'react'
+
+interface DragDropItemProps {
+  id: string
+  draggable: JSX.Element
+  onDragStart: (e: DragEvent<HTMLElement>) => void
+  onDragOver: (e: DragEvent<HTMLElement>) => void
+  onDragEnter: (e: DragEvent<HTMLElement>) => void
+  onDrop: (e: DragEvent<HTMLElement>) => void
+  onDragLeave: (e: DragEvent<HTMLElement>) => void
+  cleanup: () => void
+  disabledDropIds: string[]
+  shouldShowOnDrag?: boolean
+  draggedId?: string
+  isDiv?: boolean
+}
+
+export const DragDropItem: React.FC<DragDropItemProps> = ({
+  id,
+  draggable,
+  onDragStart,
+  onDragOver,
+  onDragEnter,
+  onDrop,
+  onDragLeave,
+  cleanup,
+  disabledDropIds,
+  shouldShowOnDrag,
+  draggedId,
+  isDiv
+}) => {
+  const Type = isDiv ? 'div' : draggable.type
+  return (
+    <Type
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      ref={(draggable as any).ref}
+      {...draggable.props}
+      onDragStart={onDragStart}
+      onDragEnd={cleanup}
+      onDragOver={onDragOver}
+      onDragEnter={onDragEnter}
+      onDrop={onDrop}
+      onDragLeave={onDragLeave}
+      draggable={!disabledDropIds.includes(id)}
+      style={
+        (!shouldShowOnDrag && id === draggedId && { display: 'none' }) ||
+        draggable.props.style
+      }
+    />
+  )
+}
diff --git a/webview/src/shared/components/dragDrop/DragDropItemWithTarget.tsx b/webview/src/shared/components/dragDrop/DragDropItemWithTarget.tsx
@@ -0,0 +1,23 @@
+import React, { PropsWithChildren } from 'react'
+import styles from './styles.module.scss'
+
+interface DragDropItemWithTargetProps {
+  dropTarget: JSX.Element | null
+  draggable: JSX.Element
+  isAfter?: boolean
+  shouldShowOnDrag?: boolean
+}
+
+export const DragDropItemWithTarget: React.FC<
+  PropsWithChildren<DragDropItemWithTargetProps>
+> = ({ dropTarget, isAfter, shouldShowOnDrag, draggable, children }) => {
+  const block = isAfter ? [children, dropTarget] : [dropTarget, children]
+
+  return shouldShowOnDrag ? (
+    <draggable.type className={styles.newBlockWhenShowingOnDrag}>
+      {block}
+    </draggable.type>
+  ) : (
+    block
+  )
+}
diff --git a/webview/src/shared/components/dragDrop/util.ts b/webview/src/shared/components/dragDrop/util.ts
@@ -1,5 +1,6 @@
 import { DragEvent } from 'react'
 import { getEventCurrentTargetDistances } from './currentTarget'
+import { getIDWithoutIndex } from '../../../util/ids'
 
 export enum DragEnterDirection {
   RIGHT = 'RIGHT',
@@ -9,6 +10,11 @@ export enum DragEnterDirection {
   BOTTOM = 'BOTTOM'
 }
 
+const AFTER_DIRECTIONS = new Set([
+  DragEnterDirection.RIGHT,
+  DragEnterDirection.BOTTOM
+])
+
 export const getDragEnterDirection = (
   e: DragEvent<HTMLElement>,
   vertical?: boolean
@@ -40,3 +46,15 @@ export const getDragEnterDirection = (
     ? DragEnterDirection.TOP
     : DragEnterDirection.BOTTOM
 }
+
+export const isEnteringAfter = (direction: DragEnterDirection) =>
+  AFTER_DIRECTIONS.has(direction)
+
+export const isExactGroup = (
+  group1?: string,
+  group1Alt?: string,
+  group2?: string
+) => group1 === group2 || group1Alt === group2
+
+export const isSameGroup = (group1?: string, group2?: string) =>
+  getIDWithoutIndex(group1) === getIDWithoutIndex(group2)
diff --git a/webview/src/test/dragDrop.ts b/webview/src/test/dragDrop.ts
@@ -109,7 +109,8 @@ export const dragAndDrop = (
   direction: DragEnterDirection = DragEnterDirection.LEFT,
   spyableModule?: SpyableEventCurrentTargetDistances
 ) => {
-  // When showing element on drag, the dragged over element is being recreated to be wrapped in another element, thus the endingNode does not exist as is in the document
+  // When showing element on drag, the dragged over element is being recreated to be wrapped in another element,
+  // thus the endingNode does not exist as is in the document
   const endingNodeId = endingNode.id
   dragEnter(startingNode, endingNodeId, direction, spyableModule)