Merge pull request #2218 from plotly/scatter-bar-hover-fix

alexcjohnson · web-flow · commit 363180bc2aea · 2017-12-21T10:14:36.000-05:00
scatter & bar hover fixes for #780
diff --git a/src/traces/bar/hover.js b/src/traces/bar/hover.js
@@ -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) {
@@ -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
@@ -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
@@ -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
@@ -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() {

Original file line number	Diff line number	Diff line change
`@@ -227,6 +227,7 @@ function setOffsetAndWidth(gd, pa, sieve) {`
`227`	`227`	`t.barwidth = barWidth;`
`228`	`228`	`t.poffset = offsetFromCenter;`
`229`	`229`	`t.bargroupwidth = barGroupWidth;`
	`230`	`+ t.bardelta = minDiff;`
`230`	`231`	`}`
`231`	`232`
`232`	`233`	`// stack bars that only differ by rounding`
`@@ -277,6 +278,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {`
`277`	`278`	`t.barwidth = barWidth;`
`278`	`279`	`t.poffset = offsetFromCenter;`
`279`	`280`	`t.bargroupwidth = barGroupWidth;`
	`281`	`+ t.bardelta = minDiff;`
`280`	`282`	`}`
`281`	`283`
`282`	`284`	`// stack bars that only differ by rounding`