Generalize spikelines #2379
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -144,8 +144,7 @@ exports.loneHover = function loneHover(hoverItem, opts) { | |
| rotateLabels: false, | ||
| bgColor: opts.bgColor || Color.background, | ||
| container: container3, | ||
| outerContainer: outerContainer3 | ||
| }; | ||
|
|
||
| var hoverLabel = createHoverText([pointData], fullOpts, opts.gd); | ||
|
|
@@ -162,9 +161,10 @@ function _hover(gd, evt, subplot, noHoverEvent) { | |
| // use those instead of finding overlayed plots | ||
| var subplots = Array.isArray(subplot) ? subplot : [subplot]; | ||
|
|
||
| var fullLayout = gd._fullLayout; | ||
| var plots = fullLayout._plots || []; | ||
| var plotinfo = plots[subplot]; | ||
| var hasCartesian = fullLayout._has('cartesian'); | ||
|
|
||
| // list of all overlaid subplots to look at | ||
| if(plotinfo) { | ||
|
|
@@ -351,9 +351,29 @@ function _hover(gd, evt, subplot, noHoverEvent) { | |
| trace: trace, | ||
| xa: xaArray[subploti], | ||
| ya: yaArray[subploti], | ||
|
|
||
| // max distances for hover and spikes - for points that want to show but do not | ||
| // want to override other points, set distance/spikeDistance equal to max*Distance | ||
| // and it will not get filtered out but it will be guaranteed to have a greater | ||
| // distance than any point that calculated a real distance. | ||
| maxHoverDistance: hoverdistance, | ||
| maxSpikeDistance: spikedistance, | ||
|
|
||
| // point properties - override all of these | ||
| index: false, // point index in trace - only used by plotly.js hoverdata consumers | ||
| distance: Math.min(distance, hoverdistance), // pixel distance or pseudo-distance | ||
|
|
||
| // distance/pseudo-distance for spikes. This distance should always be calculated | ||
| // as if in "closest" mode, and should only be set if this point should | ||
| // generate a spike. | ||
| spikeDistance: Infinity, | ||
|
|
||
| // in some cases the spikes have different positioning from the hover label | ||
| // they don't need x0/x1, just one position | ||
| xSpike: undefined, | ||
| ySpike: undefined, | ||
|
|
||
| // where and how to display the hover label | ||
| color: Color.defaultLine, // trace color | ||
| name: trace.name, | ||
| x0: undefined, | ||
|
|
@@ -418,29 +438,37 @@ function _hover(gd, evt, subplot, noHoverEvent) { | |
| } | ||
|
|
||
| // in closest mode, remove any existing (farther) points | ||
| // and don't look any farther than this latest point (or points, some | ||
| // traces like box & violin make multiple hover labels at once) | ||
| if(hovermode === 'closest' && hoverData.length > closedataPreviousLength) { | ||
| hoverData.splice(0, closedataPreviousLength); | ||
| distance = hoverData[0].distance; | ||
| } | ||
|
|
||
| // Now if there is range to look in, find the points to draw the spikelines | ||
| // Do it only if there is no hoverData | ||
| if(hasCartesian && (spikedistance !== 0)) { | ||
| if(hoverData.length === 0) { | ||
| pointData.distance = spikedistance; | ||
| pointData.index = false; | ||
| var closestPoints = trace._module.hoverPoints(pointData, xval, yval, 'closest', fullLayout._hoverlayer); | ||
| if(closestPoints) { | ||
| closestPoints = closestPoints.filter(function(point) { | ||
| // some hover points, like scatter fills, do not allow spikes, | ||
| // so will generate a hover point but without a valid spikeDistance | ||
| return point.spikeDistance <= spikedistance; | ||
| }); | ||
| } | ||
| if(closestPoints && closestPoints.length) { | ||
| var tmpPoint; | ||
| var closestVPoints = closestPoints.filter(function(point) { | ||
| return point.xa.showspikes; | ||
| }); | ||
| if(closestVPoints.length) { | ||
| var closestVPt = closestVPoints[0]; | ||
| if(isNumeric(closestVPt.x0) && isNumeric(closestVPt.y0)) { | ||
| tmpPoint = fillSpikePoint(closestVPt); | ||
| if(!spikePoints.vLinePoint || (spikePoints.vLinePoint.spikeDistance > tmpPoint.spikeDistance)) { | ||
| spikePoints.vLinePoint = tmpPoint; | ||
| } | ||
| } | ||
|
|
@@ -452,8 +480,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { | |
| if(closestHPoints.length) { | ||
| var closestHPt = closestHPoints[0]; | ||
| if(isNumeric(closestHPt.x0) && isNumeric(closestHPt.y0)) { | ||
| tmpPoint = fillSpikePoint(closestHPt); | ||
| if(!spikePoints.hLinePoint || (spikePoints.hLinePoint.spikeDistance > tmpPoint.spikeDistance)) { | ||
| spikePoints.hLinePoint = tmpPoint; | ||
| } | ||
| } | ||
|
|
@@ -464,47 +492,28 @@ function _hover(gd, evt, subplot, noHoverEvent) { | |
| } | ||
|
|
||
| function selectClosestPoint(pointsData, spikedistance) { | ||
|
There was a problem hiding this comment. We're not replicating the distance function here, because that only works for |
||
| var resultPoint = null; | ||
| var minDistance = Infinity; | ||
| var thisSpikeDistance; | ||
| for(var i = 0; i < pointsData.length; i++) { | ||
| thisSpikeDistance = pointsData[i].spikeDistance; | ||
| if(thisSpikeDistance < minDistance && thisSpikeDistance <= spikedistance) { | ||
| resultPoint = pointsData[i]; | ||
| minDistance = thisSpikeDistance; | ||
| } | ||
| } | ||
| return resultPoint; | ||
| } | ||
|
|
||
| function fillSpikePoint(point) { | ||
| if(!point) return null; | ||
| return { | ||
| xa: point.xa, | ||
| ya: point.ya, | ||
| x: point.xSpike !== undefined ? point.xSpike : (point.x0 + point.x1) / 2, | ||
| y: point.ySpike !== undefined ? point.ySpike : (point.y0 + point.y1) / 2, | ||
|
There was a problem hiding this comment. If a module wants spikes NOT to just be drawn at the midpoint of the box hover labels are attached to, it just needs to specify |
||
| distance: point.distance, | ||
| spikeDistance: point.spikeDistance, | ||
| curveNumber: point.trace.index, | ||
| color: point.color, | ||
| pointNumber: point.index | ||
|
|
@@ -525,34 +534,34 @@ function _hover(gd, evt, subplot, noHoverEvent) { | |
| gd._spikepoints = newspikepoints; | ||
|
|
||
| // Now if it is not restricted by spikedistance option, set the points to draw the spikelines | ||
| if(hasCartesian && (spikedistance !== 0)) { | ||
| if(hoverData.length !== 0) { | ||
| var tmpHPointData = hoverData.filter(function(point) { | ||
| return point.ya.showspikes; | ||
| }); | ||
| var tmpHPoint = selectClosestPoint(tmpHPointData, spikedistance); | ||
| spikePoints.hLinePoint = fillSpikePoint(tmpHPoint); | ||
|
|
||
| var tmpVPointData = hoverData.filter(function(point) { | ||
| return point.xa.showspikes; | ||
| }); | ||
| var tmpVPoint = selectClosestPoint(tmpVPointData, spikedistance); | ||
| spikePoints.vLinePoint = fillSpikePoint(tmpVPoint); | ||
| } | ||
| } | ||
|
|
||
| // if hoverData is empty check for the spikes to draw and quit if there are none | ||
| if(hoverData.length === 0) { | ||
| var result = dragElement.unhoverRaw(gd, evt); | ||
| if(hasCartesian && ((spikePoints.hLinePoint !== null) || (spikePoints.vLinePoint !== null))) { | ||
| if(spikesChanged(oldspikepoints)) { | ||
| createSpikelines(spikePoints, spikelineOpts); | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| if(hasCartesian) { | ||
| if(spikesChanged(oldspikepoints)) { | ||
| createSpikelines(spikePoints, spikelineOpts); | ||
| } | ||
|
|
@@ -653,20 +662,31 @@ function createHoverText(hoverData, opts, gd) { | |
| // show the common label, if any, on the axis | ||
| // never show a common label in array mode, | ||
| // even if sometimes there could be one | ||
| var showCommonLabel = ( | ||
| (t0 !== undefined) && | ||
|
There was a problem hiding this comment. Why this extra There was a problem hiding this comment. Related to #2379 (comment) |
||
| (c0.distance <= opts.hoverdistance) && | ||
| (hovermode === 'x' || hovermode === 'y') | ||
| ); | ||
|
|
||
| // all hover traces hoverinfo must contain the hovermode | ||
| // to have common labels | ||
| if(showCommonLabel) { | ||
| var i, traceHoverinfo; | ||
| var allHaveZ = true; | ||
| for(i = 0; i < hoverData.length; i++) { | ||
| if(allHaveZ && hoverData[i].zLabel === undefined) allHaveZ = false; | ||
|
|
||
| traceHoverinfo = hoverData[i].hoverinfo || hoverData[i].trace.hoverinfo; | ||
| var parts = Array.isArray(traceHoverinfo) ? traceHoverinfo : traceHoverinfo.split('+'); | ||
| if(parts.indexOf('all') === -1 && | ||
| parts.indexOf(hovermode) === -1) { | ||
| showCommonLabel = false; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // xyz labels put all info in their main label, so have no need of a common label | ||
| if(allHaveZ) showCommonLabel = false; | ||
|
There was a problem hiding this comment. This logic about whether to show the common label has gotten a bit more complex... previously it was being implicitly handled by There was a problem hiding this comment.
This sounds like the most robust way forward down the road 👍 |
||
| } | ||
|
|
||
| var commonLabel = container.selectAll('g.axistext') | ||
|
|
@@ -1262,8 +1282,8 @@ function createSpikelines(closestPoints, opts) { | |
| hLinePointX = evt.pointerX; | ||
| hLinePointY = evt.pointerY; | ||
| } else { | ||
| hLinePointX = xa._offset + hLinePoint.x; | ||
| hLinePointY = ya._offset + hLinePoint.y; | ||
| } | ||
| var dfltHLineColor = tinycolor.readability(hLinePoint.color, contrastColor) < 1.5 ? | ||
| Color.contrast(contrastColor) : hLinePoint.color; | ||
|
|
@@ -1338,8 +1358,8 @@ function createSpikelines(closestPoints, opts) { | |
| vLinePointX = evt.pointerX; | ||
| vLinePointY = evt.pointerY; | ||
| } else { | ||
| vLinePointX = xa._offset + vLinePoint.x; | ||
| vLinePointY = ya._offset + vLinePoint.y; | ||
| } | ||
| var dfltVLineColor = tinycolor.readability(vLinePoint.color, contrastColor) < 1.5 ? | ||
| Color.contrast(contrastColor) : vLinePoint.color; | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,8 +19,10 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { | |
| var trace = cd[0].trace; | ||
| var t = cd[0].t; | ||
| var isClosest = (hovermode === 'closest'); | ||
| var maxHoverDistance = pointData.maxHoverDistance; | ||
| var maxSpikeDistance = pointData.maxSpikeDistance; | ||
|
|
||
| var posVal, sizeVal, posLetter, sizeLetter, dx, dy, pRangeCalc; | ||
|
|
||
| function thisBarMinPos(di) { return di[posLetter] - di.w / 2; } | ||
| function thisBarMaxPos(di) { return di[posLetter] + di.w / 2; } | ||
|
|
@@ -49,15 +51,24 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { | |
| return Math.max(thisBarMaxPos(di), di.p + t.bardelta / 2); | ||
| }; | ||
|
|
||
| function _positionFn(_minPos, _maxPos) { | ||
| return Fx.inbox(_minPos - posVal, _maxPos - posVal, | ||
| maxHoverDistance - Math.min(1, Math.abs(_maxPos - _minPos) / pRangeCalc)); | ||
|
There was a problem hiding this comment. Would you mind adding a one-line comment description what this There was a problem hiding this comment. Ah, good call - it was actually wrong -> ad38f8d |
||
| } | ||
|
|
||
| function positionFn(di) { | ||
| return _positionFn(minPos(di), maxPos(di)); | ||
| } | ||
|
|
||
| function thisBarPositionFn(di) { | ||
| return _positionFn(thisBarMinPos(di), thisBarMaxPos(di)); | ||
| } | ||
|
|
||
| function sizeFn(di) { | ||
| // add a gradient so hovering near the end of a | ||
| // bar makes it a little closer match | ||
| return Fx.inbox(di.b - sizeVal, di[sizeLetter] - sizeVal, | ||
| maxHoverDistance + (di[sizeLetter] - sizeVal) / (di[sizeLetter] - di.b) - 1); | ||
| } | ||
|
|
||
| if(trace.orientation === 'h') { | ||
|
|
@@ -80,7 +91,10 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { | |
| var pa = pointData[posLetter + 'a']; | ||
| var sa = pointData[sizeLetter + 'a']; | ||
|
|
||
| pRangeCalc = Math.abs(pa.r2c(pa.range[1]) - pa.r2c(pa.range[0])); | ||
|
|
||
| function dxy(di) { return (dx(di) + dy(di)) / 2; } | ||
| var 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 | ||
|
|
@@ -116,6 +130,12 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { | |
| pointData[posLetter + '1'] = pa.c2p(maxPos(di), true); | ||
| pointData[posLetter + 'LabelVal'] = di.p; | ||
|
|
||
| // spikelines always want "closest" distance regardless of hovermode | ||
| pointData.spikeDistance = (sizeFn(di) + thisBarPositionFn(di)) / 2 + maxSpikeDistance - maxHoverDistance; | ||
| // 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); | ||
|
|
||
| fillHoverText(di, trace, pointData); | ||
| ErrorBars.hoverInfo(di, trace, pointData); | ||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
not using this constant anymore, we just have the defaults in
layout.hoverdistanceandlayout.spikedistance, with which the users of this constant were in conflict.