From 9da212767a8e99d97d56e312189caed73a0b7658 Mon Sep 17 00:00:00 2001 From: Wolmir Nemitz Date: Mon, 9 May 2022 17:35:54 -0300 Subject: [PATCH 1/3] Allow reordering of columns by group --- webview/package.json | 3 - .../src/experiments/components/App.test.tsx | 23 +--- .../experiments/components/Experiments.tsx | 7 +- .../components/table/MergeHeaderGroups.tsx | 72 +++++------ .../components/table/Table.test.tsx | 59 +++------ .../components/table/TableHead.tsx | 56 +++++--- .../components/table/TableHeader.tsx | 121 ++++++++++-------- .../components/table/styles.module.scss | 3 + webview/src/experiments/util/columns.test.ts | 46 +++++++ webview/src/experiments/util/columns.ts | 40 ++++++ .../src/react-beautiful-dnd-test-utils.d.ts | 1 - .../components/dragDrop/DragDropContext.tsx | 43 ++++++- .../components/dragDrop/DragDropWorkbench.tsx | 106 +++++++++++++++ .../src/shared/components/dragDrop/util.ts | 3 +- webview/src/test/dragDrop.ts | 23 ++-- webview/src/test/sort.ts | 5 - yarn.lock | 93 +------------- 17 files changed, 426 insertions(+), 278 deletions(-) create mode 100644 webview/src/experiments/util/columns.test.ts delete mode 100644 webview/src/react-beautiful-dnd-test-utils.d.ts create mode 100644 webview/src/shared/components/dragDrop/DragDropWorkbench.tsx diff --git a/webview/package.json b/webview/package.json index 7f85fb28b7..95d422d1f2 100644 --- a/webview/package.json +++ b/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", diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx index 3809a0f1b1..5bcaf5df05 100644 --- a/webview/src/experiments/components/App.test.tsx +++ b/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,7 +32,6 @@ import { vsCodeApi } from '../../shared/api' import { commonColumnFields, expectHeaders, - makeGetDragEl, tableData as sortingTableDataFixture } from '../../test/sort' import { @@ -46,6 +39,8 @@ import { 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() - mockDndElSpacing(view) - mockGetComputedSpacing() + render() 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']) }) diff --git a/webview/src/experiments/components/Experiments.tsx b/webview/src/experiments/components/Experiments.tsx index e6d26f7d55..a5cfc8660b 100644 --- a/webview/src/experiments/components/Experiments.tsx +++ 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 No Experiments to Display. } - return + return ( + +
+ + ) } const Experiments: React.FC<{ diff --git a/webview/src/experiments/components/table/MergeHeaderGroups.tsx b/webview/src/experiments/components/table/MergeHeaderGroups.tsx index 66b9a500bb..fcbffd3869 100644 --- a/webview/src/experiments/components/table/MergeHeaderGroups.tsx +++ b/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 - columns: HeaderGroup[] - sorts: SortDefinition[] - orderedColumns: Column[] - } & Responders -> = ({ +export const MergedHeaderGroups: React.FC<{ + headerGroup: HeaderGroup + columns: HeaderGroup[] + sorts: SortDefinition[] + orderedColumns: Column[] + onDragUpdate: OnDragOver + onDragStart: OnDragStart + onDragEnd: OnDrop +}> = ({ headerGroup, sorts, columns, orderedColumns, - onDragStart, onDragUpdate, - onDragEnd + onDragEnd, + onDragStart }) => { return ( - - - {provided => ( -
- {headerGroup.headers.map((column: HeaderGroup, i) => ( - - ))} -
{provided.placeholder}
-
- )} - - + {headerGroup.headers.map((column: HeaderGroup) => ( +
+ +
+ ))} + ) } diff --git a/webview/src/experiments/components/table/Table.test.tsx b/webview/src/experiments/components/table/Table.test.tsx index 6fac74354c..47b13abab8 100644 --- a/webview/src/experiments/components/table/Table.test.tsx +++ b/webview/src/experiments/components/table/Table.test.tsx @@ -14,13 +14,6 @@ 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 { SortOrderLabel } from './SortPicker' import { Table } from './Table' import styles from './styles.module.scss' @@ -30,9 +23,10 @@ import * as ColumnOrder from '../../hooks/useColumnOrder' import { vsCodeApi } from '../../../shared/api' import { expectHeaders, - makeGetDragEl, 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 +121,7 @@ describe('Table', () => { const renderExperimentsTable = ( data: TableData = sortingTableDataFixture ) => { - const view = render() - - mockDndElSpacing(view) - - return view + return render() } beforeAll(() => { @@ -293,45 +283,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 () => { diff --git a/webview/src/experiments/components/table/TableHead.tsx b/webview/src/experiments/components/table/TableHead.tsx index 7847f51db0..1cdfaddd4d 100644 --- a/webview/src/experiments/components/table/TableHead.tsx +++ b/webview/src/experiments/components/table/TableHead.tsx @@ -2,12 +2,16 @@ import { SortDefinition } from 'dvc/src/experiments/model/sortBy' import { Experiment, Column } from 'dvc/src/experiments/webview/contract' import React, { useRef } from 'react' import { HeaderGroup, TableInstance } from 'react-table' -import { DragUpdate } from 'react-beautiful-dnd' import { MessageFromWebviewType } from 'dvc/src/webview/contract' import styles from './styles.module.scss' -import { MergedHeaderGroup } from './MergeHeaderGroups' +import { MergedHeaderGroups } from './MergeHeaderGroups' import { useColumnOrder } from '../../hooks/useColumnOrder' import { sendMessage } from '../../../shared/vscode' +import { leafColumnIds, reorderColumnIds } from '../../util/columns' +import { + OnDragOver, + OnDragStart +} from '../../../shared/components/dragDrop/DragDropWorkbench' interface TableHeadProps { instance: TableInstance @@ -32,26 +36,44 @@ export const TableHead: React.FC = ({ } const fullColumnOrder = useRef() + const draggingIds = useRef() - const onDragStart = () => { - fullColumnOrder.current = allColumns.map(column => column.id) + const onDragStart: OnDragStart = draggedId => { + const displacerHeader = allHeaders.find(header => header.id === draggedId) + if (displacerHeader) { + draggingIds.current = leafColumnIds(displacerHeader) + fullColumnOrder.current = allColumns.map(({ id }) => id) + } } - const onDragUpdate = (column: DragUpdate) => { - if (!column.destination) { - return - } - const { draggableId, destination } = column - if (destination.index > 1) { - const newColumnOrder = [...(fullColumnOrder.current as string[])] - const oldIndex = newColumnOrder.indexOf(draggableId) - newColumnOrder.splice(oldIndex, 1) - newColumnOrder.splice(destination.index, 0, draggableId) - setColumnOrder(newColumnOrder) - } + const findDisplacedHeader = ( + draggedOverId: string, + cb: (displacedHeader: HeaderGroup) => void + ) => { + const displacedHeader = allHeaders.find( + header => header.id === draggedOverId + ) + + displacedHeader && cb(displacedHeader) + } + + const onDragUpdate: OnDragOver = (_, draggedOverId: string) => { + const displacer = draggingIds.current + displacer && + findDisplacedHeader(draggedOverId, displacedHeader => { + const displaced = leafColumnIds(displacedHeader) + if (!displaced.some(id => displacer.includes(id))) { + fullColumnOrder.current && + setColumnOrder( + reorderColumnIds(fullColumnOrder.current, displacer, displaced) + ) + } + }) } const onDragEnd = () => { + draggingIds.current = undefined + fullColumnOrder.current = undefined sendMessage({ payload: columnOrder, type: MessageFromWebviewType.REORDER_COLUMNS @@ -62,7 +84,7 @@ export const TableHead: React.FC = ({
{headerGroups.map(headerGroup => ( // eslint-disable-next-line react/jsx-key - - provided: DraggableProvided - snapshot: DraggableStateSnapshot -}> = ({ provided, snapshot, column }) => { + onDragOver: OnDragOver + onDragStart: OnDragStart + onDrop: OnDrop +}> = ({ disabled, column, onDragOver, onDragStart, onDrop }) => { + const DropTarget = {column?.name} + return ( { e.stopPropagation() }} role={'columnheader'} tabIndex={0} > - {column.render('Header')} + + {column?.render('Header')} + ) } @@ -52,19 +62,21 @@ const TableHeaderCell: React.FC<{ columns: HeaderGroup[] orderedColumns: Column[] sortOrder: SortOrder - provided: DraggableProvided - snapshot: DraggableStateSnapshot menuDisabled: boolean menuContent: React.ReactNode + onDragOver: OnDragOver + onDragStart: OnDragStart + onDrop: OnDrop }> = ({ column, columns, orderedColumns, sortOrder, - provided, - snapshot, menuContent, - menuDisabled + menuDisabled, + onDragOver, + onDragStart, + onDrop }) => { const isPlaceholder = !!column.placeholderOf const canResize = column.canResize && !isPlaceholder @@ -94,6 +106,8 @@ const TableHeaderCell: React.FC<{ ) } } + const isDraggable = + !column.placeholderOf && !['id', 'timestamp'].includes(column.id) return ( @@ -104,8 +118,10 @@ const TableHeaderCell: React.FC<{ > {canResize && (
columns: HeaderGroup[] sorts: SortDefinition[] - index: number orderedColumns: Column[] + onDragOver: OnDragOver + onDragStart: OnDragStart + onDrop: OnDrop } export const TableHeader: React.FC = ({ column, columns, sorts, - index, - orderedColumns + orderedColumns, + onDragOver, + onDragStart, + onDrop }) => { const baseColumn = column.placeholderOf || column const sort = sorts.find(sort => sort.path === baseColumn.id) - const isDraggable = + const isSortable = !column.placeholderOf && !['id', 'timestamp'].includes(column.id) && !column.columns - const isSortable = isDraggable const sortOrder: SortOrder = (() => { const possibleOrders = { @@ -177,31 +196,23 @@ export const TableHeader: React.FC = ({ } return ( - - {(provided, snapshot) => ( - { - setColumnSort(order) - }} - /> - } + setSelectedOrder={order => { + setColumnSort(order) + }} /> - )} - + } + /> ) } diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss index 1a0a9b386a..b7b18b15ba 100644 --- a/webview/src/experiments/components/table/styles.module.scss +++ b/webview/src/experiments/components/table/styles.module.scss @@ -335,6 +335,7 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; white-space: nowrap; min-width: 0; position: relative; + height: 100%; } .td { font-size: 0.8rem; @@ -406,6 +407,8 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; } .draggingColumn { + border: 1px solid $fg-color; + font-size: 0.7rem; opacity: 0.7; } diff --git a/webview/src/experiments/util/columns.test.ts b/webview/src/experiments/util/columns.test.ts new file mode 100644 index 0000000000..fd23e5ff63 --- /dev/null +++ b/webview/src/experiments/util/columns.test.ts @@ -0,0 +1,46 @@ +import { reorderColumnIds } from './columns' + +describe('reorderColumnIds()', () => { + it('should reorder the column ids given a displacer array of ids and a displaced array of ids', () => { + const emptyColumnIds: string[] = [] + expect(reorderColumnIds(emptyColumnIds, [], [])).toStrictEqual([]) + + const twoColumnIds = ['id_1', 'id_2'] + expect(reorderColumnIds(twoColumnIds, [], [])).toStrictEqual(twoColumnIds) + expect(reorderColumnIds(twoColumnIds, ['id_1'], ['id_1'])).toStrictEqual( + twoColumnIds + ) + expect(reorderColumnIds(twoColumnIds, ['id_1'], ['id_2'])).toStrictEqual([ + 'id_2', + 'id_1' + ]) + expect(reorderColumnIds(twoColumnIds, ['id_2'], ['id_1'])).toStrictEqual([ + 'id_2', + 'id_1' + ]) + + const threeColumnIds = [...twoColumnIds, 'id_3'] + expect(reorderColumnIds(threeColumnIds, [], [])).toStrictEqual( + threeColumnIds + ) + expect(reorderColumnIds(threeColumnIds, ['id_2'], ['id_2'])).toStrictEqual( + threeColumnIds + ) + expect(reorderColumnIds(threeColumnIds, ['id_2'], ['id_3'])).toStrictEqual([ + 'id_1', + 'id_3', + 'id_2' + ]) + expect(reorderColumnIds(threeColumnIds, ['id_1'], ['id_3'])).toStrictEqual([ + 'id_2', + 'id_3', + 'id_1' + ]) + expect( + reorderColumnIds(threeColumnIds, ['id_1', 'id_2'], ['id_3']) + ).toStrictEqual(['id_3', 'id_1', 'id_2']) + expect( + reorderColumnIds(threeColumnIds, ['id_2', 'id_3'], ['id_1']) + ).toStrictEqual(['id_2', 'id_3', 'id_1']) + }) +}) diff --git a/webview/src/experiments/util/columns.ts b/webview/src/experiments/util/columns.ts index 5349ed69b7..c5bd003d14 100644 --- a/webview/src/experiments/util/columns.ts +++ b/webview/src/experiments/util/columns.ts @@ -74,3 +74,43 @@ export const countUpperLevels = ( } export const isFirstLevelHeader = (id: string) => id.split(':').length - 1 === 1 + +export const reorderColumnIds = ( + columnIds: string[], + displacer: string[], + displaced: string[] +) => { + if (columnIds.length === 0 || displacer[0] === displaced[0]) { + return columnIds + } + + 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) + ] + } + return [ + ...columnIds.slice(0, displacedIndex), + ...displacer, + ...displaced, + ...columnIds.slice(displacedIndex + displaced.length, displacerIndex), + ...columnIds.slice(displacerIndex + displacer.length) + ] +} + +export const leafColumnIds: ( + column: HeaderGroup +) => string[] = column => { + if (column.headers) { + return (column as HeaderGroup).headers.flatMap(leafColumnIds) + } + + return [column.id] +} diff --git a/webview/src/react-beautiful-dnd-test-utils.d.ts b/webview/src/react-beautiful-dnd-test-utils.d.ts deleted file mode 100644 index a6d129463a..0000000000 --- a/webview/src/react-beautiful-dnd-test-utils.d.ts +++ /dev/null @@ -1 +0,0 @@ -declare module 'react-beautiful-dnd-test-utils' diff --git a/webview/src/shared/components/dragDrop/DragDropContext.tsx b/webview/src/shared/components/dragDrop/DragDropContext.tsx index 68b888c7ef..0b35d3d8d7 100644 --- a/webview/src/shared/components/dragDrop/DragDropContext.tsx +++ b/webview/src/shared/components/dragDrop/DragDropContext.tsx @@ -4,28 +4,65 @@ export type DraggedInfo = | { itemIndex: string itemId: string - group: string + group?: string } | undefined +export interface DragDropGroupState { + draggedId?: string + draggedOverId?: string +} + +export type GroupStates = { + [group: string]: DragDropGroupState | undefined +} + export type DragDropContextValue = { draggedRef: DraggedInfo setDraggedRef: ((draggedRef: DraggedInfo) => void) | undefined + groupStates?: GroupStates + setGroupState?: (group: string, handlers: DragDropGroupState) => void + removeGroupState?: (group: string) => void } export const DragDropContext = createContext({ draggedRef: undefined, - setDraggedRef: undefined + groupStates: undefined, + removeGroupState: undefined, + setDraggedRef: undefined, + setGroupState: undefined }) export const DragDropProvider: React.FC = ({ children }) => { const [draggedRef, setDraggedRef] = useState(undefined) + const [groupStates, setGroupStates] = useState({}) + const changeDraggedRef = (d: DraggedInfo) => setDraggedRef(d) + const setGroupState = (group: string, handlers: DragDropGroupState) => { + setGroupStates({ + ...groupStates, + [group]: handlers + }) + } + + const removeGroupState = (group: string) => { + setGroupStates({ + ...groupStates, + [group]: undefined + }) + } + return ( {children} diff --git a/webview/src/shared/components/dragDrop/DragDropWorkbench.tsx b/webview/src/shared/components/dragDrop/DragDropWorkbench.tsx new file mode 100644 index 0000000000..7035739764 --- /dev/null +++ b/webview/src/shared/components/dragDrop/DragDropWorkbench.tsx @@ -0,0 +1,106 @@ +import React, { DragEvent, useContext } from 'react' +import { DragDropContext, DragDropContextValue } from './DragDropContext' + +export type OnDrop = (draggedId: string, draggedOverId: string) => void +export type OnDragStart = (draggedId: string) => void +export type OnDragOver = (draggedId: string, draggedOverId: string) => void + +export interface DraggableProps { + id: string + group: string + disabled: boolean + dropTarget: { + element: JSX.Element + wrapperTag: 'div' | 'th' + } + children: JSX.Element + onDrop?: OnDrop + onDragStart?: OnDragStart + onDragOver?: OnDragOver +} + +export const Draggable: React.FC = ({ + id, + group, + children, + disabled, + dropTarget, + onDrop, + onDragOver, + onDragStart +}) => { + const { groupStates, setGroupState } = + useContext(DragDropContext) + + const groupState = groupStates?.[group] || {} + const { draggedOverId, draggedId } = groupState + + const handleDragStart = (e: DragEvent) => { + const { id } = e.currentTarget + e.dataTransfer.effectAllowed = 'move' + e.dataTransfer.dropEffect = 'move' + setGroupState?.(group, { + ...groupState, + draggedId: id + }) + onDragStart?.(id) + } + + const handleOnDrop = () => { + !disabled && + draggedId && + draggedOverId && + onDrop?.(draggedId, draggedOverId) + } + + const handleDragEnter = (e: DragEvent) => { + const { id } = e.currentTarget + if (!disabled && draggedId && id !== draggedId && id !== draggedOverId) { + setGroupState?.(group, { + ...groupState, + draggedOverId: id + }) + onDragOver?.(draggedId, id) + } + } + + const handleDragOver = (e: DragEvent) => { + e.preventDefault() + } + + const handleDragEnd = () => { + setGroupState?.(group, { + ...groupState, + draggedId: undefined, + draggedOverId: undefined + }) + } + + const item = ( + + ) + if (id === draggedOverId) { + return ( + + {dropTarget.element} + + ) + } + + return item +} diff --git a/webview/src/shared/components/dragDrop/util.ts b/webview/src/shared/components/dragDrop/util.ts index 25f85d4249..7e5492da40 100644 --- a/webview/src/shared/components/dragDrop/util.ts +++ b/webview/src/shared/components/dragDrop/util.ts @@ -2,7 +2,8 @@ import { DragEvent } from 'react' export enum DragEnterDirection { RIGHT = 'RIGHT', - LEFT = 'LEFT' + LEFT = 'LEFT', + AUTO = 'AUTO' } export const getEventCurrentTargetDistances = (e: DragEvent) => { diff --git a/webview/src/test/dragDrop.ts b/webview/src/test/dragDrop.ts index 35aadfa110..4394796cc1 100644 --- a/webview/src/test/dragDrop.ts +++ b/webview/src/test/dragDrop.ts @@ -30,15 +30,20 @@ export const dragEnter = ( draggedOver.dispatchEvent(createBubbledEvent('dragenter')) - const clientX = - 100 + (direction === DragDropUtils.DragEnterDirection.LEFT ? 1 : 51) - const left = 100 - const right = left + 100 - const dragOverEvent = createBubbledEvent('dragover', { clientX }) - jest - .spyOn(DragDropUtils, 'getEventCurrentTargetDistances') - .mockImplementationOnce(() => ({ left, right })) - draggedOver.dispatchEvent(dragOverEvent) + if (direction !== DragDropUtils.DragEnterDirection.AUTO) { + const clientX = + 100 + (direction === DragDropUtils.DragEnterDirection.LEFT ? 1 : 51) + const left = 100 + const right = left + 100 + const dragOverEvent = createBubbledEvent('dragover', { clientX }) + jest + .spyOn(DragDropUtils, 'getEventCurrentTargetDistances') + .mockImplementationOnce(() => ({ left, right })) + draggedOver.dispatchEvent(dragOverEvent) + } else { + draggedOver.dispatchEvent(createBubbledEvent('dragover')) + } + jest.useRealTimers() } diff --git a/webview/src/test/sort.ts b/webview/src/test/sort.ts index 90aeb8570d..457993f574 100644 --- a/webview/src/test/sort.ts +++ b/webview/src/test/sort.ts @@ -1,6 +1,5 @@ import { screen } from '@testing-library/react' import { ColumnType, TableData } from 'dvc/src/experiments/webview/contract' -import { DND_DRAGGABLE_DATA_ATTR } from 'react-beautiful-dnd-test-utils' export const defaultColumns = ['Experiment', 'Timestamp'] @@ -50,10 +49,6 @@ export const tableData: TableData = { sorts: [] } -export const makeGetDragEl = (text: string) => () => - // eslint-disable-next-line testing-library/no-node-access - screen.getByText(text).closest(DND_DRAGGABLE_DATA_ATTR) - export const getHeaders = async () => { const renderedHeader = await screen.findAllByTestId('rendered-header') return renderedHeader.map(header => header.textContent) diff --git a/yarn.lock b/yarn.lock index 445926c3aa..66d3e9695f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1079,7 +1079,7 @@ core-js-pure "^3.20.2" regenerator-runtime "^0.13.4" -"@babel/runtime@^7.0.0", "@babel/runtime@^7.10.2", "@babel/runtime@^7.12.5", "@babel/runtime@^7.14.8", "@babel/runtime@^7.15.4", "@babel/runtime@^7.16.3", "@babel/runtime@^7.16.7", "@babel/runtime@^7.3.1", "@babel/runtime@^7.5.0", "@babel/runtime@^7.5.5", "@babel/runtime@^7.7.2", "@babel/runtime@^7.7.6", "@babel/runtime@^7.8.4", "@babel/runtime@^7.9.2": +"@babel/runtime@^7.0.0", "@babel/runtime@^7.10.2", "@babel/runtime@^7.12.5", "@babel/runtime@^7.14.8", "@babel/runtime@^7.16.3", "@babel/runtime@^7.16.7", "@babel/runtime@^7.3.1", "@babel/runtime@^7.5.0", "@babel/runtime@^7.5.5", "@babel/runtime@^7.7.2", "@babel/runtime@^7.7.6", "@babel/runtime@^7.8.4", "@babel/runtime@^7.9.2": version "7.17.2" resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.17.2.tgz#66f68591605e59da47523c631416b18508779941" integrity sha512-hzeyJyMA1YGdJTuWU0e/j4wKXrU4OMFvY2MSlaI9B7VQb0r5cxTE3EAIS2Q7Tn2RIcDkRvTA/v2JsAEhxe99uw== @@ -3036,14 +3036,6 @@ dependencies: "@types/unist" "*" -"@types/hoist-non-react-statics@^3.3.0": - version "3.3.1" - resolved "https://registry.yarnpkg.com/@types/hoist-non-react-statics/-/hoist-non-react-statics-3.3.1.tgz#1124aafe5118cb591977aeb1ceaaed1070eb039f" - integrity sha512-iMIqiko6ooLrTh1joXodJK5X9xeEALT1kM5G3ZLhD3hszxBdIEd5C75U834D9mLcINgD4OyZf5uQXjkuYydWvA== - dependencies: - "@types/react" "*" - hoist-non-react-statics "^3.3.0" - "@types/html-minifier-terser@^5.0.0": version "5.1.2" resolved "https://registry.yarnpkg.com/@types/html-minifier-terser/-/html-minifier-terser-5.1.2.tgz#693b316ad323ea97eed6b38ed1a3cc02b1672b57" @@ -3271,13 +3263,6 @@ resolved "https://registry.yarnpkg.com/@types/range-parser/-/range-parser-1.2.4.tgz#cd667bcfdd025213aafb7ca5915a932590acdcdc" integrity sha512-EEhsLsD6UsDM1yFhAvy0Cjr6VwmpMWqFBCb9w07wVugF7w9nfajxLuVmngTIpgS6svCnm6Vaw+MZhoDCKnOfsw== -"@types/react-beautiful-dnd@^13.1.2": - version "13.1.2" - resolved "https://registry.yarnpkg.com/@types/react-beautiful-dnd/-/react-beautiful-dnd-13.1.2.tgz#510405abb09f493afdfd898bf83995dc6385c130" - integrity sha512-+OvPkB8CdE/bGdXKyIhc/Lm2U7UAYCCJgsqmopFmh9gbAudmslkI8eOrPDjg4JhwSE6wytz4a3/wRjKtovHVJg== - dependencies: - "@types/react" "*" - "@types/react-dom@*", "@types/react-dom@^17.0.11": version "17.0.11" resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-17.0.11.tgz#e1eadc3c5e86bdb5f7684e00274ae228e7bcc466" @@ -3292,16 +3277,6 @@ dependencies: "@types/react" "*" -"@types/react-redux@^7.1.20": - version "7.1.22" - resolved "https://registry.yarnpkg.com/@types/react-redux/-/react-redux-7.1.22.tgz#0eab76a37ef477cc4b53665aeaf29cb60631b72a" - integrity sha512-GxIA1kM7ClU73I6wg9IRTVwSO9GS+SAKZKe0Enj+82HMU6aoESFU2HNAdNi3+J53IaOHPiUfT3kSG4L828joDQ== - dependencies: - "@types/hoist-non-react-statics" "^3.3.0" - "@types/react" "*" - hoist-non-react-statics "^3.3.0" - redux "^4.0.0" - "@types/react-syntax-highlighter@11.0.5": version "11.0.5" resolved "https://registry.yarnpkg.com/@types/react-syntax-highlighter/-/react-syntax-highlighter-11.0.5.tgz#0d546261b4021e1f9d85b50401c0a42acb106087" @@ -5842,13 +5817,6 @@ crypto-browserify@^3.11.0: randombytes "^2.0.0" randomfill "^1.0.3" -css-box-model@^1.2.0: - version "1.2.1" - resolved "https://registry.yarnpkg.com/css-box-model/-/css-box-model-1.2.1.tgz#59951d3b81fd6b2074a62d49444415b0d2b4d7c1" - integrity sha512-a7Vr4Q/kd/aw96bnJG332W9V9LkJO69JRcaCYDUqjp6/z0w6VcZjgAcTbgFxEPfBgdnAwlh3iwu+hLopa+flJw== - dependencies: - tiny-invariant "^1.0.6" - css-loader@^3.6.0: version "3.6.0" resolved "https://registry.yarnpkg.com/css-loader/-/css-loader-3.6.0.tgz#2e4b2c7e6e2d27f8c8f28f61bffcd2e6c91ef645" @@ -8216,7 +8184,7 @@ hmac-drbg@^1.0.1: minimalistic-assert "^1.0.0" minimalistic-crypto-utils "^1.0.1" -hoist-non-react-statics@^3.3.0, hoist-non-react-statics@^3.3.2: +hoist-non-react-statics@^3.3.0: version "3.3.2" resolved "https://registry.yarnpkg.com/hoist-non-react-statics/-/hoist-non-react-statics-3.3.2.tgz#ece0acaf71d62c2969c2ec59feff42a4b1a85b45" integrity sha512-/gGivxi8JPKWNm/W0jSmzcMPpfpPLc3dY/6GxhX2hQ9iGj3aDfklV4ET7NjKpSinLpJ5vafa9iiGIEZg10SfBw== @@ -10223,11 +10191,6 @@ memfs@^3.1.2, memfs@^3.2.2, memfs@^3.4.1: dependencies: fs-monkey "1.0.3" -memoize-one@^5.1.1: - version "5.2.1" - resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-5.2.1.tgz#8337aa3c4335581839ec01c3d594090cebe8f00e" - integrity sha512-zYiwtZUcYyXKo/np96AGZAckk+FWWsUdJ3cHGGmld7+AhvcWmQyGCYUh1hc4Q/pkOhb65dQR/pqCyK0cOaHz4Q== - memoizerific@^1.11.3: version "1.11.3" resolved "https://registry.yarnpkg.com/memoizerific/-/memoizerific-1.11.3.tgz#7c87a4646444c32d75438570905f2dbd1b1a805a" @@ -11890,11 +11853,6 @@ queue-microtask@^1.2.2: resolved "https://registry.yarnpkg.com/queue-microtask/-/queue-microtask-1.2.3.tgz#4929228bbc724dfac43e0efb058caf7b6cfb6243" integrity sha512-NuaNSa6flKT5JaSYQzJok04JzTL1CA6aGhv5rfLW3PgqA+M2ChpZQnAC8h8i4ZFkBS8X5RqkDBHA7r4hej3K9A== -raf-schd@^4.0.2: - version "4.0.3" - resolved "https://registry.yarnpkg.com/raf-schd/-/raf-schd-4.0.3.tgz#5d6c34ef46f8b2a0e880a8fcdb743efc5bfdbc1a" - integrity sha512-tQkJl2GRWh83ui2DiPTJz9wEiMN20syf+5oKfB03yYP7ioZcJwsIK8FjrtLwH1m7C7e+Tt2yYBlrOpdT+dyeIQ== - ramda@^0.21.0: version "0.21.0" resolved "https://registry.yarnpkg.com/ramda/-/ramda-0.21.0.tgz#a001abedb3ff61077d4ff1d577d44de77e8d0a35" @@ -11948,24 +11906,6 @@ rc@^1.2.7: minimist "^1.2.0" strip-json-comments "~2.0.1" -react-beautiful-dnd-test-utils@^3.2.2: - version "3.2.2" - resolved "https://registry.yarnpkg.com/react-beautiful-dnd-test-utils/-/react-beautiful-dnd-test-utils-3.2.2.tgz#afc7d718a63f47c5c18cc74038a4841c1a476992" - integrity sha512-/Nyn3zww8UaJbNNP6CTikkFV29rrqmxjbbgN3XknfKD1Cq2IIlEyO/Jn8X5Zj06gE3lac8H9NXC7lJeadNOrzA== - -react-beautiful-dnd@^13.1.0: - version "13.1.0" - resolved "https://registry.yarnpkg.com/react-beautiful-dnd/-/react-beautiful-dnd-13.1.0.tgz#ec97c81093593526454b0de69852ae433783844d" - integrity sha512-aGvblPZTJowOWUNiwd6tNfEpgkX5OxmpqxHKNW/4VmvZTNTbeiq7bA3bn5T+QSF2uibXB0D1DmJsb1aC/+3cUA== - dependencies: - "@babel/runtime" "^7.9.2" - css-box-model "^1.2.0" - memoize-one "^5.1.1" - raf-schd "^4.0.2" - react-redux "^7.2.0" - redux "^4.0.4" - use-memo-one "^1.1.1" - react-colorful@^5.1.2: version "5.5.1" resolved "https://registry.yarnpkg.com/react-colorful/-/react-colorful-5.5.1.tgz#29d9c4e496f2ca784dd2bb5053a3a4340cfaf784" @@ -12075,18 +12015,6 @@ react-popper@^2.2.4: react-fast-compare "^3.0.1" warning "^4.0.2" -react-redux@^7.2.0: - version "7.2.6" - resolved "https://registry.yarnpkg.com/react-redux/-/react-redux-7.2.6.tgz#49633a24fe552b5f9caf58feb8a138936ddfe9aa" - integrity sha512-10RPdsz0UUrRL1NZE0ejTkucnclYSgXp5q+tB5SWx2qeG2ZJQJyymgAhwKy73yiL/13btfB6fPr+rgbMAaZIAQ== - dependencies: - "@babel/runtime" "^7.15.4" - "@types/react-redux" "^7.1.20" - hoist-non-react-statics "^3.3.2" - loose-envify "^1.4.0" - prop-types "^15.7.2" - react-is "^17.0.2" - react-refresh@^0.11.0: version "0.11.0" resolved "https://registry.yarnpkg.com/react-refresh/-/react-refresh-0.11.0.tgz#77198b944733f0f1f1a90e791de4541f9f074046" @@ -12266,13 +12194,6 @@ redent@^3.0.0: indent-string "^4.0.0" strip-indent "^3.0.0" -redux@^4.0.0, redux@^4.0.4: - version "4.1.2" - resolved "https://registry.yarnpkg.com/redux/-/redux-4.1.2.tgz#140f35426d99bb4729af760afcf79eaaac407104" - integrity sha512-SH8PglcebESbd/shgf6mii6EIoRM0zrQyjcuQ+ojmfxjTtE0z9Y8pa62iA/OJ58qjP6j27uyW4kUF4jl/jd6sw== - dependencies: - "@babel/runtime" "^7.9.2" - refractor@^3.1.0: version "3.5.0" resolved "https://registry.yarnpkg.com/refractor/-/refractor-3.5.0.tgz#334586f352dda4beaf354099b48c2d18e0819aec" @@ -13777,11 +13698,6 @@ timers-browserify@^2.0.4: dependencies: setimmediate "^1.0.4" -tiny-invariant@^1.0.6: - version "1.2.0" - resolved "https://registry.yarnpkg.com/tiny-invariant/-/tiny-invariant-1.2.0.tgz#a1141f86b672a9148c72e978a19a73b9b94a15a9" - integrity sha512-1Uhn/aqw5C6RI4KejVeTg6mIS7IqxnLJ8Mv2tV5rTc0qWobay7pDUz6Wi392Cnc8ak1H0F2cjoRzb2/AW4+Fvg== - tippy.js@^6.3.1: version "6.3.7" resolved "https://registry.yarnpkg.com/tippy.js/-/tippy.js-6.3.7.tgz#8ccfb651d642010ed9a32ff29b0e9e19c5b8c61c" @@ -14404,11 +14320,6 @@ use-latest@^1.0.0: dependencies: use-isomorphic-layout-effect "^1.0.0" -use-memo-one@^1.1.1: - version "1.1.2" - resolved "https://registry.yarnpkg.com/use-memo-one/-/use-memo-one-1.1.2.tgz#0c8203a329f76e040047a35a1197defe342fab20" - integrity sha512-u2qFKtxLsia/r8qG0ZKkbytbztzRb317XCkT7yP8wxL0tZ/CzK2G+WWie5vWvpyeP7+YoPIwbJoIHJ4Ba4k0oQ== - use@^3.1.0: version "3.1.1" resolved "https://registry.yarnpkg.com/use/-/use-3.1.1.tgz#d50c8cac79a19fbc20f2911f56eb973f4e10070f" From 06ff1182b1cf514876f76b4be819b243cffdff50 Mon Sep 17 00:00:00 2001 From: Wolmir Nemitz Date: Fri, 20 May 2022 12:01:25 -0300 Subject: [PATCH 2/3] Remove duplication and add group tests --- .../components/table/Table.test.tsx | 39 +++++++++++++++++ .../components/dragDrop/DragDropContainer.tsx | 43 ++++++++++++------- .../components/dragDrop/DragDropWorkbench.tsx | 30 +++++-------- 3 files changed, 76 insertions(+), 36 deletions(-) diff --git a/webview/src/experiments/components/table/Table.test.tsx b/webview/src/experiments/components/table/Table.test.tsx index 47b13abab8..9c0a5a65a5 100644 --- a/webview/src/experiments/components/table/Table.test.tsx +++ b/webview/src/experiments/components/table/Table.test.tsx @@ -14,6 +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 tableDataFixture from 'dvc/src/test/fixtures/expShow/tableData' import { SortOrderLabel } from './SortPicker' import { Table } from './Table' import styles from './styles.module.scss' @@ -23,6 +24,7 @@ import * as ColumnOrder from '../../hooks/useColumnOrder' import { vsCodeApi } from '../../../shared/api' import { expectHeaders, + getHeaders, tableData as sortingTableDataFixture } from '../../../test/sort' import { dragAndDrop } from '../../../test/dragDrop' @@ -361,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') + ) + }) }) }) diff --git a/webview/src/shared/components/dragDrop/DragDropContainer.tsx b/webview/src/shared/components/dragDrop/DragDropContainer.tsx index 763121d53e..b14e6c879e 100644 --- a/webview/src/shared/components/dragDrop/DragDropContainer.tsx +++ b/webview/src/shared/components/dragDrop/DragDropContainer.tsx @@ -3,7 +3,8 @@ import React, { useEffect, useState, useRef, - useContext + useContext, + DragEventHandler } from 'react' import { DragEnterDirection, getDragEnterDirection } from './util' import { DragDropContext, DragDropContextValue } from './DragDropContext' @@ -31,6 +32,28 @@ export type OnDrop = ( groupId: string, position: number ) => void + +export type DropTargetInfo = { + element: JSX.Element + wrapperTag: 'div' | 'th' +} + +export const makeTarget = ( + dropTarget: DropTargetInfo, + handleDragOver: DragEventHandler, + handleOnDrop: DragEventHandler, + id: string +) => ( + + {dropTarget.element} + +) interface DragDropContainerProps { order: string[] setOrder: (order: string[]) => void @@ -38,10 +61,7 @@ interface DragDropContainerProps { items: JSX.Element[] // Every item must have a id prop for drag and drop to work group: string onDrop?: OnDrop - dropTarget: { - element: JSX.Element - wrapperTag: 'div' | 'th' - } + dropTarget: DropTargetInfo wrapperComponent?: { component: React.FC props: { @@ -193,17 +213,8 @@ export const DragDropContainer: React.FC = ({ const item = id && buildItem(id, draggable) if (id === draggedOverId) { - const target = ( - - {dropTarget.element} - - ) + const target = makeTarget(dropTarget, handleDragOver, handleOnDrop, id) + return direction === DragEnterDirection.RIGHT ? [item, target] : [target, item] diff --git a/webview/src/shared/components/dragDrop/DragDropWorkbench.tsx b/webview/src/shared/components/dragDrop/DragDropWorkbench.tsx index 7035739764..cfaa3d4d7f 100644 --- a/webview/src/shared/components/dragDrop/DragDropWorkbench.tsx +++ b/webview/src/shared/components/dragDrop/DragDropWorkbench.tsx @@ -1,4 +1,5 @@ import React, { DragEvent, useContext } from 'react' +import { DropTargetInfo, makeTarget } from './DragDropContainer' import { DragDropContext, DragDropContextValue } from './DragDropContext' export type OnDrop = (draggedId: string, draggedOverId: string) => void @@ -9,10 +10,7 @@ export interface DraggableProps { id: string group: string disabled: boolean - dropTarget: { - element: JSX.Element - wrapperTag: 'div' | 'th' - } + dropTarget: DropTargetInfo children: JSX.Element onDrop?: OnDrop onDragStart?: OnDragStart @@ -55,13 +53,15 @@ export const Draggable: React.FC = ({ const handleDragEnter = (e: DragEvent) => { const { id } = e.currentTarget - if (!disabled && draggedId && id !== draggedId && id !== draggedOverId) { - setGroupState?.(group, { + !disabled && + draggedId && + id !== draggedId && + id !== draggedOverId && + (setGroupState?.(group, { ...groupState, draggedOverId: id - }) - onDragOver?.(draggedId, id) - } + }) || + onDragOver?.(draggedId, id)) } const handleDragOver = (e: DragEvent) => { @@ -89,17 +89,7 @@ export const Draggable: React.FC = ({ /> ) if (id === draggedOverId) { - return ( - - {dropTarget.element} - - ) + return makeTarget(dropTarget, handleDragOver, handleOnDrop, id) } return item From 58129a9f1b3389ace47dff7edf369d26c928e676 Mon Sep 17 00:00:00 2001 From: Wolmir Nemitz Date: Fri, 20 May 2022 15:05:07 -0300 Subject: [PATCH 3/3] Fix column drag not working in VSCode host --- webview/src/experiments/components/table/styles.module.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss index b7b18b15ba..901f59934f 100644 --- a/webview/src/experiments/components/table/styles.module.scss +++ b/webview/src/experiments/components/table/styles.module.scss @@ -249,6 +249,11 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; } .cellContents { @extend %truncateLeftChild; + display: block; + + span[draggable='true'] { + display: block; + } } &:last-child,