Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 2k
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
Uh oh!
There was an error while loading. Please reload this page.
Hover on fills #673
Changes from all commits
43e8d308aa4397876bd1d2018a74c7a9dd55552c77ffd56c7dddb6e1193110247d2df8025b7881a685d4f22fac8ba77f5b1d1a0f7aec4c951e86eb2File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
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 | ||
Contributor 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. 😑 CollaboratorAuthor 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. Contributor 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, | ||
Contributor 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 CollaboratorAuthor 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 Contributor 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; | ||
Contributor 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 CollaboratorAuthor 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; | ||
CollaboratorAuthor 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 Contributor 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!
| ||
| return gd; | ||
| }; | ||
Uh oh!
There was an error while loading. Please reload this page.
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.hovermodedefault based onfillwould a little too intrusive I think.While
hovermode: 'closest'by default forscatterternary, I'm not sure it warrants a different default than itsscattercousin.So, I'm leaning towards making
hoveron: 'points'the default forscatterandscatterternaryregardless of thefillvalue.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
hovermodechoice, 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,hovermodeis 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
hoveronback 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.