Skip to content

Commit

Permalink
scatter & bar hover fixes for #780
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@alexcjohnson
alexcjohnson committed Dec 20, 2017
1 parent 82f4c16 commit 28161d8
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
View file
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
View file
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
View file
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
View file
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 28161d8

Please sign in to comment.