Skip to content

Commit 28161d8

Browse files
committed
scatter & bar hover fixes for #780
scatter was too permissive and too flat in its distance function in compare mode bar was too restrictive, in compare mode it now allows hover while in the gap between bars/groups
1 parent 82f4c16 commit 28161d8

File tree

4 files changed

+103
-36
lines changed

4 files changed

+103
-36
lines changed

src/traces/bar/hover.js

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,35 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
1818
var cd = pointData.cd;
1919
var trace = cd[0].trace;
2020
var t = cd[0].t;
21+
var isClosest = (hovermode === 'closest');
2122

2223
var posVal, sizeVal, posLetter, sizeLetter, dx, dy;
2324

2425
function thisBarMinPos(di) { return di[posLetter] - di.w / 2; }
2526
function thisBarMaxPos(di) { return di[posLetter] + di.w / 2; }
2627

27-
var minPos = (hovermode === 'closest') ?
28+
var minPos = isClosest ?
2829
thisBarMinPos :
2930
function(di) {
3031
/*
3132
* In compare mode, accept a bar if you're on it *or* its group.
3233
* Nearly always it's the group that matters, but in case the bar
3334
* was explicitly set wider than its group we'd better accept the
3435
* whole bar.
36+
*
37+
* use `bardelta` instead of `bargroupwidth` so we accept hover
38+
* in the gap. That way hover doesn't flash on and off as you
39+
* mouse over the plot in compare modes.
40+
* In 'closest' mode though the flashing seems inevitable,
41+
* without far more complex logic
3542
*/
36-
return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
43+
return Math.min(thisBarMinPos(di), di.p - t.bardelta / 2);
3744
};
3845

39-
var maxPos = (hovermode === 'closest') ?
46+
var maxPos = isClosest ?
4047
thisBarMaxPos :
4148
function(di) {
42-
return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
49+
return Math.max(thisBarMaxPos(di), di.p + t.bardelta / 2);
4350
};
4451

4552
function positionFn(di) {
@@ -79,6 +86,18 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
7986
// skip the rest (for this trace) if we didn't find a close point
8087
if(pointData.index === false) return;
8188

89+
// if we get here and we're not in 'closest' mode, push min/max pos back
90+
// onto the group - even though that means occasionally the mouse will be
91+
// over the hover label.
92+
if(!isClosest) {
93+
minPos = function(di) {
94+
return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
95+
};
96+
maxPos = function(di) {
97+
return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
98+
};
99+
}
100+
82101
// the closest data point
83102
var index = pointData.index;
84103
var di = cd[index];

src/traces/bar/set_positions.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
227227
t.barwidth = barWidth;
228228
t.poffset = offsetFromCenter;
229229
t.bargroupwidth = barGroupWidth;
230+
t.bardelta = minDiff;
230231
}
231232

232233
// stack bars that only differ by rounding
@@ -277,6 +278,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
277278
t.barwidth = barWidth;
278279
t.poffset = offsetFromCenter;
279280
t.bargroupwidth = barGroupWidth;
281+
t.bardelta = minDiff;
280282
}
281283

282284
// stack bars that only differ by rounding

src/traces/scatter/hover.js

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,35 +18,44 @@ var fillHoverText = require('./fill_hover_text');
1818
var MAXDIST = Fx.constants.MAXDIST;
1919

2020
module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
21-
var cd = pointData.cd,
22-
trace = cd[0].trace,
23-
xa = pointData.xa,
24-
ya = pointData.ya,
25-
xpx = xa.c2p(xval),
26-
ypx = ya.c2p(yval),
27-
pt = [xpx, ypx],
28-
hoveron = trace.hoveron || '';
21+
var cd = pointData.cd;
22+
var trace = cd[0].trace;
23+
var xa = pointData.xa;
24+
var ya = pointData.ya;
25+
var xpx = xa.c2p(xval);
26+
var ypx = ya.c2p(yval);
27+
var pt = [xpx, ypx];
28+
var hoveron = trace.hoveron || '';
29+
var minRad = (trace.mode.indexOf('markers') !== -1) ? 3 : 0.5;
2930

3031
// look for points to hover on first, then take fills only if we
3132
// didn't find a point
3233
if(hoveron.indexOf('points') !== -1) {
3334
var dx = function(di) {
34-
// scatter points: d.mrc is the calculated marker radius
35-
// adjust the distance so if you're inside the marker it
36-
// always will show up regardless of point size, but
37-
// prioritize smaller points
35+
// dx and dy are used in compare modes - here we want to always
36+
// prioritize the closest data point, at least as long as markers are
37+
// the same size or nonexistent, but still try to prioritize small markers too.
3838
var rad = Math.max(3, di.mrc || 0);
39-
return Math.max(Math.abs(xa.c2p(di.x) - xpx) - rad, 1 - 3 / rad);
39+
var kink = 1 - 1 / rad;
40+
var dxRaw = Math.abs(xa.c2p(di.x) - xpx);
41+
var d = (dxRaw < rad) ? (kink * dxRaw / rad) : (dxRaw - rad + kink);
42+
return d;
4043
},
4144
dy = function(di) {
4245
var rad = Math.max(3, di.mrc || 0);
43-
return Math.max(Math.abs(ya.c2p(di.y) - ypx) - rad, 1 - 3 / rad);
46+
var kink = 1 - 1 / rad;
47+
var dyRaw = Math.abs(ya.c2p(di.y) - ypx);
48+
return (dyRaw < rad) ? (kink * dyRaw / rad) : (dyRaw - rad + kink);
4449
},
4550
dxy = function(di) {
46-
var rad = Math.max(3, di.mrc || 0),
47-
dx = xa.c2p(di.x) - xpx,
48-
dy = ya.c2p(di.y) - ypx;
49-
return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad);
51+
// scatter points: d.mrc is the calculated marker radius
52+
// adjust the distance so if you're inside the marker it
53+
// always will show up regardless of point size, but
54+
// prioritize smaller points
55+
var rad = Math.max(minRad, di.mrc || 0);
56+
var dx = xa.c2p(di.x) - xpx;
57+
var dy = ya.c2p(di.y) - ypx;
58+
return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - minRad / rad);
5059
},
5160
distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy);
5261

test/jasmine/tests/hover_label_test.js

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,6 +1108,43 @@ describe('hover info on stacked subplots', function() {
11081108
});
11091109

11101110

1111+
describe('hover on many lines+bars', function() {
1112+
'use strict';
1113+
1114+
afterEach(destroyGraphDiv);
1115+
1116+
it('shows hover info for both traces', function(done) {
1117+
// see https://github.com/plotly/plotly.js/issues/780
1118+
var values = new Array(1000);
1119+
var values2 = new Array(values.length);
1120+
for(var i = 0; i < values.length; i++) {
1121+
values[i] = i;
1122+
values2[i] = i * 2;
1123+
}
1124+
1125+
var gd = createGraphDiv();
1126+
Plotly.newPlot(gd, [
1127+
{y: values2},
1128+
{y: values, type: 'bar'}
1129+
], {
1130+
width: 400,
1131+
height: 400,
1132+
margin: {l: 100, r: 100, t: 100, b: 100}
1133+
})
1134+
.then(function() {
1135+
Lib.clearThrottle();
1136+
mouseEvent('mousemove', 200, 100);
1137+
Lib.clearThrottle();
1138+
1139+
expect(d3.select(gd).selectAll('g.hovertext').size()).toBe(2);
1140+
expect(d3.select(gd).selectAll('g.axistext').size()).toBe(1);
1141+
})
1142+
.catch(fail)
1143+
.then(done);
1144+
});
1145+
});
1146+
1147+
11111148
describe('hover info on overlaid subplots', function() {
11121149
'use strict';
11131150

@@ -1274,7 +1311,7 @@ describe('hover updates', function() {
12741311

12751312
afterEach(destroyGraphDiv);
12761313

1277-
function assertLabelsCorrect(mousePos, labelPos, labelText) {
1314+
function assertLabelsCorrect(mousePos, labelPos, labelText, msg) {
12781315
return new Promise(function(resolve) {
12791316
if(mousePos) {
12801317
mouseEvent('mousemove', mousePos[0], mousePos[1]);
@@ -1283,14 +1320,14 @@ describe('hover updates', function() {
12831320
setTimeout(function() {
12841321
var hoverText = d3.selectAll('g.hovertext');
12851322
if(labelPos) {
1286-
expect(hoverText.size()).toEqual(1);
1287-
expect(hoverText.text()).toEqual(labelText);
1323+
expect(hoverText.size()).toBe(1, msg);
1324+
expect(hoverText.text()).toBe(labelText, msg);
12881325

12891326
var transformParts = hoverText.attr('transform').split('(');
1290-
expect(transformParts[0]).toEqual('translate');
1327+
expect(transformParts[0]).toBe('translate', msg);
12911328
var transformCoords = transformParts[1].split(')')[0].split(',');
1292-
expect(+transformCoords[0]).toBeCloseTo(labelPos[0], -1, labelText + ':x');
1293-
expect(+transformCoords[1]).toBeCloseTo(labelPos[1], -1, labelText + ':y');
1329+
expect(+transformCoords[0]).toBeCloseTo(labelPos[0], -1, labelText + ':x ' + msg);
1330+
expect(+transformCoords[1]).toBeCloseTo(labelPos[1], -1, labelText + ':y ' + msg);
12941331
} else {
12951332
expect(hoverText.size()).toEqual(0);
12961333
}
@@ -1318,7 +1355,7 @@ describe('hover updates', function() {
13181355
var gd = createGraphDiv();
13191356
Plotly.plot(gd, mock).then(function() {
13201357
// The label text gets concatenated together when queried. Such is life.
1321-
return assertLabelsCorrect([100, 100], [103, 100], 'trace 00.5');
1358+
return assertLabelsCorrect([100, 100], [103, 100], 'trace 00.5', 'animation/update 0');
13221359
}).then(function() {
13231360
return Plotly.animate(gd, [{
13241361
data: [{x: [0], y: [0]}, {x: [0.5], y: [0.5]}],
@@ -1327,25 +1364,25 @@ describe('hover updates', function() {
13271364
}).then(function() {
13281365
// No mouse event this time. Just change the data and check the label.
13291366
// Ditto on concatenation. This is "trace 1" + "0.5"
1330-
return assertLabelsCorrect(null, [103, 100], 'trace 10.5');
1367+
return assertLabelsCorrect(null, [103, 100], 'trace 10.5', 'animation/update 1');
13311368
}).then(function() {
13321369
// Restyle to move the point out of the window:
13331370
return Plotly.relayout(gd, {'xaxis.range': [2, 3]});
13341371
}).then(function() {
13351372
// Assert label removed:
1336-
return assertLabelsCorrect(null, null);
1373+
return assertLabelsCorrect(null, null, null, 'animation/update 2');
13371374
}).then(function() {
13381375
// Move back to the original xaxis range:
13391376
return Plotly.relayout(gd, {'xaxis.range': [0, 1]});
13401377
}).then(function() {
13411378
// Assert label restored:
1342-
return assertLabelsCorrect(null, [103, 100], 'trace 10.5');
1379+
return assertLabelsCorrect(null, [103, 100], 'trace 10.5', 'animation/update 3');
13431380
}).catch(fail).then(done);
13441381
});
13451382

13461383
it('should not trigger infinite loop of plotly_unhover events', function(done) {
13471384
var gd = createGraphDiv();
1348-
var colors0 = ['#00000', '#00000', '#00000', '#00000', '#00000', '#00000', '#00000'];
1385+
var colors0 = ['#000000', '#000000', '#000000', '#000000', '#000000', '#000000', '#000000'];
13491386

13501387
function unhover() {
13511388
return new Promise(function(resolve) {
@@ -1365,7 +1402,7 @@ describe('hover updates', function() {
13651402
y: [1, 2, 3, 2, 3, 4, 3],
13661403
marker: {
13671404
size: 16,
1368-
colors: colors0.slice()
1405+
color: colors0.slice()
13691406
}
13701407
}])
13711408
.then(function() {
@@ -1383,14 +1420,14 @@ describe('hover updates', function() {
13831420
Plotly.restyle(gd, 'marker.color', [colors0.slice()]);
13841421
});
13851422

1386-
return assertLabelsCorrect([351, 251], [358, 272], '2');
1423+
return assertLabelsCorrect([351, 251], [358, 272], '2', 'events 0');
13871424
})
13881425
.then(unhover)
13891426
.then(function() {
13901427
expect(hoverCnt).toEqual(1);
13911428
expect(unHoverCnt).toEqual(1);
13921429

1393-
return assertLabelsCorrect([400, 200], [435, 198], '3');
1430+
return assertLabelsCorrect([420, 100], [435, 198], '3', 'events 1');
13941431
})
13951432
.then(unhover)
13961433
.then(function() {

0 commit comments

Comments
 (0)