Falsy hovermode and/or hoverinfo should not disabe click event data

[etpinard]

See http://codepen.io/etpinard/pen/zqvOav?

N.B. this issue only occurs for cartesian trace types it looks like.

[monfera]

@etpinard nice manual test case! Forked it into http://codepen.io/monfera/full/KzXNzy/ because original didn't run (a comma accidentally deleted). I could reproduce the problem. Looking into the possible cause.

[monfera]

I made a change that results in the callback being called, but the proper solution is something else. The click callback would be called, but it isn't because eventData is set to undefined (and usually no point to have a click handler if we don't know what we click on). It's the unhover function that removes eventData from the object: gd._hoverdata = undefined;. Looking into why it apparently gets repopulated for other charts but not for the erroneous one.

[monfera]

This little change solves the reported problem for one of the conditions: when the hoverinfo is set to 'none'.

https://github.com/monfera/plotly.js/pull/2/files

The problem is that the term 'hover' is used a bit inconsistently, as some bindings seemingly named to 'hover' are also used ultimately in the click handler. So once the bug is fixed, we might consider a name disambiguation.

Looking into the other condition (hovermode is falsey) which didn't yield on the first attempt.

[monfera]

@etpinard the root cause analysis and the hoverinfo='none' fix were simple enough, but dealing with hovernote is much less trivial, because of how the aspects of hover rendering and determining eventData are intertwined in graph_interact.js:hover() and a couple of subsidiary functions, 600+ lines total. Glad to press ahead but in case of getting stuck, I'd switch back updating the test cases of my prior PR. Long term, graph_interact.js:hover() might benefit from a split into compact, mostly side effect free functions.

[abalter]

@monfera Is there a way I could implement your solution as a temporary fix for my plots?

[monfera]

@abalter This issue is under resolution, however this specific commit in my fork appears to fix the issue when hoverinfo is set to none. So this is a partial fix and untested at that.

If it's helpful, use this small commit in your checkout, run npm run build and use the resulting dist/plotly.min.js file.

[monfera]

@etpinard I'm just jotting down my current thinking as it hasn't crystallized yet and maybe some of the points remind you of some important aspect or just suggest which way to go.

My earlier comment described that solving hoverinfo = 'none' was a quick thing, but solving hovermode = false isn't, because hover in the code means two things: 1) whether or not (or how) tooltips should appear; 2) how data is prepared if a mouse is over a point, e.g. for the purpose of passing that data on to the plotly_click handler, without which the handler isn't of much use.

I made code changes such that tooltips do not show up but the appropriate data is still collected. It almost works, save for the following problem: in your cited codepen example, the cartesian line graph has two points (x=1, x=3) where the y value is the same (y=2). Clicking on (x=3,y=2) returns data as though I clicked on (x=1,y=2). It works fine if I supply data with unique y values. I think the problem is that when hovermode is false, it won't run in the proper code path and maybe thinks the y axis is the independent axis (perhaps because hovermode !== 'x'), and finds the first match.

I see two options:

1. Try to explicitly separate the notion of point data collection from the notion of the tooltip by refactoring large parts of graph_interact;
2. or bypass the complexities by coercing hovermode to a proper value (closest?) when it would be false, while coercing hoverinfo to none which my minimal change seems to handle well.

My lack of confidence about deeper changes is due to various challenges. e.g.:

• there are hundreds of lines in some functions in graph_interact, and I don't yet feel studied enough for deeper, yet quick changes
• I still need to learn about overlap/clumping, as it makes sense for the tooltip, and it may be useful, or perhaps already in effect, for click handlers as well
• hovermode appears to have no layout_attributes definition except a restricted value set under gl3d so I need to manually collect the possible values and wondering if having no layout_attributes is for a purpose, or forgotten, and if I should define one.

For these reasons, unless you suggest otherwise, I'm about to try solution 2) which would take care of the hovermode and hoverinfo value via coercion: if hovermode is false, set hovermode to closest and hoverinfo to none. This would entail no change to downstream code so less chance I break something :-) Also there are discussed new tickets so I'm striving to avoid a major refactor of graph_interact which would be great learning but time consuming.