Plotly.js: Sankey hoverinfo options not honored

[antoinerg]

As of right now, Sankey traces ignore all hoverinfo options except for 'none' and 'skip'.

https://codepen.io/anon/pen/xyrXXm

[alexcjohnson]

In fact it looks like the fact that sankey even has a hoverinfo attribute is accidental, probably the whole attributes file was copied over from pie (judging from the flags) and that one never got deleted or addressed.

It's a bit of a tricky question how to handle hoverinfo here, as there are two independent classes of object you can hover on, links and nodes (speaking of which, we should probably have an attribute hoveron: 'links+nodes' here, and this comment applies to any other trace types with a hoveron flaglist). Links and nodes have some matching fields (flow value) and some different. What if you want links to show flow values but nodes not to.

What if hoverinfo in this case was split into link.hoverinfo and node.hoverinfo? Then we wouldn't need hoveron here, you could just specify the two hoverinfo values differently.

[antoinerg]

That makes sense. I guess we could also have the upcoming templates on hover be split into node.hovertemplate and link.hovertemplate 🤔

@etpinard What do you think? Should I go ahead and implement link and node separately?

[alexcjohnson]

That makes sense. I guess we could also have the upcoming templates on hover be split into node.hovertemplate and link.hovertemplate 🤔

Yes, I think those had better be separate too.

@antoinerg and I had a discussion about how to handle this for other types that support hoveron or otherwise have multiple classes of hoverable items - like scatter, how would we specify separate point and fill (and eventually line) hover templates (or separate hoverinfo and hoverlabel style)? Tentative conclusion was that trace.hover(info|template|label) should apply only to points, we should make a new container trace.fills.hover(info|template|label) for fill hover (and perhaps at least hoverlabel would be inherited from trace.hoverlabel) and when we get around to line hover we can add those to the existing line container, ie trace.line.hover(info|template|label). Any other ideas?

[antoinerg]

Because all the flags for hoveron are plural (ex.: lines, points, fills), I think it should be nodes.(info|template|label) and links.(info|template|label). As @alexcjohnson pointed out to me, we already have an attribute named fill so making it plural for hover would prevent a collision.

[antoinerg]

I wrote my last comment a bit too fast. We already have node and link attributes so I should just use them 🤔

[kristianjf]

One thing I observed, _seemingly_ related to this issue, is node.hoverinfo only seems to include the sum of values for incoming or outgoing flow. Would it be possible to provide the option to manipulate this (incoming, outgoing, or _both_: incoming and outgoing)?

[antoinerg]

Thank you @kristianjf for the suggestion! I agree that it should be included when Sankey's hoverinfo options are implemented.

[antoinerg]

The hoverinfo feature discussed in this issue might become useless with the upcoming potential inclusion of hovertemplate from PR https://github.com/plotly/plotly.js/pull/3284.

What do you think @kristianjf @plotly/plotly_js ?