From 28161d8266c6ab49cc5a19df6f7fa6bf5c03a85a Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 20 Dec 2017 11:40:58 -0500 Subject: [PATCH] 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 --- src/traces/bar/hover.js | 27 +++++++++-- src/traces/bar/set_positions.js | 2 + src/traces/scatter/hover.js | 45 +++++++++++------- test/jasmine/tests/hover_label_test.js | 65 ++++++++++++++++++++------ 4 files changed, 103 insertions(+), 36 deletions(-) diff --git a/src/traces/bar/hover.js b/src/traces/bar/hover.js index bea195126a9..cbba9d9b87c 100644 --- a/src/traces/bar/hover.js +++ b/src/traces/bar/hover.js @@ -18,13 +18,14 @@ 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) { /* @@ -32,14 +33,20 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { * 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) { @@ -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]; diff --git a/src/traces/bar/set_positions.js b/src/traces/bar/set_positions.js index 76e36a95516..e929b4f190d 100644 --- a/src/traces/bar/set_positions.js +++ b/src/traces/bar/set_positions.js @@ -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 @@ -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 diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js index 601a30c2e47..127af16efc4 100644 --- a/src/traces/scatter/hover.js +++ b/src/traces/scatter/hover.js @@ -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); diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index dcae95e16a0..74238e593df 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -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'; @@ -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]); @@ -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); } @@ -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]}], @@ -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) { @@ -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() { @@ -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() {