Merge pull request #5542 from plotly/fixup-bar-spike-dist

Fixup bar spike distance

Author: archmoj

src/components/fx/hover.js

@@ -248,8 +248,11 @@ function _hover(gd, evt, subplot, noHoverEvent) {
         return dragElement.unhoverRaw(gd, evt);
     }
 
-    var hoverdistance = fullLayout.hoverdistance === -1 ? Infinity : fullLayout.hoverdistance;
-    var spikedistance = fullLayout.spikedistance === -1 ? Infinity : fullLayout.spikedistance;
+    var hoverdistance = fullLayout.hoverdistance;
+    if(hoverdistance === -1) hoverdistance = Infinity;
+
+    var spikedistance = fullLayout.spikedistance;
+    if(spikedistance === -1) spikedistance = Infinity;
 
     // hoverData: the set of candidate points we've found to highlight
     var hoverData = [];

src/traces/bar/hover.js

@@ -31,6 +31,7 @@ function hoverOnBars(pointData, xval, yval, hovermode) {
     var isClosest = (hovermode === 'closest');
     var isWaterfall = (trace.type === 'waterfall');
     var maxHoverDistance = pointData.maxHoverDistance;
+    var maxSpikeDistance = pointData.maxSpikeDistance;
 
     var posVal, sizeVal, posLetter, sizeLetter, dx, dy, pRangeCalc;
 
@@ -61,40 +62,56 @@ function hoverOnBars(pointData, xval, yval, hovermode) {
             return Math.max(thisBarMaxPos(di), di.p + t.bardelta / 2);
         };
 
-    function _positionFn(_minPos, _maxPos) {
+    function inbox(_minPos, _maxPos, maxDistance) {
         // add a little to the pseudo-distance for wider bars, so that like scatter,
         // if you are over two overlapping bars, the narrower one wins.
         return Fx.inbox(_minPos - posVal, _maxPos - posVal,
-            maxHoverDistance + Math.min(1, Math.abs(_maxPos - _minPos) / pRangeCalc) - 1);
+            maxDistance + Math.min(1, Math.abs(_maxPos - _minPos) / pRangeCalc) - 1);
     }
 
     function positionFn(di) {
-        return _positionFn(minPos(di), maxPos(di));
+        return inbox(minPos(di), maxPos(di), maxHoverDistance);
     }
 
     function thisBarPositionFn(di) {
-        return _positionFn(thisBarMinPos(di), thisBarMaxPos(di));
+        return inbox(thisBarMinPos(di), thisBarMaxPos(di), maxSpikeDistance);
     }
 
-    function sizeFn(di) {
-        var v = sizeVal;
-        var b = di.b;
+    function getSize(di) {
         var s = di[sizeLetter];
 
         if(isWaterfall) {
             var rawS = Math.abs(di.rawS) || 0;
-            if(v > 0) {
+            if(sizeVal > 0) {
                 s += rawS;
-            } else if(v < 0) {
+            } else if(sizeVal < 0) {
                 s -= rawS;
             }
         }
 
+        return s;
+    }
+
+    function sizeFn(di) {
+        var v = sizeVal;
+        var b = di.b;
+        var s = getSize(di);
+
         // add a gradient so hovering near the end of a
         // bar makes it a little closer match
         return Fx.inbox(b - v, s - v, maxHoverDistance + (s - v) / (s - b) - 1);
     }
 
+    function thisBarSizeFn(di) {
+        var v = sizeVal;
+        var b = di.b;
+        var s = getSize(di);
+
+        // add a gradient so hovering near the end of a
+        // bar makes it a little closer match
+        return Fx.inbox(b - v, s - v, maxSpikeDistance + (s - v) / (s - b) - 1);
+    }
+
     if(trace.orientation === 'h') {
         posVal = yval;
         sizeVal = xval;
@@ -158,7 +175,7 @@ function hoverOnBars(pointData, xval, yval, hovermode) {
     pointData.baseLabel = hoverLabelText(sa, di.b);
 
     // spikelines always want "closest" distance regardless of hovermode
-    pointData.spikeDistance = (sizeFn(di) + thisBarPositionFn(di)) / 2 - maxHoverDistance;
+    pointData.spikeDistance = (thisBarSizeFn(di) + thisBarPositionFn(di)) / 2;
     // they also want to point to the data value, regardless of where the label goes
     // in case of bars shifted within groups
     pointData[posLetter + 'Spike'] = pa.c2p(di.p, true);

test/jasmine/tests/hover_spikeline_test.js

@@ -419,12 +419,13 @@ describe('spikeline hover', function() {
         .then(done, done.fail);
     });
 
-    it('correctly select the closest bar even when setting spikedistance to -1', function(done) {
+    it('correctly select the closest bar even when setting spikedistance to -1 (case of x hovermode)', function(done) {
         var mock = require('@mocks/bar_stack-with-gaps');
         var mockCopy = Lib.extendDeep({}, mock);
         mockCopy.layout.xaxis.showspikes = true;
         mockCopy.layout.yaxis.showspikes = true;
         mockCopy.layout.spikedistance = -1;
+        mockCopy.layout.hovermode = 'x';
 
         Plotly.newPlot(gd, mockCopy)
         .then(function() {
@@ -433,10 +434,96 @@ describe('spikeline hover', function() {
             expect(lines.size()).toBe(4);
             expect(lines[0][1].getAttribute('stroke')).toBe('#2ca02c');
 
-            _hover({xpx: 600, ypx: 200});
+            _hover({xpx: 600, ypx: 100});
             lines = d3SelectAll('line.spikeline');
             expect(lines.size()).toBe(4);
+            expect(lines[0][1].getAttribute('stroke')).toBe('#2ca02c');
+        })
+        .then(done, done.fail);
+    });
+
+    it('correctly select the closest bar even when setting spikedistance to -1 (case of closest hovermode)', function(done) {
+        var mock = require('@mocks/bar_stack-with-gaps');
+        var mockCopy = Lib.extendDeep({}, mock);
+        mockCopy.layout.xaxis.showspikes = true;
+        mockCopy.layout.yaxis.showspikes = true;
+        mockCopy.layout.spikedistance = -1;
+        mockCopy.layout.hovermode = 'closest';
+
+        Plotly.newPlot(gd, mockCopy)
+        .then(function() {
+            _hover({xpx: 600, ypx: 400});
+            var lines = d3SelectAll('line.spikeline');
+            expect(lines.size()).toBe(4);
             expect(lines[0][1].getAttribute('stroke')).toBe('#1f77b4');
+
+            _hover({xpx: 600, ypx: 100});
+            lines = d3SelectAll('line.spikeline');
+            expect(lines.size()).toBe(4);
+            expect(lines[0][1].getAttribute('stroke')).toBe('#2ca02c');
+        })
+        .then(done, done.fail);
+    });
+
+    it('could select the closest scatter point inside bar', function(done) {
+        Plotly.newPlot(gd, {
+            data: [{
+                type: 'scatter',
+                marker: { color: 'green' },
+                x: [
+                    -1,
+                    0,
+                    0.5,
+                    1
+                ],
+                y: [
+                    0.1,
+                    0.2,
+                    0.25,
+                    0.3
+                ]
+            },
+            {
+                type: 'bar',
+                marker: { color: 'blue' },
+                x: [
+                    -1,
+                    -0.2,
+                    1
+                ],
+                y: [
+                    1,
+                    2,
+                    0.5
+                ]
+            }],
+            layout: {
+                hovermode: 'x',
+                xaxis: { showspikes: true },
+                yaxis: { showspikes: true },
+                showlegend: false,
+                width: 500,
+                height: 500,
+                margin: {
+                    t: 50,
+                    b: 50,
+                    l: 50,
+                    r: 50,
+                }
+            }
+        })
+        .then(function() {
+            var lines;
+
+            _hover({xpx: 200, ypx: 200});
+            lines = d3SelectAll('line.spikeline');
+            expect(lines.size()).toBe(4);
+            expect(lines[0][1].getAttribute('stroke')).toBe('blue');
+
+            _hover({xpx: 200, ypx: 350});
+            lines = d3SelectAll('line.spikeline');
+            expect(lines.size()).toBe(4);
+            expect(lines[0][1].getAttribute('stroke')).toBe('green');
         })
         .then(done, done.fail);
     });