Merge pull request #1729 from iterative/groups-dnd-with-stephs-implementation

Groups DnD with our own version

Author: wolmir

webview/package.json

@@ -26,7 +26,6 @@
     "@vscode/webview-ui-toolkit": "^1.0.0",
     "classnames": "^2.2.6",
     "react": "^17.0.1",
-    "react-beautiful-dnd": "^13.1.0",
     "react-dom": "^17.0.1",
     "react-table": "^7.7.0",
     "react-vega": "^7.4.4",
@@ -49,7 +48,6 @@
     "@types/jsdom": "^16.2.6",
     "@types/node": "^16.11.8",
     "@types/react": "^17.0.35",
-    "@types/react-beautiful-dnd": "^13.1.2",
     "@types/react-dom": "^17.0.11",
     "@types/react-measure": "^2.0.8",
     "@types/react-table": "^7.7.8",
@@ -66,7 +64,6 @@
     "jest-canvas-mock": "^2.3.1",
     "lint-staged": "^12.3.7",
     "raw-loader": "^4.0.2",
-    "react-beautiful-dnd-test-utils": "^3.2.2",
     "sass": "^1.43.4",
     "sass-loader": "^12.3.0",
     "style-loader": "^3.3.1",

webview/src/experiments/components/App.test.tsx

@@ -18,12 +18,6 @@ import {
   MessageFromWebviewType,
   MessageToWebviewType
 } from 'dvc/src/webview/contract'
-import {
-  mockDndElSpacing,
-  mockGetComputedSpacing,
-  makeDnd,
-  DND_DIRECTION_RIGHT
-} from 'react-beautiful-dnd-test-utils'
 import {
   Column,
   ColumnType,
@@ -38,14 +32,15 @@ import { vsCodeApi } from '../../shared/api'
 import {
   commonColumnFields,
   expectHeaders,
-  makeGetDragEl,
   tableData as sortingTableDataFixture
 } from '../../test/sort'
 import {
   CELL_TOOLTIP_DELAY,
   HEADER_TOOLTIP_DELAY
 } from '../../shared/components/tooltip/Tooltip'
 import { getRow } from '../../test/queries'
+import { dragAndDrop } from '../../test/dragDrop'
+import { DragEnterDirection } from '../../shared/components/dragDrop/util'
 
 jest.mock('../../shared/api')
 jest.mock('../../util/styles')
@@ -200,9 +195,7 @@ describe('App', () => {
   })
 
   it('should be able to order a column to the final space after a new column is added', async () => {
-    const view = render(<App />)
-    mockDndElSpacing(view)
-    mockGetComputedSpacing()
+    render(<App />)
     fireEvent(
       window,
       new MessageEvent('message', {
@@ -236,12 +229,10 @@ describe('App', () => {
       })
     )
 
-    await makeDnd({
-      direction: DND_DIRECTION_RIGHT,
-      getByText: view.getByText,
-      getDragEl: makeGetDragEl('B'),
-      positions: 2
-    })
+    const headerB = screen.getByText('B')
+    const headerD = screen.getByText('D')
+
+    dragAndDrop(headerB, headerD, DragEnterDirection.AUTO)
 
     await expectHeaders(['A', 'C', 'D', 'B'])
   })

webview/src/experiments/components/Experiments.tsx

@@ -24,6 +24,7 @@ import buildDynamicColumns from '../util/buildDynamicColumns'
 import { sendMessage } from '../../shared/vscode'
 import { useThemeVariables } from '../../shared/components/theme/Theme'
 import { EmptyState } from '../../shared/components/emptyState/EmptyState'
+import { DragDropProvider } from '../../shared/components/dragDrop/DragDropContext'
 
 const DEFAULT_COLUMN_WIDTH = 75
 const MINIMUM_COLUMN_WIDTH = 50
@@ -208,7 +209,11 @@ export const ExperimentsTable: React.FC<{
     return <EmptyState>No Experiments to Display.</EmptyState>
   }
 
-  return <Table instance={instance} tableData={tableData} />
+  return (
+    <DragDropProvider>
+      <Table instance={instance} tableData={tableData} />
+    </DragDropProvider>
+  )
 }
 
 const Experiments: React.FC<{

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

@@ -3,54 +3,50 @@ import cx from 'classnames'
 import { SortDefinition } from 'dvc/src/experiments/model/sortBy'
 import { Experiment, Column } from 'dvc/src/experiments/webview/contract'
 import { HeaderGroup } from 'react-table'
-import { DragDropContext, Droppable, Responders } from 'react-beautiful-dnd'
 import { TableHeader } from './TableHeader'
 import styles from './styles.module.scss'
+import {
+  OnDragOver,
+  OnDragStart,
+  OnDrop
+} from '../../../shared/components/dragDrop/DragDropWorkbench'
 
-export const MergedHeaderGroup: React.FC<
-  {
-    headerGroup: HeaderGroup<Experiment>
-    columns: HeaderGroup<Experiment>[]
-    sorts: SortDefinition[]
-    orderedColumns: Column[]
-  } & Responders
-> = ({
+export const MergedHeaderGroups: React.FC<{
+  headerGroup: HeaderGroup<Experiment>
+  columns: HeaderGroup<Experiment>[]
+  sorts: SortDefinition[]
+  orderedColumns: Column[]
+  onDragUpdate: OnDragOver
+  onDragStart: OnDragStart
+  onDragEnd: OnDrop
+}> = ({
   headerGroup,
   sorts,
   columns,
   orderedColumns,
-  onDragStart,
   onDragUpdate,
-  onDragEnd
+  onDragEnd,
+  onDragStart
 }) => {
   return (
-    <DragDropContext
-      onDragStart={onDragStart}
-      onDragUpdate={onDragUpdate}
-      onDragEnd={onDragEnd}
+    <div
+      {...headerGroup.getHeaderGroupProps({
+        className: cx(styles.tr, styles.headerRow)
+      })}
     >
-      <Droppable droppableId="droppable" direction="horizontal">
-        {provided => (
-          <div
-            ref={provided.innerRef}
-            {...headerGroup.getHeaderGroupProps({
-              className: cx(styles.tr, styles.headerRow)
-            })}
-          >
-            {headerGroup.headers.map((column: HeaderGroup<Experiment>, i) => (
-              <TableHeader
-                orderedColumns={orderedColumns}
-                key={column.id}
-                column={column}
-                columns={columns}
-                sorts={sorts}
-                index={i}
-              />
-            ))}
-            <div className={styles.dndPlaceholder}>{provided.placeholder}</div>
-          </div>
-        )}
-      </Droppable>
-    </DragDropContext>
+      {headerGroup.headers.map((column: HeaderGroup<Experiment>) => (
+        <div key={column.id}>
+          <TableHeader
+            orderedColumns={orderedColumns}
+            column={column}
+            columns={columns}
+            sorts={sorts}
+            onDragOver={onDragUpdate}
+            onDragStart={onDragStart}
+            onDrop={onDragEnd}
+          />
+        </div>
+      ))}
+    </div>
   )
 }

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

@@ -14,13 +14,7 @@ import { Experiment, TableData } from 'dvc/src/experiments/webview/contract'
 import { MessageFromWebviewType } from 'dvc/src/webview/contract'
 import React from 'react'
 import { TableInstance } from 'react-table'
-import {
-  mockGetComputedSpacing,
-  mockDndElSpacing,
-  makeDnd,
-  DND_DIRECTION_LEFT,
-  DND_DIRECTION_RIGHT
-} from 'react-beautiful-dnd-test-utils'
+import tableDataFixture from 'dvc/src/test/fixtures/expShow/tableData'
 import { SortOrderLabel } from './SortPicker'
 import { Table } from './Table'
 import styles from './styles.module.scss'
@@ -30,9 +24,11 @@ import * as ColumnOrder from '../../hooks/useColumnOrder'
 import { vsCodeApi } from '../../../shared/api'
 import {
   expectHeaders,
-  makeGetDragEl,
+  getHeaders,
   tableData as sortingTableDataFixture
 } from '../../../test/sort'
+import { dragAndDrop } from '../../../test/dragDrop'
+import { DragEnterDirection } from '../../../shared/components/dragDrop/util'
 
 jest.mock('../../../shared/api')
 const { postMessage } = vsCodeApi
@@ -127,11 +123,7 @@ describe('Table', () => {
   const renderExperimentsTable = (
     data: TableData = sortingTableDataFixture
   ) => {
-    const view = render(<ExperimentsTable tableData={data} />)
-
-    mockDndElSpacing(view)
-
-    return view
+    return render(<ExperimentsTable tableData={data} />)
   }
 
   beforeAll(() => {
@@ -293,45 +285,38 @@ describe('Table', () => {
   })
 
   describe('Columns order', () => {
-    beforeEach(() => {
-      mockGetComputedSpacing()
-    })
-
     it('should move a column from its current position to its new position', async () => {
-      const { getByText } = renderExperimentsTable()
+      renderExperimentsTable()
 
       await expectHeaders(['A', 'B', 'C'])
 
-      await makeDnd({
-        direction: DND_DIRECTION_LEFT,
-        getByText,
-        getDragEl: makeGetDragEl('C'),
-        positions: 1
-      })
+      dragAndDrop(
+        screen.getByText('B'),
+        screen.getByText('C'),
+        DragEnterDirection.AUTO
+      )
 
       await expectHeaders(['A', 'C', 'B'])
 
-      await makeDnd({
-        direction: DND_DIRECTION_RIGHT,
-        getByText,
-        getDragEl: makeGetDragEl('A'),
-        positions: 2
-      })
+      dragAndDrop(
+        screen.getByText('A'),
+        screen.getByText('B'),
+        DragEnterDirection.AUTO
+      )
 
       await expectHeaders(['C', 'B', 'A'])
     })
 
     it('should not move a column before the default columns', async () => {
-      const { getByText } = renderExperimentsTable()
+      renderExperimentsTable()
 
-      await makeDnd({
-        direction: DND_DIRECTION_LEFT,
-        getByText,
-        getDragEl: makeGetDragEl('B'),
-        positions: 3
-      })
+      dragAndDrop(
+        screen.getByText('B'),
+        screen.getByText('Timestamp'),
+        DragEnterDirection.AUTO
+      )
 
-      await expectHeaders(['B', 'A', 'C'])
+      await expectHeaders(['A', 'B', 'C'])
     })
 
     it('should order the columns with the columnOrder from the data', async () => {
@@ -378,5 +363,42 @@ describe('Table', () => {
         type: MessageFromWebviewType.RESIZE_COLUMN
       })
     })
+
+    it('should move all the columns from a group from their current position to their new position', async () => {
+      renderExperimentsTable({ ...tableDataFixture })
+
+      let headers = await getHeaders()
+
+      expect(headers.indexOf('threshold')).toBeGreaterThan(
+        headers.indexOf('loss')
+      )
+      expect(headers.indexOf('test')).toBeGreaterThan(
+        headers.indexOf('accuracy')
+      )
+
+      dragAndDrop(
+        screen.getByText('process'),
+        screen.getByText('loss'),
+        DragEnterDirection.AUTO
+      )
+
+      headers = await getHeaders()
+
+      expect(headers.indexOf('loss')).toBeGreaterThan(
+        headers.indexOf('threshold')
+      )
+
+      dragAndDrop(
+        screen.getByText('summary.json'),
+        screen.getByText('test'),
+        DragEnterDirection.AUTO
+      )
+
+      headers = await getHeaders()
+
+      expect(headers.indexOf('accuracy')).toBeGreaterThan(
+        headers.indexOf('test')
+      )
+    })
   })
 })