Skip to content

Fix hover label positioning for bar trace with set width #1527

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 5, 2017
Merged

Fix hover label positioning for bar trace with set width #1527

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7a4154f
make distance fn accept pt index as arg
etpinard Mar 17, 2017
c84e2a9
generalize bar delta computation
etpinard Mar 17, 2017
591bd9e
add two bar hover cases with set 'width'
etpinard Mar 17, 2017
b34d9ef
Merge branch 'master' into bar-width-hover
alexcjohnson Apr 4, 2017
30bc8c7
broaden bar hover acceptance, and test
alexcjohnson Apr 5, 2017
06097d7
shift new bar hover test for ci
alexcjohnson Apr 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/plots/cartesian/graph_interact.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,9 @@ function p2c(axArray, v) {
}

function quadrature(dx, dy) {
return function(di) {
var x = dx(di),
y = dy(di);
return function(di, i) {
var x = dx(di, i),
y = dy(di, i);
return Math.sqrt(x * x + y * y);
};
}
Expand Down Expand Up @@ -661,7 +661,7 @@ fx.getClosest = function(cd, distfn, pointData) {
// to create pre-sorted data (by x or y), not sure how to
// do this for 'closest'
for(var i = 0; i < cd.length; i++) {
var newDistance = distfn(cd[i]);
var newDistance = distfn(cd[i], i);
if(newDistance <= pointData.distance) {
pointData.index = i;
pointData.distance = newDistance;
Expand Down
32 changes: 18 additions & 14 deletions src/traces/bar/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,35 +19,38 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
trace = cd[0].trace,
t = cd[0].t,
xa = pointData.xa,
ya = pointData.ya,
barDelta = (hovermode === 'closest') ?
t.barwidth / 2 :
t.bargroupwidth / 2,
barPos;
ya = pointData.ya;

var barPos;
if(hovermode !== 'closest') barPos = function(di) { return di.p; };
else if(trace.orientation === 'h') barPos = function(di) { return di.y; };
else barPos = function(di) { return di.x; };

var barDelta = (hovermode === 'closest') ?
function(i) { return (t.barwidth[i] || t.barwidth) / 2; } :
function() { return t.bargroupwidth / 2; };

var dx, dy;
if(trace.orientation === 'h') {
dx = function(di) {
// add a gradient so hovering near the end of a
// bar makes it a little closer match
return Fx.inbox(di.b - xval, di.x - xval) + (di.x - xval) / (di.x - di.b);
};
dy = function(di) {
dy = function(di, i) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this (providing i as an extra arg) is the most flexible way to handle the issue for future extensibility, but the way that matches better with what we do in scatter - and should be slightly better perf - would be to include the width of each point in di

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Good call! Post #1519 this will be easy!

var centerPos = barPos(di) - yval;
return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
var delta = barDelta(i);
return Fx.inbox(centerPos - delta, centerPos + delta);
};
}
else {
dy = function(di) {
return Fx.inbox(di.b - yval, di.y - yval) + (di.y - yval) / (di.y - di.b);
};
dx = function(di) {
dx = function(di, i) {
var centerPos = barPos(di) - xval;
return Fx.inbox(centerPos - barDelta, centerPos + barDelta);
var delta = barDelta(i);
return Fx.inbox(centerPos - delta, centerPos + delta);
};
}

Expand All @@ -58,7 +61,8 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
if(pointData.index === false) return;

// the closest data point
var di = cd[pointData.index],
var index = pointData.index,
di = cd[index],
mc = di.mcc || trace.marker.color,
mlc = di.mlcc || trace.marker.line.color,
mlw = di.mlw || trace.marker.line.width;
Expand All @@ -70,16 +74,16 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
pointData.x0 = pointData.x1 = xa.c2p(di.x, true);
pointData.xLabelVal = size;

pointData.y0 = ya.c2p(barPos(di) - barDelta, true);
pointData.y1 = ya.c2p(barPos(di) + barDelta, true);
pointData.y0 = ya.c2p(barPos(di) - barDelta(index), true);
pointData.y1 = ya.c2p(barPos(di) + barDelta(index), true);
pointData.yLabelVal = di.p;
}
else {
pointData.y0 = pointData.y1 = ya.c2p(di.y, true);
pointData.yLabelVal = size;

pointData.x0 = xa.c2p(barPos(di) - barDelta, true);
pointData.x1 = xa.c2p(barPos(di) + barDelta, true);
pointData.x0 = xa.c2p(barPos(di) - barDelta(index), true);
pointData.x1 = xa.c2p(barPos(di) + barDelta(index), true);
pointData.xLabelVal = di.p;
}

Expand Down
64 changes: 62 additions & 2 deletions test/jasmine/tests/bar_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,9 +1191,9 @@ describe('bar hover', function() {
};
}

function _hover(gd, xval, yval, closest) {
function _hover(gd, xval, yval, hovermode) {
var pointData = getPointData(gd);
var pt = Bar.hoverPoints(pointData, xval, yval, closest)[0];
var pt = Bar.hoverPoints(pointData, xval, yval, hovermode)[0];

return {
style: [pt.index, pt.color, pt.xLabelVal, pt.yLabelVal],
Expand Down Expand Up @@ -1274,6 +1274,66 @@ describe('bar hover', function() {
});
});

describe('with special width/offset combinations', function() {

beforeEach(function() {
gd = createGraphDiv();
});

it('should return correct \'closest\' hover data (single bar, trace width)', function(done) {
Plotly.plot(gd, [{
type: 'bar',
x: [1],
y: [2],
width: 10,
marker: { color: 'red' }
}], {
xaxis: { range: [-200, 200] }
})
.then(function() {
var out = _hover(gd, 0, 0, 'closest');

expect(out.style).toEqual([0, 'red', 1, 2]);
assertPos(out.pos, [264, 278, 14, 14]);
})
.then(done);
});

it('should return correct \'closest\' hover data (two bars, array width)', function(done) {
Plotly.plot(gd, [{
type: 'bar',
x: [1, 200],
y: [2, 1],
width: [10, 20],
marker: { color: 'red' }
}, {
type: 'bar',
x: [1, 200],
y: [1, 2],
width: [20, 10],
marker: { color: 'green' }
}], {
xaxis: { range: [-200, 300] },
width: 500,
height: 500
})
.then(function() {
var out = _hover(gd, -36, 1.5, 'closest');

expect(out.style).toEqual([0, 'red', 1, 2]);
assertPos(out.pos, [99, 106, 13, 13]);
})
.then(function() {
var out = _hover(gd, 164, 0.8, 'closest');

expect(out.style).toEqual([1, 'red', 200, 1]);
assertPos(out.pos, [222, 235, 168, 168]);
})
.then(done);
});

});

});

function mockBarPlot(dataWithoutTraceType, layout) {
Expand Down