react-d3-graph

react-d3-graph copied to clipboard

Describe the bug Only the click handler is called when nodes are collapsibe

To Reproduce Steps to reproduce the behavior:

1. Go to the live demo page
2. Double click on node -> see the double click popup on the right
3. Enable the collapsible property
4. Double click on a node now -> only the single click handler is called twice and node collapses and comes back as well

Expected behavior Node shouldn't collapse if its a double click and the double click handler should be called

Additional context I'd be happy to raise a PR to fix this, but I do have a few questions that I'd like to get your thoughts on.

1. Right now, double click is manually handled using a setTimeout hack in the onClickNode handler. Do we want this handling rewritten using the native ondblclick event?

2. If we want to stick with how it is right now, then we could tweak onClickNode to accommodate this in a couple of ways. Right now, if the collapsible property is not set, the onClick handler is delayed by 300ms so that we can detect a doubleClick if it happens before then. But if the collapsible property is set, the click event handler is triggered immediately (albeit in the callback of setState for the collapse behaviour). Hence a double click with collapsible property on is handled as 2 single click events (which occurs after the collapse and reverting of collapse), while a double click without collapsible property on is treated as a doubleClick (and even if there is no doubleClick handler prop passed, the click handler prop is not called as well). So there is a discrepancy here. So, what do we do for the above discrepancy? Also, if there isn't any doubleClick handler prop that is passed, do we need to treat a double click as 2 single clicks and trigger the single click handler twice? (this is happening now when the collapsible property is set and not happening when it is not )

3. To accommodate doubleClick during collapsible (if we don't rewrite with dblclick), we would need to delay the collapse handling code by 300ms as well whenever collapsible prop is passed so that we would invoke onDoubleClickNode instead of doing collapse twice when there is a doubleClick. Similar to question 2, we again have an option of doing this only when onDoubleClickNode is passed so that we don't always delay collapse by 300ms as well, and treat such a doubleClick as two single clicks . What do you think?

4. For the behaviour to be uniform, we could just always setTimeout by 300ms (aka check for doubleClick) in onNodeClick and then handle things accordingly within it. Hence, a doubleClick would never trigger 2 node single click handlers and all 'click behaviours' will always happen after 300ms irrespective of what the props to the graph are. So, if its a doubleClick -> doubleClick handler (if present) will be called It its a singleClick and node collapsible -> collapse node and call single click handler It its a singleClick and node not collapsible -> just call single click handler

I personally prefer option 1 or 4, but am curious as to what you want to go with and what your views are on 2/3 to standardize behaviour.

I would be very glad if you would attempt to re-implement this with native dblclick event.

FYI this has been attempted before: https://github.com/danielcaldas/react-d3-graph/pull/202

But some time has passed since then, it might be worth to give a another shoot! PRs are most welcome!

If this does not work we can revaluate.

+1

Hey @vsramanujan if you (or anyone) are willing to give this a try, I think we better go with Option 3

To accommodate doubleClick during collapsible (if we don't rewrite with dblclick), we would need to delay the collapse handling code by 300ms as well whenever collapsible prop is passed so that we would invoke onDoubleClickNode instead of doing collapse twice when there is a doubleClick. Similar to question 2, we again have an option of doing this only when onDoubleClickNode is passed so that we don't always delay collapse by 300ms as well, and treat such a doubleClick as two single clicks . What do you think?