Skip to content

Commit

Permalink
Merge pull request #2218 from plotly/scatter-bar-hover-fix
Browse files Browse the repository at this point in the history
scatter & bar hover fixes for #780
  • Loading branch information
alexcjohnson authored Dec 21, 2017
2 parents 82f4c16 + 28161d8 commit 363180b
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 36 deletions.
27 changes: 23 additions & 4 deletions src/traces/bar/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,35 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
var cd = pointData.cd;
var trace = cd[0].trace;
var t = cd[0].t;
var isClosest = (hovermode === 'closest');

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

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

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

var maxPos = (hovermode === 'closest') ?
var maxPos = isClosest ?
thisBarMaxPos :
function(di) {
return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
return Math.max(thisBarMaxPos(di), di.p + t.bardelta / 2);
};

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

// if we get here and we're not in 'closest' mode, push min/max pos back
// onto the group - even though that means occasionally the mouse will be
// over the hover label.
if(!isClosest) {
minPos = function(di) {
return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
};
maxPos = function(di) {
return Math.max(thisBarMaxPos(di), di.p + t.bargroupwidth / 2);
};
}

// the closest data point
var index = pointData.index;
var di = cd[index];
Expand Down
2 changes: 2 additions & 0 deletions src/traces/bar/set_positions.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ function setOffsetAndWidth(gd, pa, sieve) {
t.barwidth = barWidth;
t.poffset = offsetFromCenter;
t.bargroupwidth = barGroupWidth;
t.bardelta = minDiff;
}

// stack bars that only differ by rounding
Expand Down Expand Up @@ -277,6 +278,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
t.barwidth = barWidth;
t.poffset = offsetFromCenter;
t.bargroupwidth = barGroupWidth;
t.bardelta = minDiff;
}

// stack bars that only differ by rounding
Expand Down
45 changes: 27 additions & 18 deletions src/traces/scatter/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,44 @@ var fillHoverText = require('./fill_hover_text');
var MAXDIST = Fx.constants.MAXDIST;

module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
var cd = pointData.cd,
trace = cd[0].trace,
xa = pointData.xa,
ya = pointData.ya,
xpx = xa.c2p(xval),
ypx = ya.c2p(yval),
pt = [xpx, ypx],
hoveron = trace.hoveron || '';
var cd = pointData.cd;
var trace = cd[0].trace;
var xa = pointData.xa;
var ya = pointData.ya;
var xpx = xa.c2p(xval);
var ypx = ya.c2p(yval);
var pt = [xpx, ypx];
var hoveron = trace.hoveron || '';
var minRad = (trace.mode.indexOf('markers') !== -1) ? 3 : 0.5;

// look for points to hover on first, then take fills only if we
// didn't find a point
if(hoveron.indexOf('points') !== -1) {
var dx = function(di) {
// scatter points: d.mrc is the calculated marker radius
// adjust the distance so if you're inside the marker it
// always will show up regardless of point size, but
// prioritize smaller points
// dx and dy are used in compare modes - here we want to always
// prioritize the closest data point, at least as long as markers are
// the same size or nonexistent, but still try to prioritize small markers too.
var rad = Math.max(3, di.mrc || 0);
return Math.max(Math.abs(xa.c2p(di.x) - xpx) - rad, 1 - 3 / rad);
var kink = 1 - 1 / rad;
var dxRaw = Math.abs(xa.c2p(di.x) - xpx);
var d = (dxRaw < rad) ? (kink * dxRaw / rad) : (dxRaw - rad + kink);
return d;
},
dy = function(di) {
var rad = Math.max(3, di.mrc || 0);
return Math.max(Math.abs(ya.c2p(di.y) - ypx) - rad, 1 - 3 / rad);
var kink = 1 - 1 / rad;
var dyRaw = Math.abs(ya.c2p(di.y) - ypx);
return (dyRaw < rad) ? (kink * dyRaw / rad) : (dyRaw - rad + kink);
},
dxy = function(di) {
var rad = Math.max(3, di.mrc || 0),
dx = xa.c2p(di.x) - xpx,
dy = ya.c2p(di.y) - ypx;
return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad);
// scatter points: d.mrc is the calculated marker radius
// adjust the distance so if you're inside the marker it
// always will show up regardless of point size, but
// prioritize smaller points
var rad = Math.max(minRad, di.mrc || 0);
var dx = xa.c2p(di.x) - xpx;
var dy = ya.c2p(di.y) - ypx;
return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - minRad / rad);
},
distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy);

Expand Down
65 changes: 51 additions & 14 deletions test/jasmine/tests/hover_label_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1108,6 +1108,43 @@ describe('hover info on stacked subplots', function() {
});


describe('hover on many lines+bars', function() {
'use strict';

afterEach(destroyGraphDiv);

it('shows hover info for both traces', function(done) {
// see https://github.com/plotly/plotly.js/issues/780
var values = new Array(1000);
var values2 = new Array(values.length);
for(var i = 0; i < values.length; i++) {
values[i] = i;
values2[i] = i * 2;
}

var gd = createGraphDiv();
Plotly.newPlot(gd, [
{y: values2},
{y: values, type: 'bar'}
], {
width: 400,
height: 400,
margin: {l: 100, r: 100, t: 100, b: 100}
})
.then(function() {
Lib.clearThrottle();
mouseEvent('mousemove', 200, 100);
Lib.clearThrottle();

expect(d3.select(gd).selectAll('g.hovertext').size()).toBe(2);
expect(d3.select(gd).selectAll('g.axistext').size()).toBe(1);
})
.catch(fail)
.then(done);
});
});


describe('hover info on overlaid subplots', function() {
'use strict';

Expand Down Expand Up @@ -1274,7 +1311,7 @@ describe('hover updates', function() {

afterEach(destroyGraphDiv);

function assertLabelsCorrect(mousePos, labelPos, labelText) {
function assertLabelsCorrect(mousePos, labelPos, labelText, msg) {
return new Promise(function(resolve) {
if(mousePos) {
mouseEvent('mousemove', mousePos[0], mousePos[1]);
Expand All @@ -1283,14 +1320,14 @@ describe('hover updates', function() {
setTimeout(function() {
var hoverText = d3.selectAll('g.hovertext');
if(labelPos) {
expect(hoverText.size()).toEqual(1);
expect(hoverText.text()).toEqual(labelText);
expect(hoverText.size()).toBe(1, msg);
expect(hoverText.text()).toBe(labelText, msg);

var transformParts = hoverText.attr('transform').split('(');
expect(transformParts[0]).toEqual('translate');
expect(transformParts[0]).toBe('translate', msg);
var transformCoords = transformParts[1].split(')')[0].split(',');
expect(+transformCoords[0]).toBeCloseTo(labelPos[0], -1, labelText + ':x');
expect(+transformCoords[1]).toBeCloseTo(labelPos[1], -1, labelText + ':y');
expect(+transformCoords[0]).toBeCloseTo(labelPos[0], -1, labelText + ':x ' + msg);
expect(+transformCoords[1]).toBeCloseTo(labelPos[1], -1, labelText + ':y ' + msg);
} else {
expect(hoverText.size()).toEqual(0);
}
Expand Down Expand Up @@ -1318,7 +1355,7 @@ describe('hover updates', function() {
var gd = createGraphDiv();
Plotly.plot(gd, mock).then(function() {
// The label text gets concatenated together when queried. Such is life.
return assertLabelsCorrect([100, 100], [103, 100], 'trace 00.5');
return assertLabelsCorrect([100, 100], [103, 100], 'trace 00.5', 'animation/update 0');
}).then(function() {
return Plotly.animate(gd, [{
data: [{x: [0], y: [0]}, {x: [0.5], y: [0.5]}],
Expand All @@ -1327,25 +1364,25 @@ describe('hover updates', function() {
}).then(function() {
// No mouse event this time. Just change the data and check the label.
// Ditto on concatenation. This is "trace 1" + "0.5"
return assertLabelsCorrect(null, [103, 100], 'trace 10.5');
return assertLabelsCorrect(null, [103, 100], 'trace 10.5', 'animation/update 1');
}).then(function() {
// Restyle to move the point out of the window:
return Plotly.relayout(gd, {'xaxis.range': [2, 3]});
}).then(function() {
// Assert label removed:
return assertLabelsCorrect(null, null);
return assertLabelsCorrect(null, null, null, 'animation/update 2');
}).then(function() {
// Move back to the original xaxis range:
return Plotly.relayout(gd, {'xaxis.range': [0, 1]});
}).then(function() {
// Assert label restored:
return assertLabelsCorrect(null, [103, 100], 'trace 10.5');
return assertLabelsCorrect(null, [103, 100], 'trace 10.5', 'animation/update 3');
}).catch(fail).then(done);
});

it('should not trigger infinite loop of plotly_unhover events', function(done) {
var gd = createGraphDiv();
var colors0 = ['#00000', '#00000', '#00000', '#00000', '#00000', '#00000', '#00000'];
var colors0 = ['#000000', '#000000', '#000000', '#000000', '#000000', '#000000', '#000000'];

function unhover() {
return new Promise(function(resolve) {
Expand All @@ -1365,7 +1402,7 @@ describe('hover updates', function() {
y: [1, 2, 3, 2, 3, 4, 3],
marker: {
size: 16,
colors: colors0.slice()
color: colors0.slice()
}
}])
.then(function() {
Expand All @@ -1383,14 +1420,14 @@ describe('hover updates', function() {
Plotly.restyle(gd, 'marker.color', [colors0.slice()]);
});

return assertLabelsCorrect([351, 251], [358, 272], '2');
return assertLabelsCorrect([351, 251], [358, 272], '2', 'events 0');
})
.then(unhover)
.then(function() {
expect(hoverCnt).toEqual(1);
expect(unHoverCnt).toEqual(1);

return assertLabelsCorrect([400, 200], [435, 198], '3');
return assertLabelsCorrect([420, 100], [435, 198], '3', 'events 1');
})
.then(unhover)
.then(function() {
Expand Down

0 comments on commit 363180b

Please sign in to comment.