callback 'onNodeClick' was triggered when clicking the collapse/expand button in Safari

[zhoufenfens]

Describe the bug callback 'onNodeClick' was triggered when clicking the collapse/expand button in Safari To Reproduce Steps to reproduce the behavior:

1. wirte some code in onNodeClick function
2. Click on expand button
3. the code was excuted

Expected behavior I wanted to jump to other page when clicking the node, but it also jumped to other page when I was just clicking the expand/collapse button in Safari, Screenshots

Desktop (please complete the following information):

• OS: macos 10.15.7 macos 12.2.1
• Browser safari
• Version 15.3

[drosi94]

I got the same issue in safari as well

[bumbeishvili]

For the workaround, it's possible to add a custom onclick listener to the content inside and use it as onNodeClick replacement

[zhoufenfens]

For the workaround, it's possible to add a custom onclick listener to the content inside and use it as onNodeClick replacement

yes, i bind the window.clickCb to the nodeContent html string

[iae1]

@zhoufenfens @bumbeishvili can you show an example of what you're talking about?

[zhoufenfens]

@zhoufenfens @bumbeishvili can you show an example of what you're talking about?

https://stackblitz.com/edit/web-platform-xh4yff open this link in safari, click the collapse/expand button, the onNodeClick callback was triggrered.

[schindld]

I think the fix for this is to add event.stopPropagation() to the end of onButtonClick to prevent the button clicks from also triggering the node click handler.

The work-around @bumbeishvili mentioned is to override the onButtonClick handler yourself, and add the event.stopPropagation() at the end. So, after the chart's render() function is called, add this:

chart.onButtonClick = function(event, d) {
      const attrs = this.getChartState();
      if (attrs.setActiveNodeCentered) {
          d.data._centered = true;
          d.data._centeredWithDescendants = true;
      }

      // If childrens are expanded
      if (d.children) {
          //Collapse them
          d._children = d.children;
          d.children = null;

          // Set descendants expanded property to false
          this.setExpansionFlagToChildren(d, false);
      } else {
          // Expand children
          d.children = d._children;
          d._children = null;

          // Set each children as expanded
          if (d.children) {
              d.children.forEach(({ data }) => (data._expanded = true));
          }
      }

      // Redraw Graph
      this.update(d);
      event.stopPropagation();
}

While we're at it, onNodeClick should stop propagation too so that users can select text, or so we can do something else with the mouse events. And to make it so we can add event.stopPropagation() to an overridden onNodeClick, it needs the event as a parameter...Right now we just get the node id.

[zhoufenfens]

good idea, it works in safari

[bumbeishvili]

Agreed, I will add it in the next release (don't know exactly when though)