expose "onMoveNode" method (optional), added "canDragNode" method (optional), center svg icons in control buttons#280
expose "onMoveNode" method (optional), added "canDragNode" method (optional), center svg icons in control buttons#280man-person wants to merge 4 commits intouber:masterfrom
Conversation
added "canDragNode" method (optional)
|
@9renpoto @ksnyder9801 I’d be grateful if you could review this one |
| @@ -77,6 +78,7 @@ export type IGraphViewProps = { | |||
| onSwapEdge?: (sourceNode: INode, targetNode: INode, edge: IEdge) => void, | |||
| onUndo?: () => void, | |||
| onUpdateNode?: (node: INode) => void, | |||
There was a problem hiding this comment.
onUpdateNode should already tell you that the node moved. It passes the new X,Y coordinates within the INode type. Is it possible to call onUpdateNode instead of creating a new onMoveNode method?
There was a problem hiding this comment.
I think the problem with this is that onUpdateNode is only called when the move is finished, not while the move is in progress. That is also why one can not just return false from onUpdateNode and thereby prevent movement.
| hoveredNode: INode | null, | ||
| swapEdge: IEdge | ||
| ) => boolean, | ||
| canDragNode?: (nodeId: string) => boolean, |
There was a problem hiding this comment.
Change to canMoveNode or canUpdateNode?
| canSwapEdge: () => true, | ||
| canDeleteEdge: () => true, | ||
| canDeleteNode: () => true, | ||
| canDragNode: () => true, |
| if (!shiftKey && !draggingEdge) { | ||
| if (onMoveNode) { | ||
| onMoveNode(node); | ||
| } |
There was a problem hiding this comment.
This if statement needs to move within the if statement on line 809.
|
|
||
| > svg { | ||
| vertical-align: sub; | ||
| } |
There was a problem hiding this comment.
Please rebase on master. This may be fixed.
ajbogh
left a comment
There was a problem hiding this comment.
Please rebase on master and then comment to justify your reasoning and update the PR based on the conversation.
needed to do some stuff while dragging a node so i added these.
may be relevant to issues: #281 #261 #195