From f075e0742e02a013ad1ffe80792d51bc7b65c803 Mon Sep 17 00:00:00 2001 From: Furkan Kalaycioglu Date: Wed, 27 Apr 2022 20:37:59 +0300 Subject: [PATCH] Fixed the bug 'onNodeDragStop' not getting triggered Got rid of 'dragging' in the global node state --- .../Nodes/useMemoizedMouseHandler.ts | 5 +- src/components/Nodes/wrapNode.tsx | 65 ++++++++----------- src/components/NodesSelection/index.tsx | 5 -- src/container/NodeRenderer/index.tsx | 1 - src/hooks/useDrag.ts | 13 ++-- src/store/index.ts | 10 +-- src/store/utils.ts | 5 +- src/types/changes.ts | 1 - src/types/nodes.ts | 3 - src/utils/changes.ts | 4 -- src/utils/graph.ts | 4 +- 11 files changed, 41 insertions(+), 75 deletions(-) diff --git a/src/components/Nodes/useMemoizedMouseHandler.ts b/src/components/Nodes/useMemoizedMouseHandler.ts index 36334695..7d3067f3 100644 --- a/src/components/Nodes/useMemoizedMouseHandler.ts +++ b/src/components/Nodes/useMemoizedMouseHandler.ts @@ -5,18 +5,17 @@ import { ReactFlowState, Node } from '../../types'; function useMemoizedMouseHandler( id: string, - dragging: boolean, getState: GetState, handler?: (event: MouseEvent, node: Node) => void ) { const memoizedHandler = useCallback( (event: MouseEvent) => { - if (typeof handler !== 'undefined' && !dragging) { + if (typeof handler !== 'undefined') { const node = getState().nodeInternals.get(id)!; handler(event, { ...node }); } }, - [handler, dragging, id] + [handler, id] ); return memoizedHandler; diff --git a/src/components/Nodes/wrapNode.tsx b/src/components/Nodes/wrapNode.tsx index 401d7712..ab0e73bb 100644 --- a/src/components/Nodes/wrapNode.tsx +++ b/src/components/Nodes/wrapNode.tsx @@ -1,4 +1,14 @@ -import React, { useEffect, useRef, memo, ComponentType, CSSProperties, useMemo, MouseEvent, useCallback } from 'react'; +import React, { + useEffect, + useRef, + memo, + ComponentType, + CSSProperties, + useMemo, + MouseEvent, + useCallback, + useState, +} from 'react'; import cc from 'classcat'; import shallow from 'zustand/shallow'; @@ -41,7 +51,6 @@ export default (NodeComponent: ComponentType) => { sourcePosition, targetPosition, hidden, - dragging, resizeObserver, dragHandle, zIndex, @@ -69,11 +78,11 @@ export default (NodeComponent: ComponentType) => { [zIndex, xPos, yPos, hasPointerEvents, style] ); - const onMouseEnterHandler = useMemoizedMouseHandler(id, dragging, store.getState, onMouseEnter); - const onMouseMoveHandler = useMemoizedMouseHandler(id, dragging, store.getState, onMouseMove); - const onMouseLeaveHandler = useMemoizedMouseHandler(id, dragging, store.getState, onMouseLeave); - const onContextMenuHandler = useMemoizedMouseHandler(id, false, store.getState, onContextMenu); - const onNodeDoubleClickHandler = useMemoizedMouseHandler(id, false, store.getState, onNodeDoubleClick); + const onMouseEnterHandler = useMemoizedMouseHandler(id, store.getState, onMouseEnter); + const onMouseMoveHandler = useMemoizedMouseHandler(id, store.getState, onMouseMove); + const onMouseLeaveHandler = useMemoizedMouseHandler(id, store.getState, onMouseLeave); + const onContextMenuHandler = useMemoizedMouseHandler(id, store.getState, onContextMenu); + const onNodeDoubleClickHandler = useMemoizedMouseHandler(id, store.getState, onNodeDoubleClick); const onSelectNodeHandler = useCallback( (event: MouseEvent) => { @@ -121,15 +130,17 @@ export default (NodeComponent: ComponentType) => { [id, selected, selectNodesOnDrag, isSelectable, onNodeDragStart] ); + // As one of the props passed to a custom node + const [dragging, setDragging] = useState(false); + const onDrag = useCallback( (event: UseDragEvent, dragPos: UseDragData) => { - updateNodePosition({ id, dragging: true, diff: { x: dragPos.dx, y: dragPos.dy } }); - + updateNodePosition({ id, diff: { x: dragPos.dx, y: dragPos.dy } }); + setDragging(true); if (onNodeDrag) { const node = store.getState().nodeInternals.get(id)!; onNodeDrag(event.sourceEvent as MouseEvent, { ...node, - dragging: true, position: { x: node.position.x + dragPos.dx, y: node.position.y + dragPos.dy, @@ -146,37 +157,13 @@ export default (NodeComponent: ComponentType) => { const onDragStop = useCallback( (event: UseDragEvent) => { - // onDragStop also gets called when user just clicks on a node. - // Because of that we set dragging to true inside the onDrag handler and handle the click here - let node; - - if (onClick || onNodeDragStop) { - node = store.getState().nodeInternals.get(id)!; - } - - if (!dragging) { - if (isSelectable && !selectNodesOnDrag && !selected) { - addSelectedNodes([id]); - } - - if (onClick && node) { - onClick(event.sourceEvent as MouseEvent, { ...node }); - } - - return; - } - - updateNodePosition({ - id, - dragging: false, - }); - - // @TODO: Fix the bug "onNodeDragStop not getting called on first render" - if (onNodeDragStop && node) { - onNodeDragStop(event.sourceEvent as MouseEvent, { ...node, dragging: false }); + setDragging(false); + if (onNodeDragStop) { + const node = store.getState().nodeInternals.get(id)!; + onNodeDragStop(event.sourceEvent as MouseEvent, { ...node }); } }, - [id, isSelectable, selectNodesOnDrag, onClick, onNodeDragStop, dragging, selected] + [id, onNodeDragStop] ); useEffect(() => { diff --git a/src/components/NodesSelection/index.tsx b/src/components/NodesSelection/index.tsx index 735db606..494ea639 100644 --- a/src/components/NodesSelection/index.tsx +++ b/src/components/NodesSelection/index.tsx @@ -74,7 +74,6 @@ function NodesSelection({ x: data.dx, y: data.dy, }, - dragging: true, }); onSelectionDrag?.(event.sourceEvent, selectedNodes); @@ -84,10 +83,6 @@ function NodesSelection({ const onStop = useCallback( (event: UseDragEvent) => { - updateNodePosition({ - dragging: false, - }); - onSelectionDragStop?.(event.sourceEvent, selectedNodes); }, [selectedNodes, onSelectionDragStop] diff --git a/src/container/NodeRenderer/index.tsx b/src/container/NodeRenderer/index.tsx index 0a4be46f..7ff1cfa6 100644 --- a/src/container/NodeRenderer/index.tsx +++ b/src/container/NodeRenderer/index.tsx @@ -88,7 +88,6 @@ const NodeRenderer = (props: NodeRendererProps) => { hidden={node.hidden} xPos={node.positionAbsolute?.x ?? 0} yPos={node.positionAbsolute?.y ?? 0} - dragging={!!node.dragging} selectNodesOnDrag={props.selectNodesOnDrag} onClick={props.onNodeClick} onMouseEnter={props.onNodeMouseEnter} diff --git a/src/hooks/useDrag.ts b/src/hooks/useDrag.ts index 878a966d..f698af02 100644 --- a/src/hooks/useDrag.ts +++ b/src/hooks/useDrag.ts @@ -118,11 +118,12 @@ function useDrag({ dy: pos.y, }); } - } - }) - .on('end', (event) => { - if (isDragAllowedBySelector) { - onStop(event); + + event.on('end', (event) => { + if (isDragAllowedBySelector) { + onStop(event); + } + }); } }) .filter((event: any) => !event.ctrlKey && !event.button && !event.target.className.includes(noDragClassName)); @@ -134,7 +135,7 @@ function useDrag({ }; } } - }, [disabled, noDragClassName, nodeId]); + }, [onStart, onDrag, onStop, nodeRef, disabled, noDragClassName, handleSelector, nodeId]); return null; } diff --git a/src/store/index.ts b/src/store/index.ts index a1855a3d..d1c1e9c7 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -96,7 +96,7 @@ const createStore = () => onNodesChange?.(changes); } }, - updateNodePosition: ({ id, diff, dragging }: NodeDiffUpdate) => { + updateNodePosition: ({ id, diff }: NodeDiffUpdate) => { const { onNodesChange, nodeExtent, nodeInternals, hasDefaultNodes, snapGrid, snapToGrid } = get(); if (hasDefaultNodes || onNodesChange) { @@ -105,14 +105,10 @@ const createStore = () => nodeInternals.forEach((node) => { if (node.selected) { if (!node.parentNode || !isParentSelected(node, nodeInternals)) { - changes.push( - createPositionChange({ node, diff, dragging, nodeExtent, nodeInternals, snapToGrid, snapGrid }) - ); + changes.push(createPositionChange({ node, diff, nodeExtent, nodeInternals, snapToGrid, snapGrid })); } } else if (node.id === id) { - changes.push( - createPositionChange({ node, diff, dragging, nodeExtent, nodeInternals, snapToGrid, snapGrid }) - ); + changes.push(createPositionChange({ node, diff, nodeExtent, nodeInternals, snapToGrid, snapGrid })); } }); diff --git a/src/store/utils.ts b/src/store/utils.ts index 49ad662f..0d8796ec 100644 --- a/src/store/utils.ts +++ b/src/store/utils.ts @@ -44,7 +44,7 @@ export function createNodeInternals(nodes: Node[], nodeInternals: NodeInternals) const parentNodes: ParentNodes = {}; nodes.forEach((node) => { - const z = isNumeric(node.zIndex) ? node.zIndex : node.dragging || node.selected ? 1000 : 0; + const z = isNumeric(node.zIndex) ? node.zIndex : node.selected ? 1000 : 0; const internals: Node = { ...nodeInternals.get(node.id), @@ -112,7 +112,6 @@ type CreatePostionChangeParams = { nodeExtent: CoordinateExtent; nodeInternals: NodeInternals; diff?: XYPosition; - dragging?: boolean; snapToGrid?: boolean; snapGrid?: SnapGrid; }; @@ -120,7 +119,6 @@ type CreatePostionChangeParams = { export function createPositionChange({ node, diff, - dragging, nodeExtent, nodeInternals, snapToGrid, @@ -129,7 +127,6 @@ export function createPositionChange({ const change: NodePositionChange = { id: node.id, type: 'position', - dragging: !!dragging, }; if (diff) { diff --git a/src/types/changes.ts b/src/types/changes.ts index d896d03e..f833fbf0 100644 --- a/src/types/changes.ts +++ b/src/types/changes.ts @@ -13,7 +13,6 @@ export type NodePositionChange = { id: string; type: 'position'; position?: XYPosition; - dragging?: boolean; }; export type NodeSelectionChange = { diff --git a/src/types/nodes.ts b/src/types/nodes.ts index d8592cc4..4f0262b7 100644 --- a/src/types/nodes.ts +++ b/src/types/nodes.ts @@ -15,7 +15,6 @@ export interface Node { sourcePosition?: Position; hidden?: boolean; selected?: boolean; - dragging?: boolean; draggable?: boolean; selectable?: boolean; connectable?: boolean; @@ -79,7 +78,6 @@ export interface WrapNodeProps { sourcePosition: Position; targetPosition: Position; hidden?: boolean; - dragging: boolean; resizeObserver: ResizeObserver | null; dragHandle?: string; zIndex: number; @@ -96,7 +94,6 @@ export type NodeHandleBounds = { export type NodeDiffUpdate = { id?: string; diff?: XYPosition; - dragging?: boolean; }; export type NodeDimensionUpdate = { diff --git a/src/utils/changes.ts b/src/utils/changes.ts index e5278a6f..9279a3f0 100644 --- a/src/utils/changes.ts +++ b/src/utils/changes.ts @@ -69,10 +69,6 @@ function applyChanges(changes: any[], elements: any[]): any[] { updateItem.position = currentChange.position; } - if (typeof currentChange.dragging !== 'undefined') { - updateItem.dragging = currentChange.dragging; - } - if (updateItem.expandParent) { handleParentExpand(res, updateItem); } diff --git a/src/utils/graph.ts b/src/utils/graph.ts index a0927dbc..b71325e9 100644 --- a/src/utils/graph.ts +++ b/src/utils/graph.ts @@ -161,7 +161,7 @@ export const getNodesInside = ( const visibleNodes: Node[] = []; nodeInternals.forEach((node) => { - const { positionAbsolute, width, height, dragging, selectable = true } = node; + const { positionAbsolute, width, height, selectable = true } = node; if (excludeNonSelectableNodes && !selectable) { return false; @@ -172,7 +172,7 @@ export const getNodesInside = ( const yOverlap = Math.max(0, Math.min(rBox.y2, nBox.y2) - Math.max(rBox.y, nBox.y)); const overlappingArea = Math.ceil(xOverlap * yOverlap); const notInitialized = - typeof width === 'undefined' || typeof height === 'undefined' || width === null || height === null || dragging; + typeof width === 'undefined' || typeof height === 'undefined' || width === null || height === null; const partiallyVisible = partially && overlappingArea > 0; const area = (width || 0) * (height || 0);