Pie-chart hover events #150
Conversation
| @@ -309,6 +309,13 @@ fx.unhover = function (gd, evt, subplot) { | |||
| // The actual implementation is here: | |||
|
|
|||
| function hover(gd, evt, subplot){ | |||
| if(gd._fullLayout._hasPie){ | |||
There was a problem hiding this comment.
@mdtusz you're right.
The concept of (sub)plots doesn't extend to pies. Pies are considered traces that have thier own domain and no corresponding axes.
What does evt contain here?
There was a problem hiding this comment.
evt contains the data from the hover event - in this case, information on the slice that was hovered over. The plotly_hover event fires and gets passed data in the same format as the click events.
There was a problem hiding this comment.
Right, but Fx.hover for pie traces should emit that same data as for non-pie traces minus the info about the axes of course.
There was a problem hiding this comment.
The contents of [evt] mimic the same "format" as other traces hover events points value (as much as we can with a pie chart) but xvals/yvals and xaxes/yaxes are out of context here. The xvals/yvals are the mouse hover position relative to the axes scaling and values, so without any axes, these are meaningless.
There was a problem hiding this comment.
Great. Thanks for the clarification. 🍻
There was a problem hiding this comment.
@mdtusz On second thought, a check on _hasPie is too coerced.
Say we have a pie and bar trace on the same graph:
var gd = Tabs.fresh();
Plotly.plot(gd,
[
{
type: 'pie',
labels: ['a', 'b', 'c'],
values: [1, 2, 3],
domain: {
x: [0, 0.45]
}
},
{
type: 'bar',
x: ['A', 'B', 'C'],
y: [1, 2, 3],
}
],
{
title: 'Pie vs bar',
xaxis: { domain: [0.55, 1] },
showlegend: false
}
);
gd.on('plotly_hover', function(data) {
var pt = data.points[0]
console.log(pt);
});then hovering over the bars will result in an un-formatted hover data.
So, we should use the subplot argument and make the if-clause: if(subplot === 'pie').
|
Looks good. If you're up for it, it'd be nice to add a test in https://github.com/plotly/plotly.js/blob/master/test/jasmine/tests/hover_label_test.js for pie hover labels. |
|
Not sure why tests are failing... they all pass locally. Can you try pulling the branch down and testing it locally @etpinard? |
| var el = document.elementFromPoint(x,y); | ||
| var options = opts || { bubbles: true }; | ||
| var ev = new window.MouseEvent(type, options); | ||
| el.dispatchEvent(ev); |
|
@mdtusz here's why:
The local tests run on Chrome by default whereas the CircleCi tests run on FF. Looks like FF is more precise than Chrome here. |
|
Ahh lovely. Ok. I'll have to make the test expectations a lil bit more/less specific then. Good catch though. I scanned the values but my eyes must have swapped the 5 and 6! |
|
@mdtusz maybe use Jasmine's |
| expect(count).toEqual(2); | ||
| expect(futureData[0]).not.toEqual(futureData[1]) | ||
| done(); | ||
| }, 100); |
|
@etpinard anything else we want to add to this pr? |
|
💃 |
The
fx.hovermethod needed a bit of a tweak because pie gd's don't seem to have a_fullLayout.plots.