From 816066de51af69fe0f5788c7355c257d9dab9e99 Mon Sep 17 00:00:00 2001 From: Stephanie Roy Date: Mon, 1 Aug 2022 13:36:34 -0400 Subject: [PATCH 1/3] Simplify drag and drop integrations --- .../components/table/MergeHeaderGroups.tsx | 14 ++--- .../components/table/TableHead.tsx | 17 +++--- .../components/table/TableHeader.tsx | 46 ++++++++-------- .../components/dragDrop/DragDropContainer.tsx | 37 ++++--------- .../{DragDropWorkbench.tsx => Draggable.tsx} | 52 +++++++++---------- .../shared/components/dragDrop/DropTarget.tsx | 27 ++++++++++ 6 files changed, 99 insertions(+), 94 deletions(-) rename webview/src/shared/components/dragDrop/{DragDropWorkbench.tsx => Draggable.tsx} (71%) create mode 100644 webview/src/shared/components/dragDrop/DropTarget.tsx diff --git a/webview/src/experiments/components/table/MergeHeaderGroups.tsx b/webview/src/experiments/components/table/MergeHeaderGroups.tsx index 947779c645..c0803d63b6 100644 --- a/webview/src/experiments/components/table/MergeHeaderGroups.tsx +++ b/webview/src/experiments/components/table/MergeHeaderGroups.tsx @@ -4,19 +4,15 @@ import { Experiment, Column } from 'dvc/src/experiments/webview/contract' import { HeaderGroup } from 'react-table' import { TableHeader } from './TableHeader' import styles from './styles.module.scss' -import { - OnDragOver, - OnDragStart, - OnDrop -} from '../../../shared/components/dragDrop/DragDropWorkbench' +import { DragFunction } from '../../../shared/components/dragDrop/Draggable' export const MergedHeaderGroups: React.FC<{ headerGroup: HeaderGroup columns: HeaderGroup[] orderedColumns: Column[] - onDragUpdate: OnDragOver - onDragStart: OnDragStart - onDragEnd: OnDrop + onDragUpdate: DragFunction + onDragStart: DragFunction + onDragEnd: DragFunction firstExpColumnCellId: string setExpColumnNeedsShadow: (needsShadow: boolean) => void root: HTMLElement | null @@ -45,7 +41,7 @@ export const MergedHeaderGroups: React.FC<{ orderedColumns={orderedColumns} column={column} columns={columns} - onDragOver={onDragUpdate} + onDragEnter={onDragUpdate} onDragStart={onDragStart} onDrop={onDragEnd} root={root} diff --git a/webview/src/experiments/components/table/TableHead.tsx b/webview/src/experiments/components/table/TableHead.tsx index 36b947e871..0926fb3c1d 100644 --- a/webview/src/experiments/components/table/TableHead.tsx +++ b/webview/src/experiments/components/table/TableHead.tsx @@ -1,6 +1,6 @@ import { Experiment } from 'dvc/src/experiments/webview/contract' import cx from 'classnames' -import React, { useRef } from 'react' +import React, { DragEvent, useRef } from 'react' import { useSelector } from 'react-redux' import { HeaderGroup, TableInstance } from 'react-table' import { MessageFromWebviewType } from 'dvc/src/webview/contract' @@ -12,10 +12,7 @@ import { useColumnOrder } from '../../hooks/useColumnOrder' import { ExperimentsState } from '../../store' import { sendMessage } from '../../../shared/vscode' import { leafColumnIds, reorderColumnIds } from '../../util/columns' -import { - OnDragOver, - OnDragStart -} from '../../../shared/components/dragDrop/DragDropWorkbench' +import { DragFunction } from '../../../shared/components/dragDrop/Draggable' import { getSelectedForPlotsCount } from '../../util/rows' interface TableHeadProps { instance: TableInstance @@ -46,8 +43,10 @@ export const TableHead = ({ const fullColumnOrder = useRef() const draggingIds = useRef() - const onDragStart: OnDragStart = draggedId => { - const displacerHeader = allHeaders.find(header => header.id === draggedId) + const onDragStart: DragFunction = ({ currentTarget }) => { + const displacerHeader = allHeaders.find( + header => header.id === currentTarget.id + ) if (displacerHeader) { draggingIds.current = leafColumnIds(displacerHeader) fullColumnOrder.current = allColumns.map(({ id }) => id) @@ -65,10 +64,10 @@ export const TableHead = ({ displacedHeader && cb(displacedHeader) } - const onDragUpdate: OnDragOver = (_, draggedOverId: string) => { + const onDragUpdate = (e: DragEvent) => { const displacer = draggingIds.current displacer && - findDisplacedHeader(draggedOverId, displacedHeader => { + findDisplacedHeader(e.currentTarget.id, displacedHeader => { const displaced = leafColumnIds(displacedHeader) if (!displaced.some(id => displacer.includes(id))) { fullColumnOrder.current && diff --git a/webview/src/experiments/components/table/TableHeader.tsx b/webview/src/experiments/components/table/TableHeader.tsx index 0f29baecac..764b6d858e 100644 --- a/webview/src/experiments/components/table/TableHeader.tsx +++ b/webview/src/experiments/components/table/TableHeader.tsx @@ -16,10 +16,8 @@ import { ContextMenu } from '../../../shared/components/contextMenu/ContextMenu' import { ExperimentsState } from '../../store' import { Draggable, - OnDragOver, - OnDragStart, - OnDrop -} from '../../../shared/components/dragDrop/DragDropWorkbench' + DragFunction +} from '../../../shared/components/dragDrop/Draggable' import { MessagesMenu } from '../../../shared/components/messagesMenu/MessagesMenu' import { MessagesMenuOptionProps } from '../../../shared/components/messagesMenu/MessagesMenuOption' import { IconMenu } from '../../../shared/components/iconMenu/IconMenu' @@ -40,10 +38,10 @@ const possibleOrders = { export const ColumnDragHandle: React.FC<{ disabled: boolean column: HeaderGroup - onDragOver: OnDragOver - onDragStart: OnDragStart - onDrop: OnDrop -}> = ({ disabled, column, onDragOver, onDragStart, onDrop }) => { + onDragEnter: DragFunction + onDragStart: DragFunction + onDrop: DragFunction +}> = ({ disabled, column, onDragEnter, onDragStart, onDrop }) => { const DropTarget = {column?.name} return ( @@ -61,7 +59,7 @@ export const ColumnDragHandle: React.FC<{ disabled={disabled} group={'experiment-table'} dropTarget={DropTarget} - onDragOver={onDragOver} + onDragEnter={onDragEnter} onDragStart={onDragStart} onDrop={onDrop} > @@ -153,9 +151,9 @@ const TableHeaderCellContents: React.FC<{ hasFilter: boolean isDraggable: boolean menuSuppressed: boolean - onDragOver: OnDragOver - onDragStart: OnDragStart - onDrop: OnDrop + onDragEnter: DragFunction + onDragStart: DragFunction + onDrop: DragFunction canResize: boolean setMenuSuppressed: (menuSuppressed: boolean) => void resizerHeight: string @@ -166,7 +164,7 @@ const TableHeaderCellContents: React.FC<{ hasFilter, isDraggable, menuSuppressed, - onDragOver, + onDragEnter, onDragStart, onDrop, canResize, @@ -181,7 +179,7 @@ const TableHeaderCellContents: React.FC<{ @@ -207,9 +205,9 @@ const TableHeaderCell: React.FC<{ sortEnabled: boolean menuDisabled?: boolean menuContent?: React.ReactNode - onDragOver: OnDragOver - onDragStart: OnDragStart - onDrop: OnDrop + onDragEnter: DragFunction + onDragStart: DragFunction + onDrop: DragFunction firstExpColumnCellId: string setExpColumnNeedsShadow: (needsShadow: boolean) => void root: HTMLElement | null @@ -222,7 +220,7 @@ const TableHeaderCell: React.FC<{ sortEnabled, menuContent, menuDisabled, - onDragOver, + onDragEnter, onDragStart, onDrop, root, @@ -250,7 +248,7 @@ const TableHeaderCell: React.FC<{ hasFilter={hasFilter} isDraggable={isDraggable} menuSuppressed={menuSuppressed} - onDragOver={onDragOver} + onDragEnter={onDragEnter} onDragStart={onDragStart} onDrop={onDrop} canResize={canResize} @@ -293,9 +291,9 @@ interface TableHeaderProps { column: HeaderGroup columns: HeaderGroup[] orderedColumns: Column[] - onDragOver: OnDragOver - onDragStart: OnDragStart - onDrop: OnDrop + onDragEnter: DragFunction + onDragStart: DragFunction + onDrop: DragFunction firstExpColumnCellId: string setExpColumnNeedsShadow: (needsShadow: boolean) => void root: HTMLElement | null @@ -305,7 +303,7 @@ export const TableHeader: React.FC = ({ column, columns, orderedColumns, - onDragOver, + onDragEnter, onDragStart, onDrop, root, @@ -359,7 +357,7 @@ export const TableHeader: React.FC = ({ sortOrder={sortOrder} sortEnabled={isSortable} hasFilter={hasFilter} - onDragOver={onDragOver} + onDragEnter={onDragEnter} onDragStart={onDragStart} onDrop={onDrop} menuDisabled={!isSortable && column.group !== ColumnType.PARAMS} diff --git a/webview/src/shared/components/dragDrop/DragDropContainer.tsx b/webview/src/shared/components/dragDrop/DragDropContainer.tsx index 42f2c127f7..472df5c8c9 100644 --- a/webview/src/shared/components/dragDrop/DragDropContainer.tsx +++ b/webview/src/shared/components/dragDrop/DragDropContainer.tsx @@ -4,13 +4,13 @@ import React, { useEffect, useState, useRef, - DragEventHandler, CSSProperties } from 'react' import { useDispatch, useSelector } from 'react-redux' import { DragEnterDirection, getDragEnterDirection } from './util' import { changeRef } from './dragDropSlice' import styles from './styles.module.scss' +import { DropTarget } from './DropTarget' import { getIDIndex, getIDWithoutIndex } from '../../../util/ids' import { Any } from '../../../util/objects' import { PlotsState } from '../../../plots/store' @@ -38,25 +38,6 @@ export type OnDrop = ( groupId: string, position: number ) => void - -export const makeTarget = ( - dropTarget: JSX.Element, - handleDragOver: DragEventHandler, - handleOnDrop: DragEventHandler, - id: string, - className?: string -) => ( -
- {dropTarget} -
-) interface DragDropContainerProps { order: string[] setOrder: (order: string[]) => void @@ -261,12 +242,16 @@ export const DragDropContainer: React.FC = ({ [styles.dropTargetWhenShowingOnDragRight]: isEnteringRight }) : undefined - const target = makeTarget( - dropTarget, - handleDragOver, - handleOnDrop, - id, - targetClassName + const target = ( + + {dropTarget} + ) const itemWithTag = shouldShowOnDrag ? (
diff --git a/webview/src/shared/components/dragDrop/DragDropWorkbench.tsx b/webview/src/shared/components/dragDrop/Draggable.tsx similarity index 71% rename from webview/src/shared/components/dragDrop/DragDropWorkbench.tsx rename to webview/src/shared/components/dragDrop/Draggable.tsx index 5a7b976e50..900774f642 100644 --- a/webview/src/shared/components/dragDrop/DragDropWorkbench.tsx +++ b/webview/src/shared/components/dragDrop/Draggable.tsx @@ -1,22 +1,20 @@ import React, { DragEvent } from 'react' import { useDispatch, useSelector } from 'react-redux' -import { makeTarget } from './DragDropContainer' import { setGroup } from './dragDropSlice' +import { DropTarget } from './DropTarget' import { ExperimentsState } from '../../../experiments/store' -export type OnDrop = (draggedId: string, draggedOverId: string) => void -export type OnDragStart = (draggedId: string) => void -export type OnDragOver = (draggedId: string, draggedOverId: string) => void +export type DragFunction = (e: DragEvent) => void export interface DraggableProps { id: string group: string disabled: boolean - dropTarget: JSX.Element children: JSX.Element - onDrop?: OnDrop - onDragStart?: OnDragStart - onDragOver?: OnDragOver + dropTarget: JSX.Element + onDrop: DragFunction + onDragStart: DragFunction + onDragEnter: DragFunction } export const Draggable: React.FC = ({ @@ -26,9 +24,8 @@ export const Draggable: React.FC = ({ disabled, dropTarget, onDrop, - onDragOver, + onDragEnter, onDragStart - // eslint-disable-next-line sonarjs/cognitive-complexity }) => { const groupStates = useSelector( (state: ExperimentsState) => state.dragAndDrop.groups @@ -54,15 +51,10 @@ export const Draggable: React.FC = ({ const { id } = e.currentTarget e.dataTransfer.effectAllowed = 'move' e.dataTransfer.dropEffect = 'move' + e.dataTransfer.setData('itemId', id) + e.dataTransfer.setData('group', group) modifyGroup(id) - onDragStart?.(id) - } - - const handleOnDrop = () => { - !disabled && - draggedId && - draggedOverId && - onDrop?.(draggedId, draggedOverId) + onDragStart(e) } const handleDragEnter = (e: DragEvent) => { @@ -71,7 +63,7 @@ export const Draggable: React.FC = ({ if (id !== draggedId && id !== draggedOverId) { modifyGroup(id) - onDragOver?.(draggedId, id) + onDragEnter(e) } } } @@ -93,7 +85,20 @@ export const Draggable: React.FC = ({ ) } - const item = ( + if (dropTarget && id === draggedOverId) { + return ( + + {dropTarget} + + ) + } + + return ( = ({ onDragEnd={handleDragEnd} onDragOver={handleDragOver} onDragEnter={handleDragEnter} - onDrop={handleOnDrop} + onDrop={onDrop} draggable={!disabled} /> ) - if (id === draggedOverId) { - return makeTarget(dropTarget, handleDragOver, handleOnDrop, id) - } - - return item } diff --git a/webview/src/shared/components/dragDrop/DropTarget.tsx b/webview/src/shared/components/dragDrop/DropTarget.tsx new file mode 100644 index 0000000000..4c2158e152 --- /dev/null +++ b/webview/src/shared/components/dragDrop/DropTarget.tsx @@ -0,0 +1,27 @@ +import React, { DragEventHandler } from 'react' + +interface DropTargetProps { + children: JSX.Element + onDragOver: DragEventHandler + onDrop: DragEventHandler + id: string + className?: string +} + +export const DropTarget: React.FC = ({ + children, + onDragOver, + onDrop, + id, + className +}) => ( +
+ {children} +
+) From ed98d9472d7d5aabf901a50880e34d7a5dba3c3a Mon Sep 17 00:00:00 2001 From: Stephanie Roy Date: Mon, 1 Aug 2022 14:00:14 -0400 Subject: [PATCH 2/3] Fix lnt issues --- .../shared/components/dragDrop/DragDropContainer.tsx | 10 ++++++---- webview/src/shared/components/dragDrop/Draggable.tsx | 7 +++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/webview/src/shared/components/dragDrop/DragDropContainer.tsx b/webview/src/shared/components/dragDrop/DragDropContainer.tsx index 472df5c8c9..8f78f95fb6 100644 --- a/webview/src/shared/components/dragDrop/DragDropContainer.tsx +++ b/webview/src/shared/components/dragDrop/DragDropContainer.tsx @@ -234,21 +234,23 @@ export const DragDropContainer: React.FC = ({ /> ) - const createItemWithDropTarget = (id: string, item: JSX.Element) => { - const isEnteringRight = direction === DragEnterDirection.RIGHT - const targetClassName = shouldShowOnDrag + const getDropTargetClassNames = (isEnteringRight: boolean) => + shouldShowOnDrag ? cx(styles.dropTargetWhenShowingOnDrag, { [styles.dropTargetWhenShowingOnDragLeft]: !isEnteringRight, [styles.dropTargetWhenShowingOnDragRight]: isEnteringRight }) : undefined + + const createItemWithDropTarget = (id: string, item: JSX.Element) => { + const isEnteringRight = direction === DragEnterDirection.RIGHT const target = ( {dropTarget} diff --git a/webview/src/shared/components/dragDrop/Draggable.tsx b/webview/src/shared/components/dragDrop/Draggable.tsx index 900774f642..f504f0f6af 100644 --- a/webview/src/shared/components/dragDrop/Draggable.tsx +++ b/webview/src/shared/components/dragDrop/Draggable.tsx @@ -26,13 +26,12 @@ export const Draggable: React.FC = ({ onDrop, onDragEnter, onDragStart + // eslint-disable-next-line sonarjs/cognitive-complexity }) => { - const groupStates = useSelector( - (state: ExperimentsState) => state.dragAndDrop.groups + const groupState = useSelector( + (state: ExperimentsState) => state.dragAndDrop.groups[group] || {} ) const dispatch = useDispatch() - - const groupState = groupStates[group] || {} const { draggedOverId, draggedId } = groupState const modifyGroup = (id: string) => { From a8c6a36b00dc0a1942b43a8d0a1297704f187c40 Mon Sep 17 00:00:00 2001 From: Stephanie Roy Date: Mon, 1 Aug 2022 15:46:06 -0400 Subject: [PATCH 3/3] Remove eslint for sonar --- webview/src/shared/components/dragDrop/Draggable.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/webview/src/shared/components/dragDrop/Draggable.tsx b/webview/src/shared/components/dragDrop/Draggable.tsx index f504f0f6af..51e442fbf8 100644 --- a/webview/src/shared/components/dragDrop/Draggable.tsx +++ b/webview/src/shared/components/dragDrop/Draggable.tsx @@ -26,7 +26,6 @@ export const Draggable: React.FC = ({ onDrop, onDragEnter, onDragStart - // eslint-disable-next-line sonarjs/cognitive-complexity }) => { const groupState = useSelector( (state: ExperimentsState) => state.dragAndDrop.groups[group] || {}