From 7e0d980593b197ef18659d1289d1b882004a7694 Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 9 Mar 2021 14:14:41 -0500 Subject: [PATCH 1/4] refactor setting Infinity for hoverdistance and spikedistance --- src/components/fx/hover.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index bf0723787c5..8e345bd9836 100644 --- a/src/components/fx/hover.js +++ b/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 = []; From 6ee556c4a035a4cb4624bf1aea04b9d7a2b3e379 Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 9 Mar 2021 14:55:54 -0500 Subject: [PATCH 2/4] use spikedistance in bar spikes --- src/traces/bar/hover.js | 42 +++++++++++++++++----- test/jasmine/tests/hover_spikeline_test.js | 28 +++++++++++++-- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/traces/bar/hover.js b/src/traces/bar/hover.js index ac9ee706db8..909b983be1d 100644 --- a/src/traces/bar/hover.js +++ b/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,63 @@ function hoverOnBars(pointData, xval, yval, hovermode) { return Math.max(thisBarMaxPos(di), di.p + t.bardelta / 2); }; - function _positionFn(_minPos, _maxPos) { + function hoverPositionFn(_minPos, _maxPos) { // 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); } + function spikePositionFn(_minPos, _maxPos) { + // 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, + maxSpikeDistance + Math.min(1, Math.abs(_maxPos - _minPos) / pRangeCalc) - 1); + } + function positionFn(di) { - return _positionFn(minPos(di), maxPos(di)); + return hoverPositionFn(minPos(di), maxPos(di)); } function thisBarPositionFn(di) { - return _positionFn(thisBarMinPos(di), thisBarMaxPos(di)); + return spikePositionFn(thisBarMinPos(di), thisBarMaxPos(di)); } - 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 +182,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); diff --git a/test/jasmine/tests/hover_spikeline_test.js b/test/jasmine/tests/hover_spikeline_test.js index 7553721eef7..e9cac24acd0 100644 --- a/test/jasmine/tests/hover_spikeline_test.js +++ b/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,33 @@ 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); }); From 803d5a711c9ef42ecfe43c77ff1dcfd3b7f09a79 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 10 Mar 2021 10:17:05 -0500 Subject: [PATCH 3/4] add test to lock issue 4716 --- test/jasmine/tests/hover_spikeline_test.js | 63 ++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/test/jasmine/tests/hover_spikeline_test.js b/test/jasmine/tests/hover_spikeline_test.js index e9cac24acd0..ead962676d2 100644 --- a/test/jasmine/tests/hover_spikeline_test.js +++ b/test/jasmine/tests/hover_spikeline_test.js @@ -465,6 +465,69 @@ describe('spikeline hover', function() { .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); + }); + it('correctly responds to setting the spikedistance to 0 by disabling ' + 'the search for points to draw the spikelines', function(done) { var _mock = makeMock('toaxis', 'closest'); From 05c1c85bf8247f393d3a9c7b1301207765674404 Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 16 Mar 2021 09:01:52 -0400 Subject: [PATCH 4/4] pass in distance as the third argument --- src/traces/bar/hover.js | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/traces/bar/hover.js b/src/traces/bar/hover.js index 909b983be1d..ee0d226d027 100644 --- a/src/traces/bar/hover.js +++ b/src/traces/bar/hover.js @@ -62,26 +62,19 @@ function hoverOnBars(pointData, xval, yval, hovermode) { return Math.max(thisBarMaxPos(di), di.p + t.bardelta / 2); }; - function hoverPositionFn(_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); - } - - function spikePositionFn(_minPos, _maxPos) { - // 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, - maxSpikeDistance + Math.min(1, Math.abs(_maxPos - _minPos) / pRangeCalc) - 1); + maxDistance + Math.min(1, Math.abs(_maxPos - _minPos) / pRangeCalc) - 1); } function positionFn(di) { - return hoverPositionFn(minPos(di), maxPos(di)); + return inbox(minPos(di), maxPos(di), maxHoverDistance); } function thisBarPositionFn(di) { - return spikePositionFn(thisBarMinPos(di), thisBarMaxPos(di)); + return inbox(thisBarMinPos(di), thisBarMaxPos(di), maxSpikeDistance); } function getSize(di) {