-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Hover on fills #673
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.
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
Hover on fills #673
Changes from all commits
43e8d30
8aa4397
876bd1d
2018a74
c7a9dd5
5552c77
ffd56c7
dddb6e1
1931102
47d2df8
025b788
1a685d4
f22fac8
ba77f5b
1d1a0f7
aec4c95
1e86eb2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,61 +9,150 @@ | |
|
||
'use strict'; | ||
|
||
var Lib = require('../../lib'); | ||
var Fx = require('../../plots/cartesian/graph_interact'); | ||
var constants = require('../../plots/cartesian/constants'); | ||
var ErrorBars = require('../../components/errorbars'); | ||
var getTraceColor = require('./get_trace_color'); | ||
var Color = require('../../components/color'); | ||
|
||
|
||
module.exports = function hoverPoints(pointData, xval, yval, hovermode) { | ||
var cd = pointData.cd, | ||
trace = cd[0].trace, | ||
xa = pointData.xa, | ||
ya = pointData.ya, | ||
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 | ||
var rad = Math.max(3, di.mrc || 0); | ||
return Math.max(Math.abs(xa.c2p(di.x) - xa.c2p(xval)) - rad, 1 - 3 / rad); | ||
}, | ||
dy = function(di) { | ||
var rad = Math.max(3, di.mrc || 0); | ||
return Math.max(Math.abs(ya.c2p(di.y) - ya.c2p(yval)) - rad, 1 - 3 / rad); | ||
}, | ||
dxy = function(di) { | ||
var rad = Math.max(3, di.mrc || 0), | ||
dx = Math.abs(xa.c2p(di.x) - xa.c2p(xval)), | ||
dy = Math.abs(ya.c2p(di.y) - ya.c2p(yval)); | ||
return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad); | ||
}, | ||
distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy); | ||
|
||
Fx.getClosest(cd, distfn, pointData); | ||
|
||
// skip the rest (for this trace) if we didn't find a close point | ||
if(pointData.index === false) return; | ||
|
||
// the closest data point | ||
var di = cd[pointData.index], | ||
xc = xa.c2p(di.x, true), | ||
yc = ya.c2p(di.y, true), | ||
rad = di.mrc || 1; | ||
|
||
pointData.color = getTraceColor(trace, di); | ||
|
||
pointData.x0 = xc - rad; | ||
pointData.x1 = xc + rad; | ||
pointData.xLabelVal = di.x; | ||
|
||
pointData.y0 = yc - rad; | ||
pointData.y1 = yc + rad; | ||
pointData.yLabelVal = di.y; | ||
|
||
if(di.tx) pointData.text = di.tx; | ||
else if(trace.text) pointData.text = trace.text; | ||
|
||
ErrorBars.hoverInfo(di, trace, pointData); | ||
|
||
return [pointData]; | ||
xpx = xa.c2p(xval), | ||
ypx = ya.c2p(yval), | ||
pt = [xpx, ypx]; | ||
|
||
// look for points to hover on first, then take fills only if we | ||
// didn't find a point | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. I'm ok with prioritising points over fills as by default. But, is that true in general? That is, are there situations where This is getting way more complicated than I thought. I'm going to rethink about this in the morning. 😑 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think prioritizing fills over points would work well, for two reasons:
So I think we'd better leave it with points always winning, and if you don't like that you turn points off. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Agreed. The way you have it set up in aec4c95 is fine. Let's go with that. |
||
if(trace.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 | ||
var rad = Math.max(3, di.mrc || 0); | ||
return Math.max(Math.abs(xa.c2p(di.x) - xpx) - rad, 1 - 3 / rad); | ||
}, | ||
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); | ||
}, | ||
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); | ||
}, | ||
distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy); | ||
|
||
Fx.getClosest(cd, distfn, pointData); | ||
|
||
// skip the rest (for this trace) if we didn't find a close point | ||
if(pointData.index !== false) { | ||
|
||
// the closest data point | ||
var di = cd[pointData.index], | ||
xc = xa.c2p(di.x, true), | ||
yc = ya.c2p(di.y, true), | ||
rad = di.mrc || 1; | ||
|
||
Lib.extendFlat(pointData, { | ||
color: getTraceColor(trace, di), | ||
|
||
x0: xc - rad, | ||
x1: xc + rad, | ||
xLabelVal: di.x, | ||
|
||
y0: yc - rad, | ||
y1: yc + rad, | ||
yLabelVal: di.y | ||
}); | ||
|
||
if(di.tx) pointData.text = di.tx; | ||
else if(trace.text) pointData.text = trace.text; | ||
|
||
ErrorBars.hoverInfo(di, trace, pointData); | ||
|
||
return [pointData]; | ||
} | ||
} | ||
|
||
// even if hoveron is 'fills', only use it if we have polygons too | ||
if(trace.hoveron.indexOf('fills') !== -1 && trace._polygons) { | ||
var polygons = trace._polygons, | ||
polygonsIn = [], | ||
inside = false, | ||
xmin = Infinity, | ||
xmax = -Infinity, | ||
ymin = Infinity, | ||
ymax = -Infinity, | ||
i, j, polygon, pts, xCross, x0, x1, y0, y1; | ||
|
||
for(i = 0; i < polygons.length; i++) { | ||
polygon = polygons[i]; | ||
// TODO: this is not going to work right for curved edges, it will | ||
// act as though they're straight. That's probably going to need | ||
// the elements themselves to capture the events. Worth it? | ||
if(polygon.contains(pt)) { | ||
inside = !inside; | ||
// TODO: need better than just the overall bounding box | ||
polygonsIn.push(polygon); | ||
ymin = Math.min(ymin, polygon.ymin); | ||
ymax = Math.max(ymax, polygon.ymax); | ||
} | ||
} | ||
|
||
if(inside) { | ||
// find the overall left-most and right-most points of the | ||
// polygon(s) we're inside at their combined vertical midpoint. | ||
// This is where we will draw the hover label. | ||
// Note that this might not be the vertical midpoint of the | ||
// whole trace, if it's disjoint. | ||
var yAvg = (ymin + ymax) / 2; | ||
for(i = 0; i < polygonsIn.length; i++) { | ||
pts = polygonsIn[i].pts; | ||
for(j = 1; j < pts.length; j++) { | ||
y0 = pts[j - 1][1]; | ||
y1 = pts[j][1]; | ||
if((y0 > yAvg) !== (y1 >= yAvg)) { | ||
x0 = pts[j - 1][0]; | ||
x1 = pts[j][0]; | ||
xCross = x0 + (x1 - x0) * (yAvg - y0) / (y1 - y0); | ||
xmin = Math.min(xmin, xCross); | ||
xmax = Math.max(xmax, xCross); | ||
} | ||
} | ||
} | ||
|
||
// get only fill or line color for the hover color | ||
var color = Color.defaultLine; | ||
if(Color.opacity(trace.fillcolor)) color = trace.fillcolor; | ||
else if(Color.opacity((trace.line || {}).color)) { | ||
color = trace.line.color; | ||
} | ||
|
||
Lib.extendFlat(pointData, { | ||
// never let a 2D override 1D type as closest point | ||
distance: constants.MAXDIST + 10, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as heatmap - other types will return a distance that's no bigger than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parity with heatmaps is fine for now. Thanks for the clarification. 👍 |
||
x0: xmin, | ||
x1: xmax, | ||
y0: yAvg, | ||
y1: yAvg, | ||
color: color | ||
}); | ||
|
||
delete pointData.index; | ||
|
||
if(trace.text && !Array.isArray(trace.text)) { | ||
pointData.text = String(trace.text); | ||
} | ||
else pointData.text = trace.name; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I imagine some users would want per-polygon Maybe @cpsievert 's idea (in #147) of splitting segments of data using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not even sure theoretically how that API would look. It's easy enough to just make each polygon a trace if they want that. |
||
|
||
return [pointData]; | ||
} | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,5 +4,11 @@ module.exports = function createGraphDiv() { | |
var gd = document.createElement('div'); | ||
gd.id = 'graph'; | ||
document.body.appendChild(gd); | ||
|
||
// force the graph to be at position 0,0 no matter what | ||
gd.style.position = 'fixed'; | ||
gd.style.left = 0; | ||
gd.style.top = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Different browsers seem to disagree on the default body margin. Before this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fantastic. Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and thanks for fixing all the tests below. |
||
|
||
return gd; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having second thoughts about making
hoveron: 'points+fills'
the default forscatter
(my mistake).With
hovermode: 'x'
by default the'fills'
inhoveron: 'points+fills'
is hardly noticeable for most polygons.Moreover, overriding the
layout.hovermode
default based onfill
would a little too intrusive I think.While
hovermode: 'closest'
by default forscatterternary
, I'm not sure it warrants a different default than itsscatter
cousin.So, I'm leaning towards making
hoveron: 'points'
the default forscatter
andscatterternary
regardless of thefill
value.Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ternary is easier here because it doesn't even have a
hovermode: 'x'
... But I'd also argue that if you have a scatter trace with fill tonext or toself, you're making the statement "I'm enclosing an area" which doesn't make much sense in conjunction with comparing different traces at the same x value. For one, your outline trace is itself generally going to have two (or more) points at the same x value (or 2 crossings of the same x value anyway, on opposite sides of the region) which is a situation that has always confused people ("why do I never see hover data for the second point?")Not sure if there's an issue about it, but we've talked in the past about using trace info to make a smarter default
hovermode
choice, and I think this makes that case even stronger.hovermode: 'x'
should only really be the default when you have multiple traces all with monotonically increasing (or decreasing) x (or position, like horizontal bars) data. I don't think that would be too intrusive,hovermode
is a layout property but it is all about exposing the trace data anyway. If we did that, this concern would take care of itself.I suppose in the meantime we could put
hoveron
back to the way it was before as a default: only havehoveron: 'fills'
if you have an area fill and no explicit points, otherwisehoveron: 'points'
, never default to both. But I still think I'd prefer to leave it as is now, because it matches the meaning implied by your trace attributes.