xuhongxu96 / editflow

DAG workflow editor
4 stars 0 forks source link

delete node that is not dragged out from the nodetoolbox #11

Closed SerinaJingyiLu closed 4 years ago

SerinaJingyiLu commented 4 years ago
  1. When creating a node by dragging in, undo the creation if the node fails to be dragged out from the nodetoolbox.
  2. Add nodes/edges deletion feat (support keystroke and mouse click simultaneously).
  3. Modify edge/node selection behavior: Only 'unselectAllEdges' or 'unselectAllNodes' when 'mousedown' event is triggered within the canvas. (Otherwise, we cannot capture any selected edges/nodes when we want to do something with them)

Besides, I still have some concerns on edge selection. why do we need to 'unselectedAllNodes' when an edge is selected?

SerinaJingyiLu commented 4 years ago

I think it has no problem as I e.stopPropagation();?

Yeah, it works well when select an edge or a node. However, for the code below:

 useEventListener('mousedown', useCallback(() => {
        dispatch({ type: 'unselectAllEdges' });
    }, [dispatch]));

or

    useEventListener('mousedown', useCallback(() => {
        dispatch({ type: 'unselectAllNodes' });
    }, [dispatch]));

If you do not set the scope for 'unselectAllEdges' or 'unselectAllNodes' event, these events will be triggered anywhere your mouse is down, which means when you want to click the 'delete' button, it will first trigger the 'unselectAllEdges' or 'unselectAllNodes' event, and no selected edge or node can be captured.

SerinaJingyiLu commented 4 years ago

I have already added prettier to unify the format. You can run the script 'npm run prettier' to format it or the code will be automatically formatted when you run 'git commit'. The config for prettier can be changed. Right now, I am using semi with single quote and trailing comma.

xuhongxu96 commented 4 years ago

I think it has no problem as I e.stopPropagation();?

Yeah, it works well when select an edge or a node. However, for the code below:

 useEventListener('mousedown', useCallback(() => {
        dispatch({ type: 'unselectAllEdges' });
    }, [dispatch]));

or

    useEventListener('mousedown', useCallback(() => {
        dispatch({ type: 'unselectAllNodes' });
    }, [dispatch]));

If you do not set the scope for 'unselectAllEdges' or 'unselectAllNodes' event, these events will be triggered anywhere your mouse is down, which means when you want to click the 'delete' button, it will first trigger the 'unselectAllEdges' or 'unselectAllNodes' event, and no selected edge or node can be captured.

Oh, yes. Maybe the event listener should be added to Canvas instead of Window in the future. Anyway, let's use your fix for now.

xuhongxu96 commented 4 years ago

I have already added prettier to unify the format. You can run the script 'npm run prettier' to format it or the code will be automatically formatted when you run 'git commit'. The config for prettier can be changed. Right now, I am using semi with single quote and trailing comma.

Great!