Ironclad / rivet

The open-source visual AI programming environment and TypeScript library
https://rivet.ironcladapp.com
MIT License
2.56k stars 226 forks source link

[Bug]: Node flickering when selected with click, appears to interrupt double-click event #277

Closed codemile closed 5 months ago

codemile commented 6 months ago

What happened?

Double click events for nodes in the graph aren't working (reproduced on Windows & Mac). I am assuming the reason is related to the flickering of nodes when they are clicked on. A possible cause would be the node mounting/unmounting when changing state. As the body of the node is first empty and then redrawn with the node's contents.

This bug report is a follow up on #275

What was the expected functionality?

Node's shouldn't flicker when being clicked, and double click features should be working.

Describe your environment

Windows & MacOS, Node 20.10.0

Relevant log output

No response

Relevant screenshots

https://github.com/Ironclad/rivet/assets/50146659/dc3f3488-14a7-49c7-a696-25759762aa1c

Code of Conduct

codemile commented 6 months ago

I can confirm that left mouse down triggers an unmounting of DraggableNode and then mounting it again. I don't think this is a desired effect, and noticed that VisualNode uses memo().

image

codemile commented 6 months ago

I understand now. Nodes are rendered in NodeCanvas under the <div className="nodes"> section. All the nodes are rendered in the <DraggableNode> component, but when the mouse is pressed down it's removed from that list and rendered under the <DragOverlay> component. This is how DnD works on the canvas.

The problem here is that it interrupts double-click event listener, because the node is unmounted/remounted when the first click transfers it from the <DraggableNode> collection to the <DragOverlay> collection. Another side effect of moving the node to the other list is it causes the body to be redrawn which creates the flicker.

codemile commented 6 months ago

Omg... it's the title bar of the node. If you click/double inside the Node then everything works fine 🤦🏼‍♂️. That wasn't obvious to me at all. lol.

codemile commented 6 months ago

I'm revising this bug report, to be specific to drag'n'drop behavior, as it creates flickering when selecting nodes to be dragged. The issue does relate to being mounted/unmounted.

I see only two ways to fix this issue. 1) change the DnD so nodes are not moved from one list to another. 2) change VisualNode that the "first" render pass is identical to a second render pass.

I feel option 2 is the best approach, as option 1 is an optimization and not a fix.

https://github.com/Ironclad/rivet/assets/50146659/6d5d5481-0e9c-44b5-ba9a-685f1c9c1af7

codemile commented 6 months ago

So body content of nodes are loaded async. Which makes sense for the nature of the nodes features. It means that we can't easily fix the code for the first render pass to be the same as the second render pass. It's interesting how all this was built. How did you guys pull off so much work in a short time? lol