Skip to content

Fix scatter and scattergl hover on axes with period and adjust the unified hover position in respect to mean #5836

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

Merged
merged 6 commits into from
Jul 20, 2021
Merged

Fix scatter and scattergl hover on axes with period and adjust the unified hover position in respect to mean #5836

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
89f7f13
fix 5822 - period scatter hover positions
archmoj Jul 19, 2021
7d6ee37
draft log for PR 5836
archmoj Jul 19, 2021
53a67bc
Merge remote-tracking branch 'origin/master' into fix5822-hover-position
archmoj Jul 19, 2021
b44e11d
pick winning and median positions for unified hover depending on axis
archmoj Jul 19, 2021
1dce356
update PR log
archmoj Jul 19, 2021
17215eb
speed up the test
archmoj Jul 20, 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
1 change: 1 addition & 0 deletions draftlogs/5836_fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix `scatter` and `scattergl` hover on axes with period and adjust the unified hover position in respect to mean [[#5836](https://github.com/plotly/plotly.js/pull/5836)]
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 @@ -1072,8 +1072,11 @@ function createHoverText(hoverData, opts, gd) {

// Position the hover
var winningPoint = hoverData[0];
var ly = (winningPoint.y0 + winningPoint.y1) / 2;
var lx = (winningPoint.x0 + winningPoint.x1) / 2;
var ly = axLetter === 'y' ?
(winningPoint.y0 + winningPoint.y1) / 2 : Lib.mean(hoverData.map(function(c) {return (c.y0 + c.y1) / 2;}));
var lx = axLetter === 'x' ?
(winningPoint.x0 + winningPoint.x1) / 2 : Lib.mean(hoverData.map(function(c) {return (c.x0 + c.x1) / 2;}));

var legendContainer = container.select('g.legend');
var tbb = legendContainer.node().getBoundingClientRect();
lx += xa._offset;
Expand Down
10 changes: 0 additions & 10 deletions src/traces/scatter/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,13 @@ function calc(gd, trace) {
calcAxisExpansion(gd, trace, xa, ya, x, y, ppad);
}

var hasPeriodX = !!trace.xperiodalignment;
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting that this PR is mostly removals :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Less is more :)

var hasPeriodY = !!trace.yperiodalignment;

for(i = 0; i < serieslen; i++) {
var cdi = cd[i] = {};
var xValid = isNumeric(x[i]);
var yValid = isNumeric(y[i]);
if(xValid && yValid) {
cdi[xAttr] = x[i];
cdi[yAttr] = y[i];

if(hasPeriodX) {
cdi.orig_x = origX[i]; // used by hover
}
if(hasPeriodY) {
cdi.orig_y = origY[i]; // used by hover
}
} else if(stackGroupOpts && (isV ? xValid : yValid)) {
// if we're stacking we need to hold on to all valid positions
// even with invalid sizes
Expand Down
6 changes: 2 additions & 4 deletions src/traces/scatter/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,12 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
var rad = Math.max(3, di.mrc || 0);
var kink = 1 - 1 / rad;
var dxRaw = Math.abs(xa.c2p(di.x) - xpx);
if(di.orig_x !== undefined) dxRaw += xa.c2p(di.orig_x) - xa.c2p(di.x);
return (dxRaw < rad) ? (kink * dxRaw / rad) : (dxRaw - rad + kink);
};
var dy = function(di) {
var rad = Math.max(3, di.mrc || 0);
var kink = 1 - 1 / rad;
var dyRaw = Math.abs(ya.c2p(di.y) - ypx);
if(di.orig_y !== undefined) dyRaw += ya.c2p(di.orig_y) - ya.c2p(di.y);
return (dyRaw < rad) ? (kink * dyRaw / rad) : (dyRaw - rad + kink);
};
var dxy = function(di) {
Expand Down Expand Up @@ -71,8 +69,8 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
// the normalized individual sizes, so that's what I'm doing here
// for now.
var sizeVal = orientation && (di.sNorm || di.s);
var xLabelVal = (orientation === 'h') ? sizeVal : di.orig_x !== undefined ? di.orig_x : di.x;
var yLabelVal = (orientation === 'v') ? sizeVal : di.orig_y !== undefined ? di.orig_y : di.y;
var xLabelVal = (orientation === 'h') ? sizeVal : di.x;
var yLabelVal = (orientation === 'v') ? sizeVal : di.y;

Lib.extendFlat(pointData, {
color: getTraceColor(trace, di),
Expand Down
9 changes: 2 additions & 7 deletions src/traces/scattergl/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,9 @@ function hoverPoints(pointData, xval, yval, hovermode) {
for(i = 0; i < ids.length; i++) {
ptx = x[ids[i]];
dx = Math.abs(xa.c2p(ptx) - xpx);
if(trace._origX && trace._origX[i] !== undefined) dx += xa.c2p(trace._origX[i]) - xa.c2p(ptx);
if(dx < minDist) {
minDist = dx;
dy = ya.c2p(y[ids[i]]) - ypx;
if(trace._origY && trace._origY[i] !== undefined) dy += ya.c2p(trace._origY[i]) - ya.c2p(pty);
dxy = Math.sqrt(dx * dx + dy * dy);
id = ids[i];
}
Expand Down Expand Up @@ -155,19 +153,16 @@ function calcHover(pointData, x, y, trace) {
var fakeCd = {};
fakeCd[pointData.index] = di;

var origX = trace._origX;
var origY = trace._origY;

var pointData2 = Lib.extendFlat({}, pointData, {
color: getTraceColor(trace, di),

x0: xp - rad,
x1: xp + rad,
xLabelVal: origX ? origX[id] : di.x,
xLabelVal: di.x,

y0: yp - rad,
y1: yp + rad,
yLabelVal: origY ? origY[id] : di.y,
yLabelVal: di.y,

cd: fakeCd,
distance: minDist,
Expand Down
40 changes: 40 additions & 0 deletions test/jasmine/tests/gl2d_click_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,9 @@ describe('hover with (x|y)period positioning', function() {

it('@gl shows hover info for scattergl', function(done) {
Plotly.newPlot(gd, require('@mocks/gl2d_period_positioning.json'))
.then(function() { return Plotly.restyle(gd, 'xperiodalignment', 'start'); })
.then(function() { return Plotly.restyle(gd, 'yperiodalignment', 'start'); })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nonblocking, but these tests can be simplified (and sped up) with the object form

Plotly.restyle(gd,{xperiodalignment: 'start', yperiodalignment: 'start'})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 17215eb


.then(function() { _hover(100, 255); })
.then(function() {
assertHoverLabelContent({
Expand All @@ -641,6 +644,43 @@ describe('hover with (x|y)period positioning', function() {
nums: '(Jan 2006, Jun 1, 1970)'
});
})

.then(function() { return Plotly.restyle(gd, 'xperiodalignment', 'middle'); })
.then(function() { return Plotly.restyle(gd, 'yperiodalignment', 'middle'); })
.then(function() { _hover(100, 255); })
.then(function() { _hover(100, 255); })
.then(function() {
assertHoverLabelContent({
name: '',
nums: '(Jul 2001, Jan 16, 1970)'
});
})
.then(function() { _hover(470, 45); })
.then(function() {
assertHoverLabelContent({
name: '',
nums: '(Jul 2006, Jun 16, 1970)'
});
})

.then(function() { return Plotly.restyle(gd, 'xperiodalignment', 'end'); })
.then(function() { return Plotly.restyle(gd, 'yperiodalignment', 'end'); })

.then(function() { _hover(100, 255); })
.then(function() {
assertHoverLabelContent({
name: '',
nums: '(Jan 2002, Feb 1, 1970)'
});
})
.then(function() { _hover(470, 45); })
.then(function() {
assertHoverLabelContent({
name: '',
nums: '(Jan 2007, Jul 1, 1970)'
});
})

.then(done, done.fail);
});
});
Loading