plotly_click event doesn't fire when clicking non-root/leaf nodes of sunburst trace

[cpsievert]

Originally reported in plotly/plotly.R#1648

Minimal example (note how clicking on the middle ring doesn't trigger plotly_click) https://codepen.io/cpsievert/pen/OJJZyrZ

[archmoj]

plotly_sunburstclick is the event you may looking for.
Here is a demo.

[cpsievert]

Oh, nice; thanks, and why is a new event needed? It'd be nice if the event was mentioned in the release notes.

[alexcjohnson]

@archmoj @etpinard can we please dredge up a clear answer as to why this odd behavior exists with plotly_click (for treemap as well) and whether it would be feasible to make plotly_click behave the way users would expect, before we go to the trouble of documenting these trace-specific variants, adding them to dash, etc etc.

[archmoj]

For reference plotly_sunburstclick is added to sunburst/plot.js within this commit: a7e7e65ed6

[etpinard]

First off, the current behaviour is very much intended (see the logic here). So the discussion here should be about is the current behaviour correct.

Here's some background: for most traces (maybe all, but I'd have to double-check) plotly_click means "give me the plotly_hover event data" on click without doing anything else. For sunburst (and now treemap) traces, click does a lot more (it triggers a transitions). Most importantly, sunburst and treemap allow users to override the "click" behaviour with:

gd.on('plotly_sunburstclick', () => {
  /* new logic e.g. make a request to a database */

  return false; // cancel the default handler
})

This was one of the original requirements for sunburst traces. Note that returning false to in a custom plotly_click event handler does nothing - and arguably it should always do nothing.

---

So, is this ticket asking to:

1. rename plotly_sunburstclick and plotly_treemapclick --> plotly_click? That is no longer make a distinction between clicks that trigger transitions and other clicks.
2. OR trigger plotly_click (which just returns the plotly_hover event data) on all nodes, not just root and leaf nodes? This means that clicking on inner nodes (i.e. between the root and the leaves) would lead to BOTH a plotly_(sunburst|treemap)click event and a plotly_click event being triggered.

[etpinard]

Personally, I'm very (very) much in favour of 2).

[cpsievert]

Thanks @etpinard! I think the behavior you describe in 2 is more intuitive for most users.

[alexcjohnson]

Thanks for clarifying, @etpinard - we were missing that context. It does make sense to keep the transition-canceling property only in the plotly_*click events, so essentially option 2 sounds great to me as well.

But according to the existing logic, you get a plotly_click event in sunburst or treemap traces precisely when there's no transition - either because you clicked on a node that doesn't support transitions, OR because the plotly_*click handler canceled the transition. That seems weird to me - I'd say if plotly_*click canceled the transition, it should cancel the plotly_click event too. But otherwise plotly_click should alway be triggered, transition or no. It could be useful to know whether there was a transition triggered by the click or not, so perhaps we can find a way to add that to the event handler?

[etpinard]

That seems weird to me - I'd say if plotly_*click canceled the transition, it should cancel the plotly_click event too.

So, there are three scenarios:

1. user clicks on the root node OR a leaf node:

• current: fires plotly_sunburstclick and fires plotly_click
• wanted: same

2. user clicks on an inner node:

• current: executes plotly_sunburstclick handler, triggers transition
• wanted: executes plotly_sunburstclick handler, fires plotly_click, triggers transition

3. user clicks on an inner node AND had set up a plotly_sunburstclick handler that returns false:

• current: executes plotly_sunburstclick handler, fires plotly_click
• wanted (by AJ 😏 ): executes plotly_sunburstclick hander

Is that correct?

---

It could be useful to know whether there was a transition triggered by the click or not, so perhaps we can find a way to add that to the event handler?

Yeah, that's a good idea. The sunburst/treemap event data currently looks like:

I can't really imagine placing the transition info inside the points array, so maybe the event data could have three subcontainers when a transition is triggered:

{
  event: { /* instanceof MouseEvent */ }
  points: [{ /* */ }],
  transition: {
     duration: /* value from the constant file */,
     easing: /* value from the constant file */
  }
}

or maybe a just a boolean would suffice 🤔

---

Moreover, theses changes are borderline breaking, so at the very least they would have to be released in a minor.

[alexcjohnson]

1. user clicks on the root node OR a leaf node:

• current: fires plotly_click
• wanted: same

2. user clicks on an inner node:

• current: executes plotly_sunburstclick handler, triggers transition
• wanted: executes plotly_sunburstclick handler, fires plotly_click, triggers transition

3. user clicks on an inner node AND had set up a plotly_sunburstclick handler that returns false:

• current: executes plotly_sunburstclick handler, fires plotly_click
• wanted (by AJ 😏 ): executes plotly_sunburstclick hander

Is that correct?

yep, that's what I had in mind! 🍻

I can't really imagine placing the transition info inside the points array, so maybe the event data could have three subcontainers when a transition is triggered:

{
  event: { /* instanceof MouseEvent */ }
  points: [{ /* */ }],
  transition: {
     duration: /* value from the constant file */,
     easing: /* value from the constant file */
  }
}

or maybe a just a boolean would suffice 🤔

As a user what I would want to know is not the transition parameters, but: did the root node of the view change, and if so to which node?

Moreover, theses changes are borderline breaking, so at the very least they would have to be released in a minor.

For sure, minor.