Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixup bar spike distance #5542

Merged
merged 4 commits into from
Mar 16, 2021
Merged

Fixup bar spike distance #5542

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e0d980
refactor setting Infinity for hoverdistance and spikedistance
archmoj Mar 9, 2021
6ee556c
use spikedistance in bar spikes
archmoj Mar 9, 2021
803d5a7
add test to lock issue 4716
archmoj Mar 10, 2021
05c1c85
pass in distance as the third argument
archmoj Mar 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/components/fx/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand Down
42 changes: 33 additions & 9 deletions src/traces/bar/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the only difference between spikePositionFn and hoverPositionFn is which max*Distance is used, just make that a 3rd arg to the _positionFn calls below.

Same with sizeFn vs thisBarSizeFn, looks like that could be a higher-order function like sizeFn(whichMaxDist)(di)

}

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;
Expand Down Expand Up @@ -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);
Expand Down
91 changes: 89 additions & 2 deletions test/jasmine/tests/hover_spikeline_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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);
});
Expand Down