Skip to content

Commit ce06ea9

Browse files
alexcjohnsonalexcjohnson
authored
Merge pull request #1527 from plotly/bar-width-hover
Fix hover label positioning for bar trace with set `width`
2 parents b19a87f + 06097d7 commit ce06ea9

File tree

4 files changed

+147
-38
lines changed

4 files changed

+147
-38
lines changed

‎src/traces/bar/hover.js

+44-28
Original file line numberDiff line numberDiff line change
@@ -15,50 +15,66 @@ var Color = require('../../components/color');
1515

1616

1717
module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
18-
var cd = pointData.cd,
19-
trace = cd[0].trace,
20-
t = cd[0].t,
21-
xa = pointData.xa,
22-
ya = pointData.ya,
23-
barDelta = (hovermode === 'closest') ?
24-
t.barwidth / 2 :
25-
t.bargroupwidth / 2,
26-
barPos;
27-
28-
if(hovermode !== 'closest') barPos = function(di) { return di.p; };
29-
else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
30-
else barPos = function(di) { return di.x; };
31-
32-
var dx, dy;
18+
var cd = pointData.cd;
19+
var trace = cd[0].trace;
20+
var t = cd[0].t;
21+
var xa = pointData.xa;
22+
var ya = pointData.ya;
23+
24+
var posVal, thisBarMinPos, thisBarMaxPos, minPos, maxPos, dx, dy;
25+
26+
var positionFn = function(di) {
27+
return Fx.inbox(minPos(di) - posVal, maxPos(di) - posVal);
28+
};
29+
3330
if(trace.orientation === 'h') {
31+
posVal = yval;
32+
thisBarMinPos = function(di) { return di.y - di.w / 2; };
33+
thisBarMaxPos = function(di) { return di.y + di.w / 2; };
3434
dx = function(di) {
3535
// add a gradient so hovering near the end of a
3636
// bar makes it a little closer match
3737
return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
3838
};
39-
dy = function(di) {
40-
var centerPos = barPos(di) - yval;
41-
return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
42-
};
39+
dy = positionFn;
4340
}
4441
else {
42+
posVal = xval;
43+
thisBarMinPos = function(di) { return di.x - di.w / 2; };
44+
thisBarMaxPos = function(di) { return di.x + di.w / 2; };
4545
dy = function(di) {
4646
return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
4747
};
48-
dx = function(di) {
49-
var centerPos = barPos(di) - xval;
50-
return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
51-
};
48+
dx = positionFn;
5249
}
5350

51+
minPos = (hovermode === 'closest') ?
52+
thisBarMinPos :
53+
function(di) {
54+
/*
55+
* In compare mode, accept a bar if you're on it *or* its group.
56+
* Nearly always it's the group that matters, but in case the bar
57+
* was explicitly set wider than its group we'd better accept the
58+
* whole bar.
59+
*/
60+
return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
61+
};
62+
63+
maxPos = (hovermode === 'closest') ?
64+
thisBarMaxPos :
65+
function(di) {
66+
return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
67+
};
68+
5469
var distfn = Fx.getDistanceFunction(hovermode, dx, dy);
5570
Fx.getClosest(cd, distfn, pointData);
5671

5772
// skip the rest (for this trace) if we didn't find a close point
5873
if(pointData.index === false) return;
5974

6075
// the closest data point
61-
var di = cd[pointData.index],
76+
var index = pointData.index,
77+
di = cd[index],
6278
mc = di.mcc || trace.marker.color,
6379
mlc = di.mlcc || trace.marker.line.color,
6480
mlw = di.mlw || trace.marker.line.width;
@@ -70,16 +86,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
7086
pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
7187
pointData.xLabelVal = size;
7288

73-
pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
74-
pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
89+
pointData.y0 = ya.c2p(minPos(di), true);
90+
pointData.y1 = ya.c2p(maxPos(di), true);
7591
pointData.yLabelVal = di.p;
7692
}
7793
else {
7894
pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
7995
pointData.yLabelVal = size;
8096

81-
pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
82-
pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
97+
pointData.x0 = xa.c2p(minPos(di), true);
98+
pointData.x1 = xa.c2p(maxPos(di), true);
8399
pointData.xLabelVal = di.p;
84100
}
85101

‎src/traces/bar/plot.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
4848
var t = d[0].t,
4949
trace = d[0].trace,
5050
poffset = t.poffset,
51-
poffsetIsArray = Array.isArray(poffset),
52-
barwidth = t.barwidth,
53-
barwidthIsArray = Array.isArray(barwidth);
51+
poffsetIsArray = Array.isArray(poffset);
5452

5553
d3.select(this).selectAll('g.point')
5654
.data(Lib.identity)
@@ -61,7 +59,7 @@ module.exports = function plot(gd, plotinfo, cdbar) {
6159
// log values go off-screen by plotwidth
6260
// so you see them continue if you drag the plot
6361
var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
64-
p1 = p0 + ((barwidthIsArray) ? barwidth[i] : barwidth),
62+
p1 = p0 + di.w,
6563
s0 = di.b,
6664
s1 = s0 + di.s;
6765

‎src/traces/bar/set_positions.js

+6-4
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
236236
applyAttributes(sieve);
237237

238238
// store the bar center in each calcdata item
239-
setBarCenter(gd, pa, sieve);
239+
setBarCenterAndWidth(gd, pa, sieve);
240240

241241
// update position axes
242242
updatePositionAxis(gd, pa, sieve);
@@ -286,7 +286,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
286286
applyAttributes(sieve);
287287

288288
// store the bar center in each calcdata item
289-
setBarCenter(gd, pa, sieve);
289+
setBarCenterAndWidth(gd, pa, sieve);
290290

291291
// update position axes
292292
updatePositionAxis(gd, pa, sieve, overlap);
@@ -377,7 +377,7 @@ function applyAttributes(sieve) {
377377
}
378378

379379

380-
function setBarCenter(gd, pa, sieve) {
380+
function setBarCenterAndWidth(gd, pa, sieve) {
381381
var calcTraces = sieve.traces,
382382
pLetter = getAxisLetter(pa);
383383

@@ -392,9 +392,11 @@ function setBarCenter(gd, pa, sieve) {
392392
for(var j = 0; j < calcTrace.length; j++) {
393393
var calcBar = calcTrace[j];
394394

395+
// store the actual bar width and position, for use by hover
396+
var width = calcBar.w = (barwidthIsArray) ? barwidth[j] : barwidth;
395397
calcBar[pLetter] = calcBar.p +
396398
((poffsetIsArray) ? poffset[j] : poffset) +
397-
((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
399+
width / 2;
398400

399401

400402
}

‎test/jasmine/tests/bar_test.js

+95-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ var Axes = PlotlyInternal.Axes;
1010
var createGraphDiv = require('../assets/create_graph_div');
1111
var destroyGraphDiv = require('../assets/destroy_graph_div');
1212
var customMatchers = require('../assets/custom_matchers');
13+
var failTest = require('../assets/fail_test');
1314

1415
describe('Bar.supplyDefaults', function() {
1516
'use strict';
@@ -1207,9 +1208,12 @@ describe('bar hover', function() {
12071208
};
12081209
}
12091210

1210-
function _hover(gd, xval, yval, closest) {
1211+
function _hover(gd, xval, yval, hovermode) {
12111212
var pointData = getPointData(gd);
1212-
var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
1213+
var pts = Bar.hoverPoints(pointData, xval, yval, hovermode);
1214+
if(!pts) return false;
1215+
1216+
var pt = pts[0];
12131217

12141218
return {
12151219
style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
@@ -1290,6 +1294,95 @@ describe('bar hover', function() {
12901294
});
12911295
});
12921296

1297+
describe('with special width/offset combinations', function() {
1298+
1299+
beforeEach(function() {
1300+
gd = createGraphDiv();
1301+
});
1302+
1303+
it('should return correct hover data (single bar, trace width)', function(done) {
1304+
Plotly.plot(gd, [{
1305+
type: 'bar',
1306+
x: [1],
1307+
y: [2],
1308+
width: 10,
1309+
marker: { color: 'red' }
1310+
}], {
1311+
xaxis: { range: [-200, 200] }
1312+
})
1313+
.then(function() {
1314+
// all these x, y, hovermode should give the same (the only!) hover label
1315+
[
1316+
[0, 0, 'closest'],
1317+
[-3.9, 1, 'closest'],
1318+
[5.9, 1.9, 'closest'],
1319+
[-3.9, -10, 'x'],
1320+
[5.9, 19, 'x']
1321+
].forEach(function(hoverSpec) {
1322+
var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);
1323+
1324+
expect(out.style).toEqual([0, 'red', 1, 2], hoverSpec);
1325+
assertPos(out.pos, [264, 278, 14, 14], hoverSpec);
1326+
});
1327+
1328+
// then a few that are off the edge so yield nothing
1329+
[
1330+
[1, -0.1, 'closest'],
1331+
[1, 2.1, 'closest'],
1332+
[-4.1, 1, 'closest'],
1333+
[6.1, 1, 'closest'],
1334+
[-4.1, 1, 'x'],
1335+
[6.1, 1, 'x']
1336+
].forEach(function(hoverSpec) {
1337+
var out = _hover(gd, hoverSpec[0], hoverSpec[1], hoverSpec[2]);
1338+
1339+
expect(out).toBe(false, hoverSpec);
1340+
});
1341+
})
1342+
.catch(failTest)
1343+
.then(done);
1344+
});
1345+
1346+
it('should return correct hover data (two bars, array width)', function(done) {
1347+
Plotly.plot(gd, [{
1348+
type: 'bar',
1349+
x: [1, 200],
1350+
y: [2, 1],
1351+
width: [10, 20],
1352+
marker: { color: 'red' }
1353+
}, {
1354+
type: 'bar',
1355+
x: [1, 200],
1356+
y: [1, 2],
1357+
width: [20, 10],
1358+
marker: { color: 'green' }
1359+
}], {
1360+
xaxis: { range: [-200, 300] },
1361+
width: 500,
1362+
height: 500
1363+
})
1364+
.then(function() {
1365+
var out = _hover(gd, -36, 1.5, 'closest');
1366+
1367+
expect(out.style).toEqual([0, 'red', 1, 2]);
1368+
assertPos(out.pos, [99, 106, 13, 13]);
1369+
1370+
out = _hover(gd, 164, 0.8, 'closest');
1371+
1372+
expect(out.style).toEqual([1, 'red', 200, 1]);
1373+
assertPos(out.pos, [222, 235, 168, 168]);
1374+
1375+
out = _hover(gd, 125, 0.8, 'x');
1376+
1377+
expect(out.style).toEqual([1, 'red', 200, 1]);
1378+
assertPos(out.pos, [203, 304, 168, 168]);
1379+
})
1380+
.catch(failTest)
1381+
.then(done);
1382+
});
1383+
1384+
});
1385+
12931386
});
12941387

12951388
function mockBarPlot(dataWithoutTraceType, layout) {

0 commit comments

Comments
 (0)