From 132b5b23149f73f1b4d4a8d656d616b1324cf2d4 Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Wed, 8 Nov 2017 17:52:28 +0300 Subject: [PATCH 01/12] add crossline functionality --- src/components/fx/hover.js | 133 ++++++++++++++- src/components/fx/index.js | 1 + src/plot_api/plot_api.js | 3 + src/plots/cartesian/axes.js | 26 ++- src/plots/cartesian/layout_attributes.js | 22 +++ src/plots/cartesian/layout_defaults.js | 7 + test/jasmine/tests/hover_crossline_test.js | 179 +++++++++++++++++++++ 7 files changed, 367 insertions(+), 4 deletions(-) create mode 100644 test/jasmine/tests/hover_crossline_test.js diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 4c606c3bcba..baf836c11f7 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -232,7 +232,13 @@ function _hover(gd, evt, subplot, noHoverEvent) { xval, yval, pointData, - closedataPreviousLength; + closedataPreviousLength, + + // crosslinePoints: the set of candidate points we've found to draw crosslines to + crosslinePoints = { + hLinePoint: null, + vLinePoint: null + }; // Figure out what we're hovering on: // mouse location or user-supplied data @@ -402,10 +408,60 @@ function _hover(gd, evt, subplot, noHoverEvent) { hoverData.splice(0, closedataPreviousLength); distance = hoverData[0].distance; } + + var showSpikes = fullLayout.xaxis && fullLayout.xaxis.showspikes && fullLayout.yaxis && fullLayout.yaxis.showspikes; + var showCrosslines = fullLayout.xaxis && fullLayout.xaxis.showcrossline || fullLayout.yaxis && fullLayout.yaxis.showcrossline; + + if(fullLayout._has('cartesian') && showCrosslines && !(showSpikes && hovermode === 'closest')) { + // Now find the points for the crosslines. + if(fullLayout.yaxis.showcrossline) { + crosslinePoints.hLinePoint = findCrosslinePoint(pointData, xval, yval, 'y', crosslinePoints.hLinePoint); + } + if(fullLayout.xaxis.showcrossline) { + crosslinePoints.vLinePoint = findCrosslinePoint(pointData, xval, yval, 'x', crosslinePoints.vLinePoint); + } + } } - // nothing left: remove all labels and quit - if(hoverData.length === 0) return dragElement.unhoverRaw(gd, evt); + function findCrosslinePoint(pointData, xval, yval, mode, endPoint) { + var resultPoint = endPoint; + pointData.distance = Infinity; + pointData.index = false; + var closestPoints = trace._module.hoverPoints(pointData, xval, yval, mode); + if(closestPoints) { + var closestPt = closestPoints[0]; + if(isNumeric(closestPt.x0) && isNumeric(closestPt.y0)) { + var tmpPoint = { + xa: closestPt.xa, + ya: closestPt.ya, + x0: closestPt.x0, + x1: closestPt.x1, + y0: closestPt.y0, + y1: closestPt.y1, + distance: closestPt.distance, + curveNumber: closestPt.trace.index, + pointNumber: closestPt.index + }; + if(!resultPoint || (resultPoint.distance > tmpPoint.distance)) { + resultPoint = tmpPoint; + } + } + } + return resultPoint; + } + + // if hoverData is empty check for the crosslines to draw and quit if there are none + if(hoverData.length === 0) { + var result = dragElement.unhoverRaw(gd, evt); + if(fullLayout._has('cartesian') && ((crosslinePoints.hLinePoint !== null) || (crosslinePoints.vLinePoint !== null))) { + createCrosslines(crosslinePoints, fullLayout); + } + return result; + } + + if(fullLayout._has('cartesian')) { + createCrosslines(crosslinePoints, fullLayout); + } hoverData.sort(function(d1, d2) { return d1.distance - d2.distance; }); @@ -1089,6 +1145,77 @@ function cleanPoint(d, hovermode) { return d; } +function createCrosslines(hoverData, fullLayout) { + var showXSpikeline = fullLayout.xaxis && fullLayout.xaxis.showspikes; + var showYSpikeline = fullLayout.yaxis && fullLayout.yaxis.showspikes; + var showH = fullLayout.yaxis && fullLayout.yaxis.showcrossline; + var showV = fullLayout.xaxis && fullLayout.xaxis.showcrossline; + var container = fullLayout._hoverlayer; + var hovermode = fullLayout.hovermode; + if(!(showV || showH) || (showXSpikeline && showYSpikeline && hovermode === 'closest')) return; + var hLinePoint, + vLinePoint, + xa, + ya, + hLinePointY, + vLinePointX; + + // Remove old crossline items + container.selectAll('.crossline').remove(); + + var contrastColor = Color.combine(fullLayout.plot_bgcolor, fullLayout.paper_bgcolor); + var dfltCrosslineColor = Color.contrast(contrastColor); + + // do not draw a crossline if there is a spikeline + if(showV && !(showXSpikeline && hovermode === 'closest')) { + vLinePoint = hoverData.vLinePoint; + xa = vLinePoint.xa; + vLinePointX = xa._offset + (vLinePoint.x0 + vLinePoint.x1) / 2; + + var xThickness = xa.crosslinethickness; + var xDash = xa.crosslinedash; + var xColor = xa.crosslinecolor || dfltCrosslineColor; + + // Foreground vertical line (to x-axis) + container.insert('line', ':first-child') + .attr({ + 'x1': vLinePointX, + 'x2': vLinePointX, + 'y1': xa._counterSpan[0], + 'y2': xa._counterSpan[1], + 'stroke-width': xThickness, + 'stroke': xColor, + 'stroke-dasharray': Drawing.dashStyle(xDash, xThickness) + }) + .classed('crossline', true) + .classed('crisp', true); + } + + if(showH && !(showYSpikeline && hovermode === 'closest')) { + hLinePoint = hoverData.hLinePoint; + ya = hLinePoint.ya; + hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2; + + var yThickness = ya.crosslinethickness; + var yDash = ya.crosslinedash; + var yColor = ya.crosslinecolor || dfltCrosslineColor; + + // Foreground horizontal line (to y-axis) + container.insert('line', ':first-child') + .attr({ + 'x1': ya._counterSpan[0], + 'x2': ya._counterSpan[1], + 'y1': hLinePointY, + 'y2': hLinePointY, + 'stroke-width': yThickness, + 'stroke': yColor, + 'stroke-dasharray': Drawing.dashStyle(yDash, yThickness) + }) + .classed('crossline', true) + .classed('crisp', true); + } +} + function createSpikelines(hoverData, opts) { var hovermode = opts.hovermode; var container = opts.container; diff --git a/src/components/fx/index.js b/src/components/fx/index.js index 77c618b8354..7377186fa6a 100644 --- a/src/components/fx/index.js +++ b/src/components/fx/index.js @@ -59,6 +59,7 @@ function loneUnhover(containerOrSelection) { selection.selectAll('g.hovertext').remove(); selection.selectAll('.spikeline').remove(); + selection.selectAll('.crossline').remove(); } // helpers for traces that use Fx.loneHover diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 73d6e792fe8..67429e258da 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -166,6 +166,9 @@ Plotly.plot = function(gd, data, layout, config) { // save initial show spikes once per graph if(graphWasEmpty) Plotly.Axes.saveShowSpikeInitial(gd); + // save initial show crosslines once per graph + if(graphWasEmpty) Plotly.Axes.saveShowCrosslineInitial(gd); + // prepare the data and find the autorange // generate calcdata, if we need to diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 4aeea3bd743..46c728ffa11 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -423,6 +423,30 @@ axes.saveShowSpikeInitial = function(gd, overwrite) { return hasOneAxisChanged; }; +// save a copy of the initial crossline visibility +axes.saveShowCrosslineInitial = function(gd, overwrite) { + var axList = axes.list(gd, '', true), + hasOneAxisChanged = false; + + for(var i = 0; i < axList.length; i++) { + var ax = axList[i]; + + var isNew = (ax._showCrosslineInitial === undefined); + var hasChanged = ( + isNew || !( + ax.showcrossline === ax._showcrossline + ) + ); + + if((isNew) || (overwrite && hasChanged)) { + ax._showCrosslineInitial = ax.showcrossline; + hasOneAxisChanged = true; + } + + } + return hasOneAxisChanged; +}; + // axes.expand: if autoranging, include new data in the outer limits // for this axis // data is an array of numbers (ie already run through ax.d2c) @@ -2048,7 +2072,7 @@ axes.doTicks = function(gd, axid, skipTitle) { top: pos, bottom: pos, left: ax._offset, - rigth: ax._offset + ax._length, + right: ax._offset + ax._length, width: ax._length, height: 0 }; diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js index b4b3d05c856..226786018b7 100644 --- a/src/plots/cartesian/layout_attributes.js +++ b/src/plots/cartesian/layout_attributes.js @@ -387,6 +387,28 @@ module.exports = { 'plotted on' ].join(' ') }, + showcrossline: { + valType: 'boolean', + dflt: false, + role: 'style', + editType: 'none', + description: 'Determines whether or not crossline are drawn for this axis.' + }, + crosslinecolor: { + valType: 'color', + dflt: null, + role: 'style', + editType: 'none', + description: 'Sets the crossline color. If undefined, will use the contrast to background color' + }, + crosslinethickness: { + valType: 'number', + dflt: 2, + role: 'style', + editType: 'none', + description: 'Sets the width (in px) of the zero line.' + }, + crosslinedash: extendFlat({}, dash, {dflt: 'solid', editType: 'none'}), tickfont: fontAttrs({ editType: 'ticks', description: 'Sets the tick font.' diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index 2afdbe66357..9f95b77266c 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -139,6 +139,13 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { handleAxisDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions, layoutOut); + var showCrossline = coerce('showcrossline'); + if(showCrossline) { + coerce('crosslinecolor'); + coerce('crosslinethickness'); + coerce('crosslinedash'); + } + var showSpikes = coerce('showspikes'); if(showSpikes) { coerce('spikecolor'); diff --git a/test/jasmine/tests/hover_crossline_test.js b/test/jasmine/tests/hover_crossline_test.js new file mode 100644 index 00000000000..3150959af79 --- /dev/null +++ b/test/jasmine/tests/hover_crossline_test.js @@ -0,0 +1,179 @@ +var d3 = require('d3'); + +var Plotly = require('@lib/index'); +var Fx = require('@src/components/fx'); +var Lib = require('@src/lib'); + +var fail = require('../assets/fail_test'); +var createGraphDiv = require('../assets/create_graph_div'); +var destroyGraphDiv = require('../assets/destroy_graph_div'); + +describe('crossline', function() { + 'use strict'; + + afterEach(destroyGraphDiv); + + describe('hover', function() { + var gd; + + function makeMock() { + var _mock = Lib.extendDeep({}, require('@mocks/19.json')); + _mock.layout.xaxis.showcrossline = true; + _mock.layout.yaxis.showcrossline = true; + _mock.layout.xaxis2.showcrossline = true; + _mock.layout.hovermode = 'closest'; + return _mock; + } + + function _hover(evt, subplot) { + Fx.hover(gd, evt, subplot); + Lib.clearThrottle(); + } + + function _assert(lineExpect) { + var TOL = 5; + var lines = d3.selectAll('line.crossline'); + + expect(lines.size()).toBe(lineExpect.length, '# of line nodes'); + + lines.each(function(_, i) { + var sel = d3.select(this); + ['x1', 'y1', 'x2', 'y2'].forEach(function(d, j) { + expect(sel.attr(d)) + .toBeWithin(lineExpect[i][j], TOL, 'line ' + i + ' attr ' + d); + }); + }); + } + + it('draws lines and markers on enabled axes in the closest hovermode', function(done) { + gd = createGraphDiv(); + var _mock = makeMock(); + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[80, 250, 1033, 250], [557, 100, 557, 401]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[651, 167, 985, 167], [820, 115, 820, 220]] + ); + }) + .catch(fail) + .then(done); + }); + + it('draws lines and markers on enabled axes in the x hovermode', function(done) { + gd = createGraphDiv(); + var _mock = makeMock(); + + _mock.layout.hovermode = 'x'; + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[80, 250, 1033, 250], [557, 100, 557, 401]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[651, 167, 985, 167], [820, 115, 820, 220]] + ); + }) + .catch(fail) + .then(done); + }); + + it('draws lines and markers on enabled axes in the y hovermode', function(done) { + gd = createGraphDiv(); + var _mock = makeMock(); + + _mock.layout.hovermode = 'y'; + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[80, 250, 1033, 250], [557, 100, 557, 401]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[652, 167, 985, 167], [820, 115, 820, 220]] + ); + }) + .catch(fail) + .then(done); + }); + + it('does not draw lines and markers on enabled axes if spikes are enabled on the same axes', function(done) { + gd = createGraphDiv(); + var _mock = makeMock(); + + _mock.layout.xaxis.showspikes = true; + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[80, 250, 1033, 250]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[652, 167, 985, 167]] + ); + }) + .catch(fail) + .then(done); + }); + + it('does not draw lines and markers on enabled axes if spikes are enabled on the same axes', function(done) { + gd = createGraphDiv(); + var _mock = makeMock(); + + _mock.layout.yaxis.showspikes = true; + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[557, 100, 557, 401]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[818, 115, 818, 220]] + ); + }) + .catch(fail) + .then(done); + }); + + it('draws lines and markers on enabled axes w/o tick labels', function(done) { + gd = createGraphDiv(); + var _mock = makeMock(); + + _mock.layout.xaxis.showticklabels = false; + _mock.layout.yaxis.showticklabels = false; + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[80, 250, 1033, 250], [557, 100, 557, 401]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[652, 167, 985, 167], [820, 115, 820, 220]] + ); + }) + .catch(fail) + .then(done); + }); + }); +}); From 85714e89215c0d50ce707f951a6fd7d8b4a37ac6 Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Thu, 9 Nov 2017 12:24:13 +0300 Subject: [PATCH 02/12] fix crossline tests --- test/jasmine/tests/hover_crossline_test.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/jasmine/tests/hover_crossline_test.js b/test/jasmine/tests/hover_crossline_test.js index 3150959af79..6ae0350b4d1 100644 --- a/test/jasmine/tests/hover_crossline_test.js +++ b/test/jasmine/tests/hover_crossline_test.js @@ -52,13 +52,13 @@ describe('crossline', function() { Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); _assert( - [[80, 250, 1033, 250], [557, 100, 557, 401]] + [[80, 250, 1036, 250], [557, 100, 557, 401]] ); }) .then(function() { _hover({xval: 30, yval: 40}, 'x2y2'); _assert( - [[651, 167, 985, 167], [820, 115, 820, 220]] + [[651, 167, 988, 167], [820, 115, 820, 220]] ); }) .catch(fail) @@ -74,13 +74,13 @@ describe('crossline', function() { Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); _assert( - [[80, 250, 1033, 250], [557, 100, 557, 401]] + [[80, 250, 1036, 250], [557, 100, 557, 401]] ); }) .then(function() { _hover({xval: 30, yval: 40}, 'x2y2'); _assert( - [[651, 167, 985, 167], [820, 115, 820, 220]] + [[651, 167, 988, 167], [820, 115, 820, 220]] ); }) .catch(fail) @@ -96,13 +96,13 @@ describe('crossline', function() { Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); _assert( - [[80, 250, 1033, 250], [557, 100, 557, 401]] + [[80, 250, 1036, 250], [557, 100, 557, 401]] ); }) .then(function() { _hover({xval: 30, yval: 40}, 'x2y2'); _assert( - [[652, 167, 985, 167], [820, 115, 820, 220]] + [[652, 167, 988, 167], [820, 115, 820, 220]] ); }) .catch(fail) @@ -118,13 +118,13 @@ describe('crossline', function() { Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); _assert( - [[80, 250, 1033, 250]] + [[80, 250, 1036, 250]] ); }) .then(function() { _hover({xval: 30, yval: 40}, 'x2y2'); _assert( - [[652, 167, 985, 167]] + [[652, 167, 988, 167]] ); }) .catch(fail) @@ -146,7 +146,7 @@ describe('crossline', function() { .then(function() { _hover({xval: 30, yval: 40}, 'x2y2'); _assert( - [[818, 115, 818, 220]] + [[820, 115, 820, 220]] ); }) .catch(fail) From cd83ecf33416a14537f6993d9f266c471fcd943b Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Thu, 9 Nov 2017 12:48:55 +0300 Subject: [PATCH 03/12] fix crossline tests --- test/jasmine/tests/hover_crossline_test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jasmine/tests/hover_crossline_test.js b/test/jasmine/tests/hover_crossline_test.js index 6ae0350b4d1..6e054f3d61c 100644 --- a/test/jasmine/tests/hover_crossline_test.js +++ b/test/jasmine/tests/hover_crossline_test.js @@ -163,13 +163,13 @@ describe('crossline', function() { Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); _assert( - [[80, 250, 1033, 250], [557, 100, 557, 401]] + [[80, 250, 1036, 250], [557, 100, 557, 401]] ); }) .then(function() { _hover({xval: 30, yval: 40}, 'x2y2'); _assert( - [[652, 167, 985, 167], [820, 115, 820, 220]] + [[652, 167, 988, 167], [820, 115, 820, 220]] ); }) .catch(fail) From 006b55dfccf94b247cceb47b596aa64a04814334 Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Thu, 9 Nov 2017 15:26:12 +0300 Subject: [PATCH 04/12] move the search for crosslinePoints to ensure that the hoverLabels data will not be overwritten --- src/components/fx/hover.js | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index baf836c11f7..1ed3bf8ca9e 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -385,6 +385,19 @@ function _hover(gd, evt, subplot, noHoverEvent) { yval = yvalArray[subploti]; } + var showSpikes = fullLayout.xaxis && fullLayout.xaxis.showspikes && fullLayout.yaxis && fullLayout.yaxis.showspikes; + var showCrosslines = fullLayout.xaxis && fullLayout.xaxis.showcrossline || fullLayout.yaxis && fullLayout.yaxis.showcrossline; + + // Find the points for the crosslines first to avoid overwriting the hoverLabels data. + if(fullLayout._has('cartesian') && showCrosslines && !(showSpikes && hovermode === 'closest')) { + if(fullLayout.yaxis.showcrossline) { + crosslinePoints.hLinePoint = findCrosslinePoint(pointData, xval, yval, 'y', crosslinePoints.hLinePoint); + } + if(fullLayout.xaxis.showcrossline) { + crosslinePoints.vLinePoint = findCrosslinePoint(pointData, xval, yval, 'x', crosslinePoints.vLinePoint); + } + } + // Now find the points. if(trace._module && trace._module.hoverPoints) { var newPoints = trace._module.hoverPoints(pointData, xval, yval, mode, fullLayout._hoverlayer); @@ -408,22 +421,11 @@ function _hover(gd, evt, subplot, noHoverEvent) { hoverData.splice(0, closedataPreviousLength); distance = hoverData[0].distance; } - - var showSpikes = fullLayout.xaxis && fullLayout.xaxis.showspikes && fullLayout.yaxis && fullLayout.yaxis.showspikes; - var showCrosslines = fullLayout.xaxis && fullLayout.xaxis.showcrossline || fullLayout.yaxis && fullLayout.yaxis.showcrossline; - - if(fullLayout._has('cartesian') && showCrosslines && !(showSpikes && hovermode === 'closest')) { - // Now find the points for the crosslines. - if(fullLayout.yaxis.showcrossline) { - crosslinePoints.hLinePoint = findCrosslinePoint(pointData, xval, yval, 'y', crosslinePoints.hLinePoint); - } - if(fullLayout.xaxis.showcrossline) { - crosslinePoints.vLinePoint = findCrosslinePoint(pointData, xval, yval, 'x', crosslinePoints.vLinePoint); - } - } } function findCrosslinePoint(pointData, xval, yval, mode, endPoint) { + var tmpDistance = pointData.distance; + var tmpIndex = pointData.index; var resultPoint = endPoint; pointData.distance = Infinity; pointData.index = false; @@ -447,6 +449,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { } } } + pointData.index = tmpIndex; + pointData.distance = tmpDistance; return resultPoint; } @@ -1145,7 +1149,7 @@ function cleanPoint(d, hovermode) { return d; } -function createCrosslines(hoverData, fullLayout) { +function createCrosslines(closestPoints, fullLayout) { var showXSpikeline = fullLayout.xaxis && fullLayout.xaxis.showspikes; var showYSpikeline = fullLayout.yaxis && fullLayout.yaxis.showspikes; var showH = fullLayout.yaxis && fullLayout.yaxis.showcrossline; @@ -1168,7 +1172,7 @@ function createCrosslines(hoverData, fullLayout) { // do not draw a crossline if there is a spikeline if(showV && !(showXSpikeline && hovermode === 'closest')) { - vLinePoint = hoverData.vLinePoint; + vLinePoint = closestPoints.vLinePoint; xa = vLinePoint.xa; vLinePointX = xa._offset + (vLinePoint.x0 + vLinePoint.x1) / 2; @@ -1192,7 +1196,7 @@ function createCrosslines(hoverData, fullLayout) { } if(showH && !(showYSpikeline && hovermode === 'closest')) { - hLinePoint = hoverData.hLinePoint; + hLinePoint = closestPoints.hLinePoint; ya = hLinePoint.ya; hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2; From b14286b38a43a141abac57b990e7421fb4e50593 Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Wed, 6 Dec 2017 15:23:56 +0300 Subject: [PATCH 05/12] add spikedistance, hoverdistance, spikesnap --- src/components/fx/hover.js | 319 +++++++++++---------- src/components/fx/index.js | 1 - src/components/fx/layout_attributes.js | 23 +- src/components/fx/layout_defaults.js | 2 + src/components/modebar/buttons.js | 7 +- src/plot_api/plot_api.js | 3 - src/plots/cartesian/axes.js | 24 -- src/plots/cartesian/layout_attributes.js | 24 +- src/plots/cartesian/layout_defaults.js | 8 +- test/jasmine/tests/hover_crossline_test.js | 179 ------------ test/jasmine/tests/modebar_test.js | 25 -- 11 files changed, 191 insertions(+), 424 deletions(-) delete mode 100644 test/jasmine/tests/hover_crossline_test.js diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 1ed3bf8ca9e..55c5d711f9c 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -234,8 +234,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { pointData, closedataPreviousLength, - // crosslinePoints: the set of candidate points we've found to draw crosslines to - crosslinePoints = { + // closestPoints: the set of candidate points we've found to draw spikes to + closestPoints = { hLinePoint: null, vLinePoint: null }; @@ -331,6 +331,9 @@ function _hover(gd, evt, subplot, noHoverEvent) { // within one trace mode can sometimes be overridden mode = hovermode; + var hoverdistance = fullLayout.hoverdistance ? fullLayout.hoverdistance === -1 ? Infinity : fullLayout.hoverdistance : constants.MAXDIST; + var spikedistance = fullLayout.spikedistance ? fullLayout.spikedistance === -1 ? Infinity : fullLayout.spikedistance : hoverdistance; + // container for new point, also used to pass info into module.hoverPoints pointData = { // trace properties @@ -340,7 +343,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { ya: yaArray[subploti], // point properties - override all of these index: false, // point index in trace - only used by plotly.js hoverdata consumers - distance: Math.min(distance, constants.MAXDIST), // pixel distance or pseudo-distance + distance: Math.min(distance, hoverdistance), // pixel distance or pseudo-distance color: Color.defaultLine, // trace color name: trace.name, x0: undefined, @@ -385,16 +388,14 @@ function _hover(gd, evt, subplot, noHoverEvent) { yval = yvalArray[subploti]; } - var showSpikes = fullLayout.xaxis && fullLayout.xaxis.showspikes && fullLayout.yaxis && fullLayout.yaxis.showspikes; - var showCrosslines = fullLayout.xaxis && fullLayout.xaxis.showcrossline || fullLayout.yaxis && fullLayout.yaxis.showcrossline; - - // Find the points for the crosslines first to avoid overwriting the hoverLabels data. - if(fullLayout._has('cartesian') && showCrosslines && !(showSpikes && hovermode === 'closest')) { - if(fullLayout.yaxis.showcrossline) { - crosslinePoints.hLinePoint = findCrosslinePoint(pointData, xval, yval, 'y', crosslinePoints.hLinePoint); - } - if(fullLayout.xaxis.showcrossline) { - crosslinePoints.vLinePoint = findCrosslinePoint(pointData, xval, yval, 'x', crosslinePoints.vLinePoint); + // Find the points for the spikes first to avoid overwriting the hoverLabels data. + if(fullLayout._has('cartesian')) { + if(fullLayout.hovermode === 'closest') { + closestPoints.hLinePoint = findClosestPoint(pointData, xval, yval, 'closest', closestPoints.hLinePoint, spikedistance); + closestPoints.vLinePoint = findClosestPoint(pointData, xval, yval, 'closest', closestPoints.vLinePoint, spikedistance); + } else { + closestPoints.hLinePoint = findClosestPoint(pointData, xval, yval, 'y', closestPoints.hLinePoint, spikedistance); + closestPoints.vLinePoint = findClosestPoint(pointData, xval, yval, 'x', closestPoints.vLinePoint, spikedistance); } } @@ -423,29 +424,41 @@ function _hover(gd, evt, subplot, noHoverEvent) { } } - function findCrosslinePoint(pointData, xval, yval, mode, endPoint) { + function findClosestPoint(pointData, xval, yval, mode, endPoint, spikedistance) { var tmpDistance = pointData.distance; var tmpIndex = pointData.index; var resultPoint = endPoint; - pointData.distance = Infinity; + pointData.distance = spikedistance; pointData.index = false; var closestPoints = trace._module.hoverPoints(pointData, xval, yval, mode); if(closestPoints) { - var closestPt = closestPoints[0]; - if(isNumeric(closestPt.x0) && isNumeric(closestPt.y0)) { - var tmpPoint = { - xa: closestPt.xa, - ya: closestPt.ya, - x0: closestPt.x0, - x1: closestPt.x1, - y0: closestPt.y0, - y1: closestPt.y1, - distance: closestPt.distance, - curveNumber: closestPt.trace.index, - pointNumber: closestPt.index - }; - if(!resultPoint || (resultPoint.distance > tmpPoint.distance)) { - resultPoint = tmpPoint; + closestPoints = closestPoints.filter(function(point) { + if(mode === 'x') { + return point.xa.showspikes; + } else if(mode === 'y') { + return point.ya.showspikes; + } else if(mode === 'closest') { + return point.ya.showspikes && point.ya.showspikes; + } + }); + if(closestPoints.length) { + var closestPt = closestPoints[0]; + if(isNumeric(closestPt.x0) && isNumeric(closestPt.y0)) { + var tmpPoint = { + xa: closestPt.xa, + ya: closestPt.ya, + x0: closestPt.x0, + x1: closestPt.x1, + y0: closestPt.y0, + y1: closestPt.y1, + distance: closestPt.distance, + curveNumber: closestPt.trace.index, + color: closestPt.color, + pointNumber: closestPt.index + }; + if(!resultPoint || (resultPoint.distance > tmpPoint.distance)) { + resultPoint = tmpPoint; + } } } } @@ -454,17 +467,35 @@ function _hover(gd, evt, subplot, noHoverEvent) { return resultPoint; } - // if hoverData is empty check for the crosslines to draw and quit if there are none + // if hoverData is empty check for the spikes to draw and quit if there are none + var spikelineOpts = { + hovermode: hovermode, + fullLayout: fullLayout, + container: fullLayout._hoverlayer, + outerContainer: fullLayout._paperdiv, + event: evt + }; + var oldspikepoints = gd._spikepoints, + newspikepoints = { + vLinePoint: closestPoints.vLinePoint, + hLinePoint: closestPoints.hLinePoint + }; + gd._spikepoints = newspikepoints; + if(hoverData.length === 0) { var result = dragElement.unhoverRaw(gd, evt); - if(fullLayout._has('cartesian') && ((crosslinePoints.hLinePoint !== null) || (crosslinePoints.vLinePoint !== null))) { - createCrosslines(crosslinePoints, fullLayout); + if(fullLayout._has('cartesian') && ((closestPoints.hLinePoint !== null) || (closestPoints.vLinePoint !== null))) { + if(spikesChanged(oldspikepoints)) { + createSpikelines2(closestPoints, spikelineOpts); + } } return result; } if(fullLayout._has('cartesian')) { - createCrosslines(crosslinePoints, fullLayout); + if(spikesChanged(oldspikepoints)) { + createSpikelines2(closestPoints, spikelineOpts); + } } hoverData.sort(function(d1, d2) { return d1.distance - d2.distance; }); @@ -482,16 +513,6 @@ function _hover(gd, evt, subplot, noHoverEvent) { gd._hoverdata = newhoverdata; - if(hoverChanged(gd, evt, oldhoverdata) && fullLayout._hasCartesian) { - var spikelineOpts = { - hovermode: hovermode, - fullLayout: fullLayout, - container: fullLayout._hoverlayer, - outerContainer: fullLayout._paperdiv - }; - createSpikelines(hoverData, spikelineOpts); - } - // if there's more than one horz bar trace, // rotate the labels so they don't overlap var rotateLabels = hovermode === 'y' && searchData.length > 1; @@ -1149,14 +1170,11 @@ function cleanPoint(d, hovermode) { return d; } -function createCrosslines(closestPoints, fullLayout) { - var showXSpikeline = fullLayout.xaxis && fullLayout.xaxis.showspikes; - var showYSpikeline = fullLayout.yaxis && fullLayout.yaxis.showspikes; - var showH = fullLayout.yaxis && fullLayout.yaxis.showcrossline; - var showV = fullLayout.xaxis && fullLayout.xaxis.showcrossline; - var container = fullLayout._hoverlayer; - var hovermode = fullLayout.hovermode; - if(!(showV || showH) || (showXSpikeline && showYSpikeline && hovermode === 'closest')) return; +function createSpikelines2(closestPoints, opts) { + var hovermode = opts.hovermode; + var container = opts.container; + var fullLayout = opts.fullLayout; + var evt = opts.event; var hLinePoint, vLinePoint, xa, @@ -1164,131 +1182,95 @@ function createCrosslines(closestPoints, fullLayout) { hLinePointY, vLinePointX; - // Remove old crossline items - container.selectAll('.crossline').remove(); + var showY = closestPoints.hLinePoint ? true : false; + var showX = closestPoints.vLinePoint ? true : false; - var contrastColor = Color.combine(fullLayout.plot_bgcolor, fullLayout.paper_bgcolor); - var dfltCrosslineColor = Color.contrast(contrastColor); + // Remove old spikeline items + container.selectAll('.spikeline').remove(); - // do not draw a crossline if there is a spikeline - if(showV && !(showXSpikeline && hovermode === 'closest')) { - vLinePoint = closestPoints.vLinePoint; - xa = vLinePoint.xa; - vLinePointX = xa._offset + (vLinePoint.x0 + vLinePoint.x1) / 2; - - var xThickness = xa.crosslinethickness; - var xDash = xa.crosslinedash; - var xColor = xa.crosslinecolor || dfltCrosslineColor; - - // Foreground vertical line (to x-axis) - container.insert('line', ':first-child') - .attr({ - 'x1': vLinePointX, - 'x2': vLinePointX, - 'y1': xa._counterSpan[0], - 'y2': xa._counterSpan[1], - 'stroke-width': xThickness, - 'stroke': xColor, - 'stroke-dasharray': Drawing.dashStyle(xDash, xThickness) - }) - .classed('crossline', true) - .classed('crisp', true); - } + if(!(showX || showY)) return; - if(showH && !(showYSpikeline && hovermode === 'closest')) { + if(showY) { hLinePoint = closestPoints.hLinePoint; - ya = hLinePoint.ya; - hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2; - - var yThickness = ya.crosslinethickness; - var yDash = ya.crosslinedash; - var yColor = ya.crosslinecolor || dfltCrosslineColor; - - // Foreground horizontal line (to y-axis) - container.insert('line', ':first-child') - .attr({ - 'x1': ya._counterSpan[0], - 'x2': ya._counterSpan[1], - 'y1': hLinePointY, - 'y2': hLinePointY, - 'stroke-width': yThickness, - 'stroke': yColor, - 'stroke-dasharray': Drawing.dashStyle(yDash, yThickness) - }) - .classed('crossline', true) - .classed('crisp', true); + ya = hLinePoint && hLinePoint.ya; + var ySnap = ya.spikesnap; + if(ySnap === 'cursor') { + hLinePointY = evt.offsetY; + } else { + hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2; + } + } + if(showX) { + vLinePoint = closestPoints.vLinePoint; + xa = vLinePoint && vLinePoint.xa; + var xSnap = xa.spikesnap; + if(xSnap === 'cursor') { + vLinePointX = evt.offsetX; + } else { + vLinePointX = xa._offset + (vLinePoint.x0 + vLinePoint.x1) / 2; + } } -} - -function createSpikelines(hoverData, opts) { - var hovermode = opts.hovermode; - var container = opts.container; - var c0 = hoverData[0]; - var xa = c0.xa; - var ya = c0.ya; - var showX = xa.showspikes; - var showY = ya.showspikes; - - // Remove old spikeline items - container.selectAll('.spikeline').remove(); - - if(hovermode !== 'closest' || !(showX || showY)) return; - var fullLayout = opts.fullLayout; - var xPoint = xa._offset + (c0.x0 + c0.x1) / 2; - var yPoint = ya._offset + (c0.y0 + c0.y1) / 2; var contrastColor = Color.combine(fullLayout.plot_bgcolor, fullLayout.paper_bgcolor); - var dfltDashColor = tinycolor.readability(c0.color, contrastColor) < 1.5 ? - Color.contrast(contrastColor) : c0.color; + // Horizontal line (to y-axis) if(showY) { + var dfltHLineColor = tinycolor.readability(hLinePoint.color, contrastColor) < 1.5 ? + Color.contrast(contrastColor) : hLinePoint.color; var yMode = ya.spikemode; + if(hovermode !== 'closest' && yMode.indexOf('toaxis') !== -1) { + yMode = yMode.replace('toaxis', 'across'); + } var yThickness = ya.spikethickness; - var yColor = ya.spikecolor || dfltDashColor; + var yColor = ya.spikecolor || dfltHLineColor; var yBB = ya._boundingBox; - var xEdge = ((yBB.left + yBB.right) / 2) < xPoint ? yBB.right : yBB.left; + var xEdge = ((yBB.left + yBB.right) / 2) < vLinePointX ? yBB.right : yBB.left; + var xBase; + var xEndSpike; if(yMode.indexOf('toaxis') !== -1 || yMode.indexOf('across') !== -1) { - var xBase = xEdge; - var xEndSpike = xPoint; + if(yMode.indexOf('toaxis') !== -1) { + xBase = xEdge; + xEndSpike = vLinePointX; + } if(yMode.indexOf('across') !== -1) { xBase = ya._counterSpan[0]; xEndSpike = ya._counterSpan[1]; } - // Background horizontal Line (to y-axis) - container.append('line') + // Foreground horizontal line (to y-axis) + container.insert('line', ':first-child') .attr({ 'x1': xBase, 'x2': xEndSpike, - 'y1': yPoint, - 'y2': yPoint, - 'stroke-width': yThickness + 2, - 'stroke': contrastColor + 'y1': hLinePointY, + 'y2': hLinePointY, + 'stroke-width': yThickness, + 'stroke': yColor, + 'stroke-dasharray': Drawing.dashStyle(ya.spikedash, yThickness) }) .classed('spikeline', true) .classed('crisp', true); - // Foreground horizontal line (to y-axis) - container.append('line') + // Background horizontal Line (to y-axis) + container.insert('line', ':first-child') .attr({ 'x1': xBase, 'x2': xEndSpike, - 'y1': yPoint, - 'y2': yPoint, - 'stroke-width': yThickness, - 'stroke': yColor, - 'stroke-dasharray': Drawing.dashStyle(ya.spikedash, yThickness) + 'y1': hLinePointY, + 'y2': hLinePointY, + 'stroke-width': yThickness + 2, + 'stroke': contrastColor }) .classed('spikeline', true) .classed('crisp', true); } // Y axis marker if(yMode.indexOf('marker') !== -1) { - container.append('circle') + container.insert('circle', ':first-child') .attr({ 'cx': xEdge + (ya.side !== 'right' ? yThickness : -yThickness), - 'cy': yPoint, + 'cy': hLinePointY, 'r': yThickness, 'fill': yColor }) @@ -1297,43 +1279,53 @@ function createSpikelines(hoverData, opts) { } if(showX) { + var dfltVLineColor = tinycolor.readability(vLinePoint.color, contrastColor) < 1.5 ? + Color.contrast(contrastColor) : vLinePoint.color; var xMode = xa.spikemode; + if(hovermode !== 'closest' && xMode.indexOf('toaxis') !== -1) { + xMode = xMode.replace('toaxis', 'across'); + } var xThickness = xa.spikethickness; - var xColor = xa.spikecolor || dfltDashColor; + var xColor = xa.spikecolor || dfltVLineColor; var xBB = xa._boundingBox; - var yEdge = ((xBB.top + xBB.bottom) / 2) < yPoint ? xBB.bottom : xBB.top; + var yEdge = ((xBB.top + xBB.bottom) / 2) < hLinePointY ? xBB.bottom : xBB.top; + + var yBase; + var yEndSpike; if(xMode.indexOf('toaxis') !== -1 || xMode.indexOf('across') !== -1) { - var yBase = yEdge; - var yEndSpike = yPoint; + if(xMode.indexOf('toaxis') !== -1) { + yBase = yEdge; + yEndSpike = hLinePointY; + } if(xMode.indexOf('across') !== -1) { yBase = xa._counterSpan[0]; yEndSpike = xa._counterSpan[1]; } - // Background vertical line (to x-axis) - container.append('line') + // Foreground vertical line (to x-axis) + container.insert('line', ':first-child') .attr({ - 'x1': xPoint, - 'x2': xPoint, + 'x1': vLinePointX, + 'x2': vLinePointX, 'y1': yBase, 'y2': yEndSpike, - 'stroke-width': xThickness + 2, - 'stroke': contrastColor + 'stroke-width': xThickness, + 'stroke': xColor, + 'stroke-dasharray': Drawing.dashStyle(xa.spikedash, xThickness) }) .classed('spikeline', true) .classed('crisp', true); - // Foreground vertical line (to x-axis) - container.append('line') + // Background vertical line (to x-axis) + container.insert('line', ':first-child') .attr({ - 'x1': xPoint, - 'x2': xPoint, + 'x1': vLinePointX, + 'x2': vLinePointX, 'y1': yBase, 'y2': yEndSpike, - 'stroke-width': xThickness, - 'stroke': xColor, - 'stroke-dasharray': Drawing.dashStyle(xa.spikedash, xThickness) + 'stroke-width': xThickness + 2, + 'stroke': contrastColor }) .classed('spikeline', true) .classed('crisp', true); @@ -1341,9 +1333,9 @@ function createSpikelines(hoverData, opts) { // X axis marker if(xMode.indexOf('marker') !== -1) { - container.append('circle') + container.insert('circle', ':first-child') .attr({ - 'cx': xPoint, + 'cx': vLinePointX, 'cy': yEdge - (xa.side !== 'top' ? xThickness : -xThickness), 'r': xThickness, 'fill': xColor @@ -1367,3 +1359,12 @@ function hoverChanged(gd, evt, oldhoverdata) { } return false; } + +function spikesChanged(gd, oldspikepoints) { + // don't emit any events if nothing changed + if(!oldspikepoints) return true; + if(oldspikepoints.vLinePoint !== gd._spikepoints.vLinePoint || + oldspikepoints.hLinePoint !== gd._spikepoints.hLinePoint + ) return true; + return false; +} diff --git a/src/components/fx/index.js b/src/components/fx/index.js index 7377186fa6a..77c618b8354 100644 --- a/src/components/fx/index.js +++ b/src/components/fx/index.js @@ -59,7 +59,6 @@ function loneUnhover(containerOrSelection) { selection.selectAll('g.hovertext').remove(); selection.selectAll('.spikeline').remove(); - selection.selectAll('.crossline').remove(); } // helpers for traces that use Fx.loneHover diff --git a/src/components/fx/layout_attributes.js b/src/components/fx/layout_attributes.js index 642a0b35853..54ecb74edd0 100644 --- a/src/components/fx/layout_attributes.js +++ b/src/components/fx/layout_attributes.js @@ -38,7 +38,28 @@ module.exports = { editType: 'modebar', description: 'Determines the mode of hover interactions.' }, - + hoverdistance: { + valType: 'integer', + min: -1, + dflt: 20, + role: 'style', + editType: 'none', + description: [ + 'Sets the default distance (in points) to look for data', + 'to add hover labels' + ].join(' ') + }, + spikedistance: { + valType: 'integer', + min: -1, + dflt: 20, + role: 'style', + editType: 'none', + description: [ + 'Sets the default distance (in points) to look for data', + 'to draw spikelines to. By default inherits from hoverdistance' + ].join(' ') + }, hoverlabel: { bgcolor: { valType: 'color', diff --git a/src/components/fx/layout_defaults.js b/src/components/fx/layout_defaults.js index a7839edb003..0768292f257 100644 --- a/src/components/fx/layout_defaults.js +++ b/src/components/fx/layout_defaults.js @@ -28,6 +28,8 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { else hovermodeDflt = 'closest'; coerce('hovermode', hovermodeDflt); + coerce('hoverdistance'); + coerce('spikedistance'); // if only mapbox or geo subplots is present on graph, // reset 'zoom' dragmode to 'pan' until 'zoom' is implemented, diff --git a/src/components/modebar/buttons.js b/src/components/modebar/buttons.js index 1d367df783b..b8fd69ad6f6 100644 --- a/src/components/modebar/buttons.js +++ b/src/components/modebar/buttons.js @@ -237,9 +237,6 @@ function handleCartesian(gd, ev) { if(astr === 'hovermode' && (val === 'x' || val === 'y')) { val = fullLayout._isHoriz ? 'y' : 'x'; button.setAttribute('data-val', val); - if(val !== 'closest') { - fullLayout._cartesianSpikesEnabled = 'off'; - } } else if(astr === 'hovermode' && val === 'closest') { for(i = 0; i < axList.length; i++) { ax = axList[i]; @@ -551,12 +548,10 @@ modeBarButtons.toggleSpikelines = { click: function(gd) { var fullLayout = gd._fullLayout; - fullLayout._cartesianSpikesEnabled = fullLayout.hovermode === 'closest' ? - (fullLayout._cartesianSpikesEnabled === 'on' ? 'off' : 'on') : 'on'; + fullLayout._cartesianSpikesEnabled = fullLayout._cartesianSpikesEnabled === 'on' ? 'off' : 'on'; var aobj = setSpikelineVisibility(gd); - aobj.hovermode = 'closest'; Plotly.relayout(gd, aobj); } }; diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 67429e258da..73d6e792fe8 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -166,9 +166,6 @@ Plotly.plot = function(gd, data, layout, config) { // save initial show spikes once per graph if(graphWasEmpty) Plotly.Axes.saveShowSpikeInitial(gd); - // save initial show crosslines once per graph - if(graphWasEmpty) Plotly.Axes.saveShowCrosslineInitial(gd); - // prepare the data and find the autorange // generate calcdata, if we need to diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 46c728ffa11..90a41451b08 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -423,30 +423,6 @@ axes.saveShowSpikeInitial = function(gd, overwrite) { return hasOneAxisChanged; }; -// save a copy of the initial crossline visibility -axes.saveShowCrosslineInitial = function(gd, overwrite) { - var axList = axes.list(gd, '', true), - hasOneAxisChanged = false; - - for(var i = 0; i < axList.length; i++) { - var ax = axList[i]; - - var isNew = (ax._showCrosslineInitial === undefined); - var hasChanged = ( - isNew || !( - ax.showcrossline === ax._showcrossline - ) - ); - - if((isNew) || (overwrite && hasChanged)) { - ax._showCrosslineInitial = ax.showcrossline; - hasOneAxisChanged = true; - } - - } - return hasOneAxisChanged; -}; - // axes.expand: if autoranging, include new data in the outer limits // for this axis // data is an array of numbers (ie already run through ax.d2c) diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js index 226786018b7..225c409e249 100644 --- a/src/plots/cartesian/layout_attributes.js +++ b/src/plots/cartesian/layout_attributes.js @@ -387,28 +387,14 @@ module.exports = { 'plotted on' ].join(' ') }, - showcrossline: { - valType: 'boolean', - dflt: false, - role: 'style', - editType: 'none', - description: 'Determines whether or not crossline are drawn for this axis.' - }, - crosslinecolor: { - valType: 'color', - dflt: null, - role: 'style', - editType: 'none', - description: 'Sets the crossline color. If undefined, will use the contrast to background color' - }, - crosslinethickness: { - valType: 'number', - dflt: 2, + spikesnap: { + valType: 'enumerated', + values: ['data', 'cursor'], + dflt: 'data', role: 'style', editType: 'none', - description: 'Sets the width (in px) of the zero line.' + description: 'Determines whether spikelines are stuck to the cursor or to the closest datapoints.' }, - crosslinedash: extendFlat({}, dash, {dflt: 'solid', editType: 'none'}), tickfont: fontAttrs({ editType: 'ticks', description: 'Sets the tick font.' diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index 9f95b77266c..275a1c19299 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -139,19 +139,13 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { handleAxisDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions, layoutOut); - var showCrossline = coerce('showcrossline'); - if(showCrossline) { - coerce('crosslinecolor'); - coerce('crosslinethickness'); - coerce('crosslinedash'); - } - var showSpikes = coerce('showspikes'); if(showSpikes) { coerce('spikecolor'); coerce('spikethickness'); coerce('spikedash'); coerce('spikemode'); + coerce('spikesnap'); } var positioningOptions = { diff --git a/test/jasmine/tests/hover_crossline_test.js b/test/jasmine/tests/hover_crossline_test.js deleted file mode 100644 index 6e054f3d61c..00000000000 --- a/test/jasmine/tests/hover_crossline_test.js +++ /dev/null @@ -1,179 +0,0 @@ -var d3 = require('d3'); - -var Plotly = require('@lib/index'); -var Fx = require('@src/components/fx'); -var Lib = require('@src/lib'); - -var fail = require('../assets/fail_test'); -var createGraphDiv = require('../assets/create_graph_div'); -var destroyGraphDiv = require('../assets/destroy_graph_div'); - -describe('crossline', function() { - 'use strict'; - - afterEach(destroyGraphDiv); - - describe('hover', function() { - var gd; - - function makeMock() { - var _mock = Lib.extendDeep({}, require('@mocks/19.json')); - _mock.layout.xaxis.showcrossline = true; - _mock.layout.yaxis.showcrossline = true; - _mock.layout.xaxis2.showcrossline = true; - _mock.layout.hovermode = 'closest'; - return _mock; - } - - function _hover(evt, subplot) { - Fx.hover(gd, evt, subplot); - Lib.clearThrottle(); - } - - function _assert(lineExpect) { - var TOL = 5; - var lines = d3.selectAll('line.crossline'); - - expect(lines.size()).toBe(lineExpect.length, '# of line nodes'); - - lines.each(function(_, i) { - var sel = d3.select(this); - ['x1', 'y1', 'x2', 'y2'].forEach(function(d, j) { - expect(sel.attr(d)) - .toBeWithin(lineExpect[i][j], TOL, 'line ' + i + ' attr ' + d); - }); - }); - } - - it('draws lines and markers on enabled axes in the closest hovermode', function(done) { - gd = createGraphDiv(); - var _mock = makeMock(); - - Plotly.plot(gd, _mock).then(function() { - _hover({xval: 2, yval: 3}, 'xy'); - _assert( - [[80, 250, 1036, 250], [557, 100, 557, 401]] - ); - }) - .then(function() { - _hover({xval: 30, yval: 40}, 'x2y2'); - _assert( - [[651, 167, 988, 167], [820, 115, 820, 220]] - ); - }) - .catch(fail) - .then(done); - }); - - it('draws lines and markers on enabled axes in the x hovermode', function(done) { - gd = createGraphDiv(); - var _mock = makeMock(); - - _mock.layout.hovermode = 'x'; - - Plotly.plot(gd, _mock).then(function() { - _hover({xval: 2, yval: 3}, 'xy'); - _assert( - [[80, 250, 1036, 250], [557, 100, 557, 401]] - ); - }) - .then(function() { - _hover({xval: 30, yval: 40}, 'x2y2'); - _assert( - [[651, 167, 988, 167], [820, 115, 820, 220]] - ); - }) - .catch(fail) - .then(done); - }); - - it('draws lines and markers on enabled axes in the y hovermode', function(done) { - gd = createGraphDiv(); - var _mock = makeMock(); - - _mock.layout.hovermode = 'y'; - - Plotly.plot(gd, _mock).then(function() { - _hover({xval: 2, yval: 3}, 'xy'); - _assert( - [[80, 250, 1036, 250], [557, 100, 557, 401]] - ); - }) - .then(function() { - _hover({xval: 30, yval: 40}, 'x2y2'); - _assert( - [[652, 167, 988, 167], [820, 115, 820, 220]] - ); - }) - .catch(fail) - .then(done); - }); - - it('does not draw lines and markers on enabled axes if spikes are enabled on the same axes', function(done) { - gd = createGraphDiv(); - var _mock = makeMock(); - - _mock.layout.xaxis.showspikes = true; - - Plotly.plot(gd, _mock).then(function() { - _hover({xval: 2, yval: 3}, 'xy'); - _assert( - [[80, 250, 1036, 250]] - ); - }) - .then(function() { - _hover({xval: 30, yval: 40}, 'x2y2'); - _assert( - [[652, 167, 988, 167]] - ); - }) - .catch(fail) - .then(done); - }); - - it('does not draw lines and markers on enabled axes if spikes are enabled on the same axes', function(done) { - gd = createGraphDiv(); - var _mock = makeMock(); - - _mock.layout.yaxis.showspikes = true; - - Plotly.plot(gd, _mock).then(function() { - _hover({xval: 2, yval: 3}, 'xy'); - _assert( - [[557, 100, 557, 401]] - ); - }) - .then(function() { - _hover({xval: 30, yval: 40}, 'x2y2'); - _assert( - [[820, 115, 820, 220]] - ); - }) - .catch(fail) - .then(done); - }); - - it('draws lines and markers on enabled axes w/o tick labels', function(done) { - gd = createGraphDiv(); - var _mock = makeMock(); - - _mock.layout.xaxis.showticklabels = false; - _mock.layout.yaxis.showticklabels = false; - - Plotly.plot(gd, _mock).then(function() { - _hover({xval: 2, yval: 3}, 'xy'); - _assert( - [[80, 250, 1036, 250], [557, 100, 557, 401]] - ); - }) - .then(function() { - _hover({xval: 30, yval: 40}, 'x2y2'); - _assert( - [[652, 167, 988, 167], [820, 115, 820, 220]] - ); - }) - .catch(fail) - .then(done); - }); - }); -}); diff --git a/test/jasmine/tests/modebar_test.js b/test/jasmine/tests/modebar_test.js index f17dbbd9f8e..ccda5983a18 100644 --- a/test/jasmine/tests/modebar_test.js +++ b/test/jasmine/tests/modebar_test.js @@ -861,14 +861,6 @@ describe('ModeBar', function() { }); describe('button toggleSpikelines', function() { - it('should update layout hovermode', function() { - expect(gd._fullLayout.hovermode).toBe('x'); - assertActive(hovermodeButtons, buttonCompare); - - buttonToggle.click(); - expect(gd._fullLayout.hovermode).toBe('closest'); - assertActive(hovermodeButtons, buttonClosest); - }); it('should makes spikelines visible', function() { buttonToggle.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); @@ -876,23 +868,6 @@ describe('ModeBar', function() { buttonToggle.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); }); - it('should become disabled when hovermode is switched off closest', function() { - buttonToggle.click(); - expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); - - buttonCompare.click(); - expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); - }); - it('should be re-enabled when hovermode is set to closest if it was previously on', function() { - buttonToggle.click(); - expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); - - buttonCompare.click(); - expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); - - buttonClosest.click(); - expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); - }); }); }); From e5875a9651e6d7608200df832327e46f52d60428 Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Thu, 7 Dec 2017 15:44:55 +0300 Subject: [PATCH 06/12] fix tests related to new spike mode --- src/components/fx/hover.js | 149 ++++++++++++++------- src/components/fx/layout_attributes.js | 6 +- test/jasmine/tests/hover_spikeline_test.js | 4 +- 3 files changed, 105 insertions(+), 54 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 55c5d711f9c..e8f8a72e634 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -24,6 +24,7 @@ var Registry = require('../../registry'); var helpers = require('./helpers'); var constants = require('./constants'); +var getTraceColor = require('../../traces/scatter/get_trace_color'); // hover labels for multiple horizontal bars get tilted by some angle, // then need to be offset differently if they overlap @@ -234,8 +235,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { pointData, closedataPreviousLength, - // closestPoints: the set of candidate points we've found to draw spikes to - closestPoints = { + // spikePoints: the set of candidate points we've found to draw spikes to + spikePoints = { hLinePoint: null, vLinePoint: null }; @@ -331,8 +332,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { // within one trace mode can sometimes be overridden mode = hovermode; - var hoverdistance = fullLayout.hoverdistance ? fullLayout.hoverdistance === -1 ? Infinity : fullLayout.hoverdistance : constants.MAXDIST; - var spikedistance = fullLayout.spikedistance ? fullLayout.spikedistance === -1 ? Infinity : fullLayout.spikedistance : hoverdistance; + var hoverdistance = fullLayout.hoverdistance ? fullLayout.hoverdistance === 0 ? Infinity : fullLayout.hoverdistance : constants.MAXDIST; + var spikedistance = fullLayout.spikedistance ? fullLayout.spikedistance === 0 ? Infinity : fullLayout.spikedistance : hoverdistance; // container for new point, also used to pass info into module.hoverPoints pointData = { @@ -391,11 +392,11 @@ function _hover(gd, evt, subplot, noHoverEvent) { // Find the points for the spikes first to avoid overwriting the hoverLabels data. if(fullLayout._has('cartesian')) { if(fullLayout.hovermode === 'closest') { - closestPoints.hLinePoint = findClosestPoint(pointData, xval, yval, 'closest', closestPoints.hLinePoint, spikedistance); - closestPoints.vLinePoint = findClosestPoint(pointData, xval, yval, 'closest', closestPoints.vLinePoint, spikedistance); + spikePoints.hLinePoint = setClosestPoint(pointData, xval, yval, 'closest', spikePoints.hLinePoint, spikedistance, 'h'); + spikePoints.vLinePoint = setClosestPoint(pointData, xval, yval, 'closest', spikePoints.vLinePoint, spikedistance, 'v'); } else { - closestPoints.hLinePoint = findClosestPoint(pointData, xval, yval, 'y', closestPoints.hLinePoint, spikedistance); - closestPoints.vLinePoint = findClosestPoint(pointData, xval, yval, 'x', closestPoints.vLinePoint, spikedistance); + spikePoints.hLinePoint = setClosestPoint(pointData, xval, yval, 'y', spikePoints.hLinePoint, spikedistance, 'h'); + spikePoints.vLinePoint = setClosestPoint(pointData, xval, yval, 'x', spikePoints.vLinePoint, spikedistance, 'v'); } } @@ -424,21 +425,72 @@ function _hover(gd, evt, subplot, noHoverEvent) { } } - function findClosestPoint(pointData, xval, yval, mode, endPoint, spikedistance) { + function findClosestPoints(pointData, xval, yval, hovermode) { + var cd = pointData.cd, + trace = cd[0].trace, + xa = pointData.xa, + ya = pointData.ya, + xpx = xa.c2p(xval), + ypx = ya.c2p(yval), + hoveron = trace.hoveron || ''; + + if(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 = helpers.getDistanceFunction(hovermode, dx, dy, dxy); + + helpers.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, + y0: yc - rad, + y1: yc + rad, + }); + return [pointData]; + } + } + } + + function setClosestPoint(pointData, xval, yval, mode, endPoint, spikedistance, type) { var tmpDistance = pointData.distance; var tmpIndex = pointData.index; var resultPoint = endPoint; pointData.distance = spikedistance; pointData.index = false; - var closestPoints = trace._module.hoverPoints(pointData, xval, yval, mode); + var closestPoints = findClosestPoints(pointData, xval, yval, mode); if(closestPoints) { closestPoints = closestPoints.filter(function(point) { - if(mode === 'x') { + if(type === 'v') { return point.xa.showspikes; - } else if(mode === 'y') { + } else if(type === 'h') { return point.ya.showspikes; - } else if(mode === 'closest') { - return point.ya.showspikes && point.ya.showspikes; } }); if(closestPoints.length) { @@ -477,16 +529,16 @@ function _hover(gd, evt, subplot, noHoverEvent) { }; var oldspikepoints = gd._spikepoints, newspikepoints = { - vLinePoint: closestPoints.vLinePoint, - hLinePoint: closestPoints.hLinePoint + vLinePoint: spikePoints.vLinePoint, + hLinePoint: spikePoints.hLinePoint }; gd._spikepoints = newspikepoints; if(hoverData.length === 0) { var result = dragElement.unhoverRaw(gd, evt); - if(fullLayout._has('cartesian') && ((closestPoints.hLinePoint !== null) || (closestPoints.vLinePoint !== null))) { + if(fullLayout._has('cartesian') && ((spikePoints.hLinePoint !== null) || (spikePoints.vLinePoint !== null))) { if(spikesChanged(oldspikepoints)) { - createSpikelines2(closestPoints, spikelineOpts); + createSpikelines(spikePoints, spikelineOpts); } } return result; @@ -494,7 +546,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { if(fullLayout._has('cartesian')) { if(spikesChanged(oldspikepoints)) { - createSpikelines2(closestPoints, spikelineOpts); + createSpikelines(spikePoints, spikelineOpts); } } @@ -1170,17 +1222,13 @@ function cleanPoint(d, hovermode) { return d; } -function createSpikelines2(closestPoints, opts) { +function createSpikelines(closestPoints, opts) { var hovermode = opts.hovermode; var container = opts.container; var fullLayout = opts.fullLayout; var evt = opts.event; - var hLinePoint, - vLinePoint, - xa, - ya, - hLinePointY, - vLinePointX; + var xa, + ya; var showY = closestPoints.hLinePoint ? true : false; var showX = closestPoints.vLinePoint ? true : false; @@ -1190,31 +1238,22 @@ function createSpikelines2(closestPoints, opts) { if(!(showX || showY)) return; + var contrastColor = Color.combine(fullLayout.plot_bgcolor, fullLayout.paper_bgcolor); + + // Horizontal line (to y-axis) if(showY) { - hLinePoint = closestPoints.hLinePoint; + var hLinePoint = closestPoints.hLinePoint; + xa = hLinePoint && hLinePoint.xa; ya = hLinePoint && hLinePoint.ya; - var ySnap = ya.spikesnap; + var ySnap = ya.spikesnap, + hLinePointX, + hLinePointY; if(ySnap === 'cursor') { hLinePointY = evt.offsetY; } else { hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2; } - } - if(showX) { - vLinePoint = closestPoints.vLinePoint; - xa = vLinePoint && vLinePoint.xa; - var xSnap = xa.spikesnap; - if(xSnap === 'cursor') { - vLinePointX = evt.offsetX; - } else { - vLinePointX = xa._offset + (vLinePoint.x0 + vLinePoint.x1) / 2; - } - } - - var contrastColor = Color.combine(fullLayout.plot_bgcolor, fullLayout.paper_bgcolor); - - // Horizontal line (to y-axis) - if(showY) { + hLinePointX = xa._offset + (hLinePoint.x0 + hLinePoint.x1) / 2; var dfltHLineColor = tinycolor.readability(hLinePoint.color, contrastColor) < 1.5 ? Color.contrast(contrastColor) : hLinePoint.color; var yMode = ya.spikemode; @@ -1224,14 +1263,14 @@ function createSpikelines2(closestPoints, opts) { var yThickness = ya.spikethickness; var yColor = ya.spikecolor || dfltHLineColor; var yBB = ya._boundingBox; - var xEdge = ((yBB.left + yBB.right) / 2) < vLinePointX ? yBB.right : yBB.left; + var xEdge = ((yBB.left + yBB.right) / 2) < hLinePointX ? yBB.right : yBB.left; var xBase; var xEndSpike; if(yMode.indexOf('toaxis') !== -1 || yMode.indexOf('across') !== -1) { if(yMode.indexOf('toaxis') !== -1) { xBase = xEdge; - xEndSpike = vLinePointX; + xEndSpike = hLinePointX; } if(yMode.indexOf('across') !== -1) { xBase = ya._counterSpan[0]; @@ -1279,8 +1318,20 @@ function createSpikelines2(closestPoints, opts) { } if(showX) { + var vLinePoint = closestPoints.vLinePoint; + xa = vLinePoint && vLinePoint.xa; + ya = vLinePoint && vLinePoint.ya; + var xSnap = xa.spikesnap, + vLinePointX, + vLinePointY; + if(xSnap === 'cursor') { + vLinePointX = evt.offsetX; + } else { + vLinePointX = xa._offset + (vLinePoint.x0 + vLinePoint.x1) / 2; + } + vLinePointY = ya._offset + (vLinePoint.y0 + vLinePoint.y1) / 2; var dfltVLineColor = tinycolor.readability(vLinePoint.color, contrastColor) < 1.5 ? - Color.contrast(contrastColor) : vLinePoint.color; + Color.contrast(contrastColor) : vLinePoint.color; var xMode = xa.spikemode; if(hovermode !== 'closest' && xMode.indexOf('toaxis') !== -1) { xMode = xMode.replace('toaxis', 'across'); @@ -1288,7 +1339,7 @@ function createSpikelines2(closestPoints, opts) { var xThickness = xa.spikethickness; var xColor = xa.spikecolor || dfltVLineColor; var xBB = xa._boundingBox; - var yEdge = ((xBB.top + xBB.bottom) / 2) < hLinePointY ? xBB.bottom : xBB.top; + var yEdge = ((xBB.top + xBB.bottom) / 2) < vLinePointY ? xBB.bottom : xBB.top; var yBase; var yEndSpike; @@ -1296,7 +1347,7 @@ function createSpikelines2(closestPoints, opts) { if(xMode.indexOf('toaxis') !== -1 || xMode.indexOf('across') !== -1) { if(xMode.indexOf('toaxis') !== -1) { yBase = yEdge; - yEndSpike = hLinePointY; + yEndSpike = vLinePointY; } if(xMode.indexOf('across') !== -1) { yBase = xa._counterSpan[0]; diff --git a/src/components/fx/layout_attributes.js b/src/components/fx/layout_attributes.js index 54ecb74edd0..3cbdce25e48 100644 --- a/src/components/fx/layout_attributes.js +++ b/src/components/fx/layout_attributes.js @@ -46,7 +46,7 @@ module.exports = { editType: 'none', description: [ 'Sets the default distance (in points) to look for data', - 'to add hover labels' + 'to add hover labels (zero means no cutoff)' ].join(' ') }, spikedistance: { @@ -56,8 +56,8 @@ module.exports = { role: 'style', editType: 'none', description: [ - 'Sets the default distance (in points) to look for data', - 'to draw spikelines to. By default inherits from hoverdistance' + 'Sets the default distance (in points) to look for data to draw', + 'spikelines to (zero means no cutoff). By default inherits from hoverdistance' ].join(' ') }, hoverlabel: { diff --git a/test/jasmine/tests/hover_spikeline_test.js b/test/jasmine/tests/hover_spikeline_test.js index aca3c9f29d2..6fe5fae9513 100644 --- a/test/jasmine/tests/hover_spikeline_test.js +++ b/test/jasmine/tests/hover_spikeline_test.js @@ -65,7 +65,7 @@ describe('spikeline', function() { Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); _assert( - [[80, 250, 557, 250], [80, 250, 557, 250], [557, 401, 557, 250], [557, 401, 557, 250]], + [[557, 401, 557, 250], [557, 401, 557, 250], [80, 250, 557, 250], [80, 250, 557, 250]], [[83, 250]] ); }) @@ -90,7 +90,7 @@ describe('spikeline', function() { Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); _assert( - [[80, 250, 557, 250], [80, 250, 557, 250], [557, 401, 557, 250], [557, 401, 557, 250]], + [[557, 401, 557, 250], [557, 401, 557, 250], [80, 250, 557, 250], [80, 250, 557, 250]], [[83, 250]] ); }) From 37e419f8e94691ee2df579c82367cb88c4fe13bb Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Thu, 7 Dec 2017 16:50:15 +0300 Subject: [PATCH 07/12] fix hoverdistance and spikedistance when it set to 0 --- src/components/fx/hover.js | 4 ++-- src/components/fx/layout_attributes.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index e8f8a72e634..8dd907b6064 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -332,8 +332,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { // within one trace mode can sometimes be overridden mode = hovermode; - var hoverdistance = fullLayout.hoverdistance ? fullLayout.hoverdistance === 0 ? Infinity : fullLayout.hoverdistance : constants.MAXDIST; - var spikedistance = fullLayout.spikedistance ? fullLayout.spikedistance === 0 ? Infinity : fullLayout.spikedistance : hoverdistance; + var hoverdistance = fullLayout.hoverdistance === 0 ? Infinity : fullLayout.hoverdistance; + var spikedistance = fullLayout.spikedistance === 0 ? Infinity : fullLayout.spikedistance; // container for new point, also used to pass info into module.hoverPoints pointData = { diff --git a/src/components/fx/layout_attributes.js b/src/components/fx/layout_attributes.js index 3cbdce25e48..0bea247ba23 100644 --- a/src/components/fx/layout_attributes.js +++ b/src/components/fx/layout_attributes.js @@ -40,7 +40,7 @@ module.exports = { }, hoverdistance: { valType: 'integer', - min: -1, + min: 0, dflt: 20, role: 'style', editType: 'none', @@ -51,13 +51,13 @@ module.exports = { }, spikedistance: { valType: 'integer', - min: -1, + min: 0, dflt: 20, role: 'style', editType: 'none', description: [ 'Sets the default distance (in points) to look for data to draw', - 'spikelines to (zero means no cutoff). By default inherits from hoverdistance' + 'spikelines to (zero means no cutoff).' ].join(' ') }, hoverlabel: { From 911ae0e51516a29fbde016715e4dd12c7f3c34cf Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Tue, 19 Dec 2017 11:20:21 +0300 Subject: [PATCH 08/12] re-work spikeline based on PR review --- src/components/fx/hover.js | 264 ++++++++++----------- test/jasmine/tests/hover_label_test.js | 161 +++++++++++++ test/jasmine/tests/hover_spikeline_test.js | 181 +++++++++++++- 3 files changed, 462 insertions(+), 144 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 8dd907b6064..6d32e86bbd0 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -24,7 +24,6 @@ var Registry = require('../../registry'); var helpers = require('./helpers'); var constants = require('./constants'); -var getTraceColor = require('../../traces/scatter/get_trace_color'); // hover labels for multiple horizontal bars get tilted by some angle, // then need to be offset differently if they overlap @@ -145,7 +144,8 @@ exports.loneHover = function loneHover(hoverItem, opts) { rotateLabels: false, bgColor: opts.bgColor || Color.background, container: container3, - outerContainer: outerContainer3 + outerContainer: outerContainer3, + hoverdistance: constants.MAXDIST }; var hoverLabel = createHoverText([pointData], fullOpts, opts.gd); @@ -208,6 +208,9 @@ function _hover(gd, evt, subplot, noHoverEvent) { return dragElement.unhoverRaw(gd, evt); } + var hoverdistance = fullLayout.hoverdistance === 0 ? Infinity : fullLayout.hoverdistance; + var spikedistance = fullLayout.spikedistance === 0 ? Infinity : fullLayout.spikedistance; + // hoverData: the set of candidate points we've found to highlight var hoverData = [], @@ -332,9 +335,6 @@ function _hover(gd, evt, subplot, noHoverEvent) { // within one trace mode can sometimes be overridden mode = hovermode; - var hoverdistance = fullLayout.hoverdistance === 0 ? Infinity : fullLayout.hoverdistance; - var spikedistance = fullLayout.spikedistance === 0 ? Infinity : fullLayout.spikedistance; - // container for new point, also used to pass info into module.hoverPoints pointData = { // trace properties @@ -389,17 +389,6 @@ function _hover(gd, evt, subplot, noHoverEvent) { yval = yvalArray[subploti]; } - // Find the points for the spikes first to avoid overwriting the hoverLabels data. - if(fullLayout._has('cartesian')) { - if(fullLayout.hovermode === 'closest') { - spikePoints.hLinePoint = setClosestPoint(pointData, xval, yval, 'closest', spikePoints.hLinePoint, spikedistance, 'h'); - spikePoints.vLinePoint = setClosestPoint(pointData, xval, yval, 'closest', spikePoints.vLinePoint, spikedistance, 'v'); - } else { - spikePoints.hLinePoint = setClosestPoint(pointData, xval, yval, 'y', spikePoints.hLinePoint, spikedistance, 'h'); - spikePoints.vLinePoint = setClosestPoint(pointData, xval, yval, 'x', spikePoints.vLinePoint, spikedistance, 'v'); - } - } - // Now find the points. if(trace._module && trace._module.hoverPoints) { var newPoints = trace._module.hoverPoints(pointData, xval, yval, mode, fullLayout._hoverlayer); @@ -423,105 +412,96 @@ function _hover(gd, evt, subplot, noHoverEvent) { hoverData.splice(0, closedataPreviousLength); distance = hoverData[0].distance; } - } - function findClosestPoints(pointData, xval, yval, hovermode) { - var cd = pointData.cd, - trace = cd[0].trace, - xa = pointData.xa, - ya = pointData.ya, - xpx = xa.c2p(xval), - ypx = ya.c2p(yval), - hoveron = trace.hoveron || ''; - - if(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 = helpers.getDistanceFunction(hovermode, dx, dy, dxy); - - helpers.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, - y0: yc - rad, - y1: yc + rad, - }); - return [pointData]; + // Now look for the points to draw the spikelines + // Do it only if there is no hoverData + if(fullLayout._has('cartesian')) { + if(hoverData.length === 0) { + pointData.distance = spikedistance; + pointData.index = false; + var closestPoints = trace._module.hoverPoints(pointData, xval, yval, 'closest'); + if(closestPoints) { + 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 = clearClosestPoint(closestVPt); + if(!spikePoints.vLinePoint || (spikePoints.vLinePoint.distance > tmpPoint.distance)) { + spikePoints.vLinePoint = tmpPoint; + } + } + } + + var closestHPoints = closestPoints.filter(function(point) { + return point.ya.showspikes; + }); + if(closestHPoints.length) { + var closestHPt = closestHPoints[0]; + if(isNumeric(closestHPt.x0) && isNumeric(closestHPt.y0)) { + tmpPoint = clearClosestPoint(closestHPt); + if(!spikePoints.hLinePoint || (spikePoints.hLinePoint.distance > tmpPoint.distance)) { + spikePoints.hLinePoint = tmpPoint; + } + } + } + } } } } - function setClosestPoint(pointData, xval, yval, mode, endPoint, spikedistance, type) { - var tmpDistance = pointData.distance; - var tmpIndex = pointData.index; - var resultPoint = endPoint; - pointData.distance = spikedistance; - pointData.index = false; - var closestPoints = findClosestPoints(pointData, xval, yval, mode); - if(closestPoints) { - closestPoints = closestPoints.filter(function(point) { - if(type === 'v') { - return point.xa.showspikes; - } else if(type === 'h') { - return point.ya.showspikes; - } + function selectClosestPoint(pointsData, spikedistance) { + if(!pointsData.length) return null; + var resultPoint; + var pointsDistances = pointsData.map(function(point, index) { + var xa = point.xa, + ya = point.ya, + xpx = xa.c2p(xval), + ypx = ya.c2p(yval), + dxy = function(point) { + var rad = Math.max(3, point.mrc || 0), + dx = (point.x1 + point.x0) / 2 - xpx, + dy = (point.y1 + point.y0) / 2 - ypx; + return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad); + }; + var distance = dxy(point); + return {distance: distance, index: index}; + }); + pointsDistances = pointsDistances + .filter(function(point) { + return point.distance <= spikedistance; + }) + .sort(function(a, b) { + return a.distance - b.distance; }); - if(closestPoints.length) { - var closestPt = closestPoints[0]; - if(isNumeric(closestPt.x0) && isNumeric(closestPt.y0)) { - var tmpPoint = { - xa: closestPt.xa, - ya: closestPt.ya, - x0: closestPt.x0, - x1: closestPt.x1, - y0: closestPt.y0, - y1: closestPt.y1, - distance: closestPt.distance, - curveNumber: closestPt.trace.index, - color: closestPt.color, - pointNumber: closestPt.index - }; - if(!resultPoint || (resultPoint.distance > tmpPoint.distance)) { - resultPoint = tmpPoint; - } - } - } + if(pointsDistances.length) { + resultPoint = pointsData[pointsDistances[0].index]; + } else { + resultPoint = null; } - pointData.index = tmpIndex; - pointData.distance = tmpDistance; return resultPoint; } + function clearClosestPoint(point) { + if(!point) return null; + return { + xa: point.xa, + ya: point.ya, + x0: point.x0, + x1: point.x1, + y0: point.y0, + y1: point.y1, + distance: point.distance, + curveNumber: point.trace.index, + color: point.color, + pointNumber: point.index + }; + } + // if hoverData is empty check for the spikes to draw and quit if there are none var spikelineOpts = { - hovermode: hovermode, fullLayout: fullLayout, container: fullLayout._hoverlayer, outerContainer: fullLayout._paperdiv, @@ -534,6 +514,22 @@ function _hover(gd, evt, subplot, noHoverEvent) { }; gd._spikepoints = newspikepoints; + if(fullLayout._has('cartesian')) { + if(hoverData.length !== 0) { + var tmpHPointData = hoverData.filter(function(point) { + return point.ya.showspikes; + }); + var tmpHPoint = selectClosestPoint(tmpHPointData, spikedistance); + spikePoints.hLinePoint = clearClosestPoint(tmpHPoint); + + var tmpVPointData = hoverData.filter(function(point) { + return point.xa.showspikes; + }); + var tmpVPoint = selectClosestPoint(tmpVPointData, spikedistance); + spikePoints.vLinePoint = clearClosestPoint(tmpVPoint); + } + } + if(hoverData.length === 0) { var result = dragElement.unhoverRaw(gd, evt); if(fullLayout._has('cartesian') && ((spikePoints.hLinePoint !== null) || (spikePoints.vLinePoint !== null))) { @@ -580,7 +576,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { bgColor: bgColor, container: fullLayout._hoverlayer, outerContainer: fullLayout._paperdiv, - commonLabelOpts: fullLayout.hoverlabel + commonLabelOpts: fullLayout.hoverlabel, + hoverdistance: fullLayout.hoverdistance }; var hoverLabels = createHoverText(hoverData, labelOpts, gd); @@ -644,7 +641,7 @@ 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 = c0.distance <= constants.MAXDIST && + var showCommonLabel = c0.distance <= opts.hoverdistance && (hovermode === 'x' || hovermode === 'y'); // all hover traces hoverinfo must contain the hovermode @@ -1223,7 +1220,6 @@ function cleanPoint(d, hovermode) { } function createSpikelines(closestPoints, opts) { - var hovermode = opts.hovermode; var container = opts.container; var fullLayout = opts.fullLayout; var evt = opts.event; @@ -1242,12 +1238,13 @@ function createSpikelines(closestPoints, opts) { // Horizontal line (to y-axis) if(showY) { - var hLinePoint = closestPoints.hLinePoint; - xa = hLinePoint && hLinePoint.xa; - ya = hLinePoint && hLinePoint.ya; - var ySnap = ya.spikesnap, + var hLinePoint = closestPoints.hLinePoint, hLinePointX, hLinePointY; + xa = hLinePoint && hLinePoint.xa; + ya = hLinePoint && hLinePoint.ya; + var ySnap = ya.spikesnap; + if(ySnap === 'cursor') { hLinePointY = evt.offsetY; } else { @@ -1256,16 +1253,13 @@ function createSpikelines(closestPoints, opts) { hLinePointX = xa._offset + (hLinePoint.x0 + hLinePoint.x1) / 2; var dfltHLineColor = tinycolor.readability(hLinePoint.color, contrastColor) < 1.5 ? Color.contrast(contrastColor) : hLinePoint.color; - var yMode = ya.spikemode; - if(hovermode !== 'closest' && yMode.indexOf('toaxis') !== -1) { - yMode = yMode.replace('toaxis', 'across'); - } - var yThickness = ya.spikethickness; - var yColor = ya.spikecolor || dfltHLineColor; - var yBB = ya._boundingBox; - var xEdge = ((yBB.left + yBB.right) / 2) < hLinePointX ? yBB.right : yBB.left; - var xBase; - var xEndSpike; + var yMode = ya.spikemode, + yThickness = ya.spikethickness, + yColor = ya.spikecolor || dfltHLineColor, + yBB = ya._boundingBox, + xEdge = ((yBB.left + yBB.right) / 2) < hLinePointX ? yBB.right : yBB.left, + xBase, + xEndSpike; if(yMode.indexOf('toaxis') !== -1 || yMode.indexOf('across') !== -1) { if(yMode.indexOf('toaxis') !== -1) { @@ -1318,12 +1312,14 @@ function createSpikelines(closestPoints, opts) { } if(showX) { - var vLinePoint = closestPoints.vLinePoint; - xa = vLinePoint && vLinePoint.xa; - ya = vLinePoint && vLinePoint.ya; - var xSnap = xa.spikesnap, + var vLinePoint = closestPoints.vLinePoint, vLinePointX, vLinePointY; + + xa = vLinePoint && vLinePoint.xa; + ya = vLinePoint && vLinePoint.ya; + var xSnap = xa.spikesnap; + if(xSnap === 'cursor') { vLinePointX = evt.offsetX; } else { @@ -1331,18 +1327,14 @@ function createSpikelines(closestPoints, opts) { } vLinePointY = ya._offset + (vLinePoint.y0 + vLinePoint.y1) / 2; var dfltVLineColor = tinycolor.readability(vLinePoint.color, contrastColor) < 1.5 ? - Color.contrast(contrastColor) : vLinePoint.color; - var xMode = xa.spikemode; - if(hovermode !== 'closest' && xMode.indexOf('toaxis') !== -1) { - xMode = xMode.replace('toaxis', 'across'); - } - var xThickness = xa.spikethickness; - var xColor = xa.spikecolor || dfltVLineColor; - var xBB = xa._boundingBox; - var yEdge = ((xBB.top + xBB.bottom) / 2) < vLinePointY ? xBB.bottom : xBB.top; - - var yBase; - var yEndSpike; + Color.contrast(contrastColor) : vLinePoint.color; + var xMode = xa.spikemode, + xThickness = xa.spikethickness, + xColor = xa.spikecolor || dfltVLineColor, + xBB = xa._boundingBox, + yEdge = ((xBB.top + xBB.bottom) / 2) < vLinePointY ? xBB.bottom : xBB.top, + yBase, + yEndSpike; if(xMode.indexOf('toaxis') !== -1 || xMode.indexOf('across') !== -1) { if(xMode.indexOf('toaxis') !== -1) { diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 74238e593df..97d1e320435 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1723,3 +1723,164 @@ describe('ohlc hover interactions', function() { expect(d3.select('.hovertext').size()).toBe(1); }); }); + +describe('hover distance', function() { + 'use strict'; + + var mock = require('@mocks/19.json'); + + afterEach(destroyGraphDiv); + + describe('closest hovermode', function() { + var mockCopy = Lib.extendDeep({}, mock); + mockCopy.layout.hovermode = 'closest'; + + beforeEach(function(done) { + Plotly.plot(createGraphDiv(), mockCopy.data, mockCopy.layout).then(done); + }); + + it('does not render if distance to the point is larger than default (>20)', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 475, ypx: 175 }; + Fx.hover('graph', evt, 'xy'); + + expect(gd._hoverdata).toEqual(undefined); + }); + + it('render if distance to the point is less than default (<20)', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 475, ypx: 155 }; + Fx.hover('graph', evt, 'xy'); + + var hoverTrace = gd._hoverdata[0]; + + expect(hoverTrace.curveNumber).toEqual(0); + expect(hoverTrace.pointNumber).toEqual(1); + expect(hoverTrace.x).toEqual(2); + expect(hoverTrace.y).toEqual(3); + + assertHoverLabelContent({ + nums: '(2, 3)', + name: 'trace 0' + }); + }); + + it('responds to hoverdistance change', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 475, ypx: 180 }; + gd._fullLayout.hoverdistance = 30; + + Fx.hover('graph', evt, 'xy'); + + var hoverTrace = gd._hoverdata[0]; + + expect(hoverTrace.curveNumber).toEqual(0); + expect(hoverTrace.pointNumber).toEqual(1); + expect(hoverTrace.x).toEqual(2); + expect(hoverTrace.y).toEqual(3); + + assertHoverLabelContent({ + nums: '(2, 3)', + name: 'trace 0' + }); + }); + + it('correctly responds to setting the hoverdistance to 0 by increasing ' + + 'the range of search for points to hover to Infinity', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 475, ypx: 180 }; + gd._fullLayout.hoverdistance = 0; + + Fx.hover('graph', evt, 'xy'); + + var hoverTrace = gd._hoverdata[0]; + + expect(hoverTrace.curveNumber).toEqual(0); + expect(hoverTrace.pointNumber).toEqual(1); + expect(hoverTrace.x).toEqual(2); + expect(hoverTrace.y).toEqual(3); + + assertHoverLabelContent({ + nums: '(2, 3)', + name: 'trace 0' + }); + }); + }); + + describe('x hovermode', function() { + var mockCopy = Lib.extendDeep({}, mock); + mockCopy.layout.hovermode = 'x'; + + beforeEach(function(done) { + Plotly.plot(createGraphDiv(), mockCopy.data, mockCopy.layout).then(done); + }); + + it('does not render if distance to the point is larger than default (>20)', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 450, ypx: 155 }; + Fx.hover('graph', evt, 'xy'); + + expect(gd._hoverdata).toEqual(undefined); + }); + + it('render if distance to the point is less than default (<20)', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 475, ypx: 155 }; + Fx.hover('graph', evt, 'xy'); + + var hoverTrace = gd._hoverdata[0]; + + expect(hoverTrace.curveNumber).toEqual(0); + expect(hoverTrace.pointNumber).toEqual(1); + expect(hoverTrace.x).toEqual(2); + expect(hoverTrace.y).toEqual(3); + + assertHoverLabelContent({ + nums: '3', + axis: '2', + name: 'trace 0' + }); + }); + + it('responds to hoverdistance change', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 450, ypx: 155 }; + gd._fullLayout.hoverdistance = 30; + + Fx.hover('graph', evt, 'xy'); + + var hoverTrace = gd._hoverdata[0]; + + expect(hoverTrace.curveNumber).toEqual(0); + expect(hoverTrace.pointNumber).toEqual(1); + expect(hoverTrace.x).toEqual(2); + expect(hoverTrace.y).toEqual(3); + + assertHoverLabelContent({ + nums: '(2, 3)', + name: 'trace 0' + }); + }); + + it('correctly responds to setting the hoverdistance to 0 by increasing ' + + 'the range of search for points to hover to Infinity', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 450, ypx: 155 }; + gd._fullLayout.hoverdistance = 0; + + Fx.hover('graph', evt, 'xy'); + + var hoverTrace = gd._hoverdata[0]; + + expect(hoverTrace.curveNumber).toEqual(0); + expect(hoverTrace.pointNumber).toEqual(1); + expect(hoverTrace.x).toEqual(2); + expect(hoverTrace.y).toEqual(3); + + assertHoverLabelContent({ + nums: '(2, 3)', + name: 'trace 0' + }); + }); + }); +}); diff --git a/test/jasmine/tests/hover_spikeline_test.js b/test/jasmine/tests/hover_spikeline_test.js index 6fe5fae9513..e9de301d614 100644 --- a/test/jasmine/tests/hover_spikeline_test.js +++ b/test/jasmine/tests/hover_spikeline_test.js @@ -16,15 +16,15 @@ describe('spikeline', function() { describe('hover', function() { var gd; - function makeMock() { + function makeMock(spikemode, hovermode) { var _mock = Lib.extendDeep({}, require('@mocks/19.json')); _mock.layout.xaxis.showspikes = true; - _mock.layout.xaxis.spikemode = 'toaxis'; + _mock.layout.xaxis.spikemode = spikemode; _mock.layout.yaxis.showspikes = true; - _mock.layout.yaxis.spikemode = 'toaxis+marker'; + _mock.layout.yaxis.spikemode = spikemode + '+marker'; _mock.layout.xaxis2.showspikes = true; - _mock.layout.xaxis2.spikemode = 'toaxis'; - _mock.layout.hovermode = 'closest'; + _mock.layout.xaxis2.spikemode = spikemode; + _mock.layout.hovermode = hovermode; return _mock; } @@ -33,6 +33,14 @@ describe('spikeline', function() { Lib.clearThrottle(); } + function _set_hovermode(hovermode) { + gd._fullLayout.hovermode = hovermode; + } + + function _set_spikedistance(spikedistance) { + gd._fullLayout.spikedistance = spikedistance; + } + function _assert(lineExpect, circleExpect) { var TOL = 5; var lines = d3.selectAll('line.spikeline'); @@ -58,9 +66,9 @@ describe('spikeline', function() { }); } - it('draws lines and markers on enabled axes', function(done) { + it('draws lines and markers on enabled axes in the closest hovermode', function(done) { gd = createGraphDiv(); - var _mock = makeMock(); + var _mock = makeMock('toaxis', 'closest'); Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); @@ -82,7 +90,7 @@ describe('spikeline', function() { it('draws lines and markers on enabled axes w/o tick labels', function(done) { gd = createGraphDiv(); - var _mock = makeMock(); + var _mock = makeMock('toaxis', 'closest'); _mock.layout.xaxis.showticklabels = false; _mock.layout.yaxis.showticklabels = false; @@ -104,5 +112,162 @@ describe('spikeline', function() { .catch(fail) .then(done); }); + + it('draws lines and markers on enabled axes in the x hovermode', function(done) { + gd = createGraphDiv(); + var _mock = makeMock('across', 'x'); + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[557, 100, 557, 401], [557, 100, 557, 401], [80, 250, 1033, 250], [80, 250, 1033, 250]], + [[83, 250]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[820, 116, 820, 220], [820, 116, 820, 220]], + [] + ); + }) + .catch(fail) + .then(done); + }); + + it('automatically switch between toaxis and across spikemodes on switching the hovermodes', function(done) { + gd = createGraphDiv(); + var _mock = makeMock('toaxis', 'closest'); + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[557, 401, 557, 250], [557, 401, 557, 250], [80, 250, 557, 250], [80, 250, 557, 250]], + [[83, 250]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[820, 220, 820, 167], [820, 220, 820, 167]], + [] + ); + }) + .then(function() { + _set_hovermode('x'); + }) + .then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[557, 100, 557, 401], [557, 100, 557, 401], [80, 250, 1033, 250], [80, 250, 1033, 250]], + [[83, 250]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[820, 115, 820, 220], [820, 115, 820, 220]], + [] + ); + }) + .then(function() { + _set_hovermode('closest'); + }) + .then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[557, 401, 557, 250], [557, 401, 557, 250], [80, 250, 557, 250], [80, 250, 557, 250]], + [[83, 250]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[820, 220, 820, 167], [820, 220, 820, 167]], + [] + ); + }) + .catch(fail) + .then(done); + }); + + it('increase the range of search for points to draw the spikelines', function(done) { + gd = createGraphDiv(); + var _mock = makeMock('toaxis', 'closest'); + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 1.6, yval: 2.6}, 'xy'); + _assert( + [], + [] + ); + }) + .then(function() { + _hover({xval: 26, yval: 36}, 'x2y2'); + _assert( + [], + [] + ); + }) + .then(function() { + _set_spikedistance(200); + }) + .then(function() { + _hover({xval: 1.6, yval: 2.6}, 'xy'); + _assert( + [[557, 401, 557, 250], [557, 401, 557, 250], [80, 250, 557, 250], [80, 250, 557, 250]], + [[83, 250]] + ); + }) + .then(function() { + _hover({xval: 26, yval: 36}, 'x2y2'); + _assert( + [[820, 220, 820, 167], [820, 220, 820, 167]], + [] + ); + }) + .catch(fail) + .then(done); + }); + + it('correctly responds to setting the hoverdistance to 0 by increasing ' + + 'the range of search for points to hover to Infinity', function(done) { + gd = createGraphDiv(); + var _mock = makeMock('toaxis', 'closest'); + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 1.6, yval: 2.6}, 'xy'); + _assert( + [], + [] + ); + }) + .then(function() { + _hover({xval: 26, yval: 36}, 'x2y2'); + _assert( + [], + [] + ); + }) + .then(function() { + _set_spikedistance(0); + }) + .then(function() { + _hover({xval: 1.6, yval: 2.6}, 'xy'); + _assert( + [[557, 401, 557, 250], [557, 401, 557, 250], [80, 250, 557, 250], [80, 250, 557, 250]], + [[83, 250]] + ); + }) + .then(function() { + _hover({xval: 26, yval: 36}, 'x2y2'); + _assert( + [[820, 220, 820, 167], [820, 220, 820, 167]], + [] + ); + }) + .catch(fail) + .then(done); + }); }); }); From 751d527d86abc0689b87886811dd25e410a7aa9a Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Fri, 12 Jan 2018 15:06:19 +0300 Subject: [PATCH 09/12] add new tests and fix cursor --- src/components/fx/hover.js | 12 ++++-- test/jasmine/tests/hover_label_test.js | 15 +++++++- test/jasmine/tests/hover_spikeline_test.js | 45 ++++++++++++++-------- 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 6d32e86bbd0..e4455100b67 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -274,9 +274,11 @@ function _hover(gd, evt, subplot, noHoverEvent) { if(hasUserCalledHover) { if('xpx' in evt) xpx = evt.xpx; else xpx = xaArray[0]._length / 2; + if(!('offsetX' in evt)) evt.offsetX = xpx + xaArray[0]._offset; if('ypx' in evt) ypx = evt.ypx; else ypx = yaArray[0]._length / 2; + if(!('offsetY' in evt)) evt.offsetY = ypx + yaArray[0]._offset; } else { // fire the beforehover event and quit if it returns false @@ -419,7 +421,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { if(hoverData.length === 0) { pointData.distance = spikedistance; pointData.index = false; - var closestPoints = trace._module.hoverPoints(pointData, xval, yval, 'closest'); + var closestPoints = trace._module.hoverPoints(pointData, xval, yval, 'closest', fullLayout._hoverlayer); if(closestPoints) { var tmpPoint; var closestVPoints = closestPoints.filter(function(point) { @@ -1247,10 +1249,11 @@ function createSpikelines(closestPoints, opts) { if(ySnap === 'cursor') { hLinePointY = evt.offsetY; + hLinePointX = evt.offsetX; } else { hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2; + hLinePointX = xa._offset + (hLinePoint.x0 + hLinePoint.x1) / 2; } - hLinePointX = xa._offset + (hLinePoint.x0 + hLinePoint.x1) / 2; var dfltHLineColor = tinycolor.readability(hLinePoint.color, contrastColor) < 1.5 ? Color.contrast(contrastColor) : hLinePoint.color; var yMode = ya.spikemode, @@ -1322,12 +1325,13 @@ function createSpikelines(closestPoints, opts) { if(xSnap === 'cursor') { vLinePointX = evt.offsetX; + vLinePointY = evt.offsetY; } else { vLinePointX = xa._offset + (vLinePoint.x0 + vLinePoint.x1) / 2; + vLinePointY = ya._offset + (vLinePoint.y0 + vLinePoint.y1) / 2; } - vLinePointY = ya._offset + (vLinePoint.y0 + vLinePoint.y1) / 2; var dfltVLineColor = tinycolor.readability(vLinePoint.color, contrastColor) < 1.5 ? - Color.contrast(contrastColor) : vLinePoint.color; + Color.contrast(contrastColor) : vLinePoint.color; var xMode = xa.spikemode, xThickness = xa.spikethickness, xColor = xa.spikecolor || dfltVLineColor, diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 97d1e320435..dddf02f73a2 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1842,7 +1842,17 @@ describe('hover distance', function() { }); }); - it('responds to hoverdistance change', function() { + it('responds to hoverdistance change part 1', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 450, ypx: 155 }; + gd._fullLayout.hoverdistance = 10; + + Fx.hover('graph', evt, 'xy'); + + expect(gd._hoverdata).toEqual(undefined); + }); + + it('responds to hoverdistance change part 2', function() { var gd = document.getElementById('graph'); var evt = { xpx: 450, ypx: 155 }; gd._fullLayout.hoverdistance = 30; @@ -1857,7 +1867,8 @@ describe('hover distance', function() { expect(hoverTrace.y).toEqual(3); assertHoverLabelContent({ - nums: '(2, 3)', + nums: '3', + axis: '2', name: 'trace 0' }); }); diff --git a/test/jasmine/tests/hover_spikeline_test.js b/test/jasmine/tests/hover_spikeline_test.js index e9de301d614..4a373be02d7 100644 --- a/test/jasmine/tests/hover_spikeline_test.js +++ b/test/jasmine/tests/hover_spikeline_test.js @@ -135,43 +135,56 @@ describe('spikeline', function() { .then(done); }); - it('automatically switch between toaxis and across spikemodes on switching the hovermodes', function(done) { + it('draws lines and markers on enabled axes in the spikesnap "cursor" mode', function(done) { gd = createGraphDiv(); - var _mock = makeMock('toaxis', 'closest'); + var _mock = makeMock('toaxis', 'x'); - Plotly.plot(gd, _mock).then(function() { - _hover({xval: 2, yval: 3}, 'xy'); + _mock.layout.xaxis.spikesnap = 'cursor'; + _mock.layout.yaxis.spikesnap = 'cursor'; + _mock.layout.xaxis2.spikesnap = 'cursor'; + + Plotly.plot(gd, _mock) + .then(function() { + _set_spikedistance(200); + }) + .then(function() { + _hover({xpx: 120, ypx: 180}, 'xy'); _assert( - [[557, 401, 557, 250], [557, 401, 557, 250], [80, 250, 557, 250], [80, 250, 557, 250]], - [[83, 250]] + [[200, 401, 200, 280], [200, 401, 200, 280], [80, 280, 200, 280], [80, 280, 200, 280]], + [[83, 280]] ); }) .then(function() { - _hover({xval: 30, yval: 40}, 'x2y2'); + _hover({xpx: 31, ypx: 41}, 'x2y2'); _assert( - [[820, 220, 820, 167], [820, 220, 820, 167]], + [[682, 220, 682, 156], [682, 220, 682, 156]], [] ); }) - .then(function() { - _set_hovermode('x'); - }) - .then(function() { + .catch(fail) + .then(done); + }); + + it('doesn\'t switch between toaxis and across spikemodes on switching the hovermodes', function(done) { + gd = createGraphDiv(); + var _mock = makeMock('toaxis', 'closest'); + + Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); _assert( - [[557, 100, 557, 401], [557, 100, 557, 401], [80, 250, 1033, 250], [80, 250, 1033, 250]], + [[557, 401, 557, 250], [557, 401, 557, 250], [80, 250, 557, 250], [80, 250, 557, 250]], [[83, 250]] ); }) .then(function() { _hover({xval: 30, yval: 40}, 'x2y2'); _assert( - [[820, 115, 820, 220], [820, 115, 820, 220]], + [[820, 220, 820, 167], [820, 220, 820, 167]], [] ); }) .then(function() { - _set_hovermode('closest'); + _set_hovermode('x'); }) .then(function() { _hover({xval: 2, yval: 3}, 'xy'); @@ -191,7 +204,7 @@ describe('spikeline', function() { .then(done); }); - it('increase the range of search for points to draw the spikelines', function(done) { + it('increase the range of search for points to draw the spikelines on spikedistance change', function(done) { gd = createGraphDiv(); var _mock = makeMock('toaxis', 'closest'); From f56283254e779775db69682b9ae677f101b8e72b Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Fri, 12 Jan 2018 16:14:52 +0300 Subject: [PATCH 10/12] fix tests --- test/jasmine/tests/hover_spikeline_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/tests/hover_spikeline_test.js b/test/jasmine/tests/hover_spikeline_test.js index 4a373be02d7..618f4b4b61d 100644 --- a/test/jasmine/tests/hover_spikeline_test.js +++ b/test/jasmine/tests/hover_spikeline_test.js @@ -120,7 +120,7 @@ describe('spikeline', function() { Plotly.plot(gd, _mock).then(function() { _hover({xval: 2, yval: 3}, 'xy'); _assert( - [[557, 100, 557, 401], [557, 100, 557, 401], [80, 250, 1033, 250], [80, 250, 1033, 250]], + [[557, 100, 557, 401], [557, 100, 557, 401], [80, 250, 1036, 250], [80, 250, 1036, 250]], [[83, 250]] ); }) From 6c2f368b796a74172cbc6ed575514307cd4edafe Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Wed, 17 Jan 2018 17:00:21 +0300 Subject: [PATCH 11/12] fix issues after PR code review --- src/components/fx/hover.js | 74 ++++++++++++---------- src/components/fx/layout_attributes.js | 16 ++--- src/components/fx/layout_defaults.js | 8 ++- src/components/modebar/buttons.js | 16 ++--- src/plots/cartesian/axes.js | 8 +-- src/plots/cartesian/layout_defaults.js | 24 +++++-- src/traces/scatter/hover.js | 4 +- test/jasmine/tests/hover_label_test.js | 58 ++++++++++++++--- test/jasmine/tests/hover_spikeline_test.js | 50 +++++++++++++-- 9 files changed, 178 insertions(+), 80 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index e4455100b67..2419a58af2b 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -208,8 +208,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { return dragElement.unhoverRaw(gd, evt); } - var hoverdistance = fullLayout.hoverdistance === 0 ? Infinity : fullLayout.hoverdistance; - var spikedistance = fullLayout.spikedistance === 0 ? Infinity : fullLayout.spikedistance; + var hoverdistance = fullLayout.hoverdistance === -1 ? Infinity : fullLayout.hoverdistance; + var spikedistance = fullLayout.spikedistance === -1 ? Infinity : fullLayout.spikedistance; // hoverData: the set of candidate points we've found to highlight var hoverData = [], @@ -268,17 +268,18 @@ function _hover(gd, evt, subplot, noHoverEvent) { // [x|y]px: the pixels (from top left) of the mouse location // on the currently selected plot area + // add pointerX|Y property for drawing the spikes in spikesnap 'cursor' situation var hasUserCalledHover = !evt.target, xpx, ypx; if(hasUserCalledHover) { if('xpx' in evt) xpx = evt.xpx; else xpx = xaArray[0]._length / 2; - if(!('offsetX' in evt)) evt.offsetX = xpx + xaArray[0]._offset; + evt.pointerX = xpx + xaArray[0]._offset; if('ypx' in evt) ypx = evt.ypx; else ypx = yaArray[0]._length / 2; - if(!('offsetY' in evt)) evt.offsetY = ypx + yaArray[0]._offset; + evt.pointerY = ypx + yaArray[0]._offset; } else { // fire the beforehover event and quit if it returns false @@ -298,6 +299,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { if(xpx < 0 || xpx > dbb.width || ypx < 0 || ypx > dbb.height) { return dragElement.unhoverRaw(gd, evt); } + evt.pointerX = evt.offsetX; + evt.pointerY = evt.offsetY; } if('xval' in evt) xvalArray = helpers.flat(subplots, evt.xval); @@ -391,21 +394,23 @@ function _hover(gd, evt, subplot, noHoverEvent) { yval = yvalArray[subploti]; } - // Now find the points. - if(trace._module && trace._module.hoverPoints) { - var newPoints = trace._module.hoverPoints(pointData, xval, yval, mode, fullLayout._hoverlayer); - if(newPoints) { - var newPoint; - for(var newPointNum = 0; newPointNum < newPoints.length; newPointNum++) { - newPoint = newPoints[newPointNum]; - if(isNumeric(newPoint.x0) && isNumeric(newPoint.y0)) { - hoverData.push(cleanPoint(newPoint, hovermode)); + // Now if there is range to look in, find the points to hover. + if(hoverdistance !== 0) { + if(trace._module && trace._module.hoverPoints) { + var newPoints = trace._module.hoverPoints(pointData, xval, yval, mode, fullLayout._hoverlayer); + if(newPoints) { + var newPoint; + for(var newPointNum = 0; newPointNum < newPoints.length; newPointNum++) { + newPoint = newPoints[newPointNum]; + if(isNumeric(newPoint.x0) && isNumeric(newPoint.y0)) { + hoverData.push(cleanPoint(newPoint, hovermode)); + } } } } - } - else { - Lib.log('Unrecognized trace type in hover:', trace); + else { + Lib.log('Unrecognized trace type in hover:', trace); + } } // in closest mode, remove any existing (farther) points @@ -415,9 +420,9 @@ function _hover(gd, evt, subplot, noHoverEvent) { distance = hoverData[0].distance; } - // Now look for the points to draw the spikelines + // Now if there is range to look in, find the points to draw the spikelines // Do it only if there is no hoverData - if(fullLayout._has('cartesian')) { + if(fullLayout._has('cartesian') && (spikedistance !== 0)) { if(hoverData.length === 0) { pointData.distance = spikedistance; pointData.index = false; @@ -430,7 +435,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { if(closestVPoints.length) { var closestVPt = closestVPoints[0]; if(isNumeric(closestVPt.x0) && isNumeric(closestVPt.y0)) { - tmpPoint = clearClosestPoint(closestVPt); + tmpPoint = fillClosestPoint(closestVPt); if(!spikePoints.vLinePoint || (spikePoints.vLinePoint.distance > tmpPoint.distance)) { spikePoints.vLinePoint = tmpPoint; } @@ -443,7 +448,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { if(closestHPoints.length) { var closestHPt = closestHPoints[0]; if(isNumeric(closestHPt.x0) && isNumeric(closestHPt.y0)) { - tmpPoint = clearClosestPoint(closestHPt); + tmpPoint = fillClosestPoint(closestHPt); if(!spikePoints.hLinePoint || (spikePoints.hLinePoint.distance > tmpPoint.distance)) { spikePoints.hLinePoint = tmpPoint; } @@ -463,7 +468,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { xpx = xa.c2p(xval), ypx = ya.c2p(yval), dxy = function(point) { - var rad = Math.max(3, point.mrc || 0), + var rad = point.kink, dx = (point.x1 + point.x0) / 2 - xpx, dy = (point.y1 + point.y0) / 2 - ypx; return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad); @@ -486,7 +491,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { return resultPoint; } - function clearClosestPoint(point) { + function fillClosestPoint(point) { if(!point) return null; return { xa: point.xa, @@ -502,7 +507,6 @@ function _hover(gd, evt, subplot, noHoverEvent) { }; } - // if hoverData is empty check for the spikes to draw and quit if there are none var spikelineOpts = { fullLayout: fullLayout, container: fullLayout._hoverlayer, @@ -516,22 +520,24 @@ function _hover(gd, evt, subplot, noHoverEvent) { }; gd._spikepoints = newspikepoints; - if(fullLayout._has('cartesian')) { + // Now if it is not restricted by spikedistance option, set the points to draw the spikelines + if(fullLayout._has('cartesian') && (spikedistance !== 0)) { if(hoverData.length !== 0) { var tmpHPointData = hoverData.filter(function(point) { return point.ya.showspikes; }); var tmpHPoint = selectClosestPoint(tmpHPointData, spikedistance); - spikePoints.hLinePoint = clearClosestPoint(tmpHPoint); + spikePoints.hLinePoint = fillClosestPoint(tmpHPoint); var tmpVPointData = hoverData.filter(function(point) { return point.xa.showspikes; }); var tmpVPoint = selectClosestPoint(tmpVPointData, spikedistance); - spikePoints.vLinePoint = clearClosestPoint(tmpVPoint); + spikePoints.vLinePoint = fillClosestPoint(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(fullLayout._has('cartesian') && ((spikePoints.hLinePoint !== null) || (spikePoints.vLinePoint !== null))) { @@ -1228,8 +1234,8 @@ function createSpikelines(closestPoints, opts) { var xa, ya; - var showY = closestPoints.hLinePoint ? true : false; - var showX = closestPoints.vLinePoint ? true : false; + var showY = !!closestPoints.hLinePoint; + var showX = !!closestPoints.vLinePoint; // Remove old spikeline items container.selectAll('.spikeline').remove(); @@ -1248,11 +1254,11 @@ function createSpikelines(closestPoints, opts) { var ySnap = ya.spikesnap; if(ySnap === 'cursor') { - hLinePointY = evt.offsetY; - hLinePointX = evt.offsetX; + hLinePointX = evt.pointerX; + hLinePointY = evt.pointerY; } else { - hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2; hLinePointX = xa._offset + (hLinePoint.x0 + hLinePoint.x1) / 2; + hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2; } var dfltHLineColor = tinycolor.readability(hLinePoint.color, contrastColor) < 1.5 ? Color.contrast(contrastColor) : hLinePoint.color; @@ -1324,8 +1330,8 @@ function createSpikelines(closestPoints, opts) { var xSnap = xa.spikesnap; if(xSnap === 'cursor') { - vLinePointX = evt.offsetX; - vLinePointY = evt.offsetY; + vLinePointX = evt.pointerX; + vLinePointY = evt.pointerY; } else { vLinePointX = xa._offset + (vLinePoint.x0 + vLinePoint.x1) / 2; vLinePointY = ya._offset + (vLinePoint.y0 + vLinePoint.y1) / 2; @@ -1408,7 +1414,7 @@ function hoverChanged(gd, evt, oldhoverdata) { } function spikesChanged(gd, oldspikepoints) { - // don't emit any events if nothing changed + // don't relayout the plot because of new spikelines if spikelines points didn't change if(!oldspikepoints) return true; if(oldspikepoints.vLinePoint !== gd._spikepoints.vLinePoint || oldspikepoints.hLinePoint !== gd._spikepoints.hLinePoint diff --git a/src/components/fx/layout_attributes.js b/src/components/fx/layout_attributes.js index 0bea247ba23..1196cfec9f1 100644 --- a/src/components/fx/layout_attributes.js +++ b/src/components/fx/layout_attributes.js @@ -40,24 +40,24 @@ module.exports = { }, hoverdistance: { valType: 'integer', - min: 0, + min: -1, dflt: 20, - role: 'style', + role: 'info', editType: 'none', description: [ - 'Sets the default distance (in points) to look for data', - 'to add hover labels (zero means no cutoff)' + 'Sets the default distance (in pixels) to look for data', + 'to add hover labels (-1 means no cutoff, 0 means no looking for data)' ].join(' ') }, spikedistance: { valType: 'integer', - min: 0, + min: -1, dflt: 20, - role: 'style', + role: 'info', editType: 'none', description: [ - 'Sets the default distance (in points) to look for data to draw', - 'spikelines to (zero means no cutoff).' + 'Sets the default distance (in pixels) to look for data to draw', + 'spikelines to (-1 means no cutoff, 0 means no looking for data).' ].join(' ') }, hoverlabel: { diff --git a/src/components/fx/layout_defaults.js b/src/components/fx/layout_defaults.js index 0768292f257..b88b652d9e2 100644 --- a/src/components/fx/layout_defaults.js +++ b/src/components/fx/layout_defaults.js @@ -27,9 +27,11 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { } else hovermodeDflt = 'closest'; - coerce('hovermode', hovermodeDflt); - coerce('hoverdistance'); - coerce('spikedistance'); + var hoverMode = coerce('hovermode', hovermodeDflt); + if(hoverMode) { + coerce('hoverdistance'); + coerce('spikedistance'); + } // if only mapbox or geo subplots is present on graph, // reset 'zoom' dragmode to 'pan' until 'zoom' is implemented, diff --git a/src/components/modebar/buttons.js b/src/components/modebar/buttons.js index b8fd69ad6f6..1d3e2c643d8 100644 --- a/src/components/modebar/buttons.js +++ b/src/components/modebar/buttons.js @@ -181,7 +181,7 @@ function handleCartesian(gd, ev) { var fullLayout = gd._fullLayout; var aobj = {}; var axList = axisIds.list(gd, null, true); - var allEnabled = 'on'; + var allSpikesEnabled = 'on'; var ax, i; @@ -209,8 +209,8 @@ function handleCartesian(gd, ev) { } if(ax._showSpikeInitial !== undefined) { aobj[axName + '.showspikes'] = ax._showSpikeInitial; - if(allEnabled === 'on' && !ax._showSpikeInitial) { - allEnabled = 'off'; + if(allSpikesEnabled === 'on' && !ax._showSpikeInitial) { + allSpikesEnabled = 'off'; } } } @@ -230,7 +230,7 @@ function handleCartesian(gd, ev) { } } } - fullLayout._cartesianSpikesEnabled = allEnabled; + fullLayout._cartesianSpikesEnabled = allSpikesEnabled; } else { // if ALL traces have orientation 'h', 'hovermode': 'x' otherwise: 'y' @@ -240,11 +240,11 @@ function handleCartesian(gd, ev) { } else if(astr === 'hovermode' && val === 'closest') { for(i = 0; i < axList.length; i++) { ax = axList[i]; - if(allEnabled === 'on' && !ax.showspikes) { - allEnabled = 'off'; + if(allSpikesEnabled === 'on' && !ax.showspikes) { + allSpikesEnabled = 'off'; } } - fullLayout._cartesianSpikesEnabled = allEnabled; + fullLayout._cartesianSpikesEnabled = allSpikesEnabled; } aobj[astr] = val; @@ -566,7 +566,7 @@ function setSpikelineVisibility(gd) { for(var i = 0; i < axList.length; i++) { ax = axList[i]; axName = ax._name; - aobj[axName + '.showspikes'] = fullLayout._cartesianSpikesEnabled === 'on' ? true : false; + aobj[axName + '.showspikes'] = fullLayout._cartesianSpikesEnabled === 'on' ? true : ax._showSpikeInitial; } return aobj; diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 90a41451b08..fe7c51ef793 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -398,7 +398,7 @@ axes.saveRangeInitial = function(gd, overwrite) { axes.saveShowSpikeInitial = function(gd, overwrite) { var axList = axes.list(gd, '', true), hasOneAxisChanged = false, - allEnabled = 'on'; + allSpikesEnabled = 'on'; for(var i = 0; i < axList.length; i++) { var ax = axList[i]; @@ -415,11 +415,11 @@ axes.saveShowSpikeInitial = function(gd, overwrite) { hasOneAxisChanged = true; } - if(allEnabled === 'on' && !ax.showspikes) { - allEnabled = 'off'; + if(allSpikesEnabled === 'on' && !ax.showspikes) { + allSpikesEnabled = 'off'; } } - gd._fullLayout._cartesianSpikesEnabled = allEnabled; + gd._fullLayout._cartesianSpikesEnabled = allSpikesEnabled; return hasOneAxisChanged; }; diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js index 275a1c19299..149bb203d1e 100644 --- a/src/plots/cartesian/layout_defaults.js +++ b/src/plots/cartesian/layout_defaults.js @@ -89,6 +89,10 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { return Lib.coerce(axLayoutIn, axLayoutOut, layoutAttributes, attr, dflt); } + function coerce2(attr, dflt) { + return Lib.coerce2(axLayoutIn, axLayoutOut, layoutAttributes, attr, dflt); + } + function getCounterAxes(axLetter) { return (axLetter === 'x') ? yIds : xIds; } @@ -139,13 +143,19 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { handleAxisDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions, layoutOut); - var showSpikes = coerce('showspikes'); - if(showSpikes) { - coerce('spikecolor'); - coerce('spikethickness'); - coerce('spikedash'); - coerce('spikemode'); - coerce('spikesnap'); + var spikecolor = coerce2('spikecolor'), + spikethickness = coerce2('spikethickness'), + spikedash = coerce2('spikedash'), + spikemode = coerce2('spikemode'), + spikesnap = coerce2('spikesnap'), + showSpikes = coerce('showspikes', !!spikecolor || !!spikethickness || !!spikedash || !!spikemode || !!spikesnap); + + if(!showSpikes) { + delete axLayoutOut.spikecolor; + delete axLayoutOut.spikethickness; + delete axLayoutOut.spikedash; + delete axLayoutOut.spikemode; + delete axLayoutOut.spikesnap; } var positioningOptions = { diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js index 66d5973ad18..b7348fa383c 100644 --- a/src/traces/scatter/hover.js +++ b/src/traces/scatter/hover.js @@ -79,7 +79,9 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { y0: yc - rad, y1: yc + rad, - yLabelVal: di.y + yLabelVal: di.y, + + kink: Math.max(minRad, di.mrc || 0) }); fillHoverText(di, trace, pointData); diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index dddf02f73a2..f50362f9a90 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1768,7 +1768,7 @@ describe('hover distance', function() { it('responds to hoverdistance change', function() { var gd = document.getElementById('graph'); var evt = { xpx: 475, ypx: 180 }; - gd._fullLayout.hoverdistance = 30; + Plotly.relayout(gd, 'hoverdistance', 30); Fx.hover('graph', evt, 'xy'); @@ -1785,11 +1785,11 @@ describe('hover distance', function() { }); }); - it('correctly responds to setting the hoverdistance to 0 by increasing ' + + it('correctly responds to setting the hoverdistance to -1 by increasing ' + 'the range of search for points to hover to Infinity', function() { var gd = document.getElementById('graph'); var evt = { xpx: 475, ypx: 180 }; - gd._fullLayout.hoverdistance = 0; + Plotly.relayout(gd, 'hoverdistance', -1); Fx.hover('graph', evt, 'xy'); @@ -1842,20 +1842,20 @@ describe('hover distance', function() { }); }); - it('responds to hoverdistance change part 1', function() { + it('responds to hoverdistance change from 10 to 30 (part 1)', function() { var gd = document.getElementById('graph'); var evt = { xpx: 450, ypx: 155 }; - gd._fullLayout.hoverdistance = 10; + Plotly.relayout(gd, 'hoverdistance', 10); Fx.hover('graph', evt, 'xy'); expect(gd._hoverdata).toEqual(undefined); }); - it('responds to hoverdistance change part 2', function() { + it('responds to hoverdistance change from 10 to 30 (part 2)', function() { var gd = document.getElementById('graph'); var evt = { xpx: 450, ypx: 155 }; - gd._fullLayout.hoverdistance = 30; + Plotly.relayout(gd, 'hoverdistance', 30); Fx.hover('graph', evt, 'xy'); @@ -1873,11 +1873,49 @@ describe('hover distance', function() { }); }); - it('correctly responds to setting the hoverdistance to 0 by increasing ' + - 'the range of search for points to hover to Infinity', function() { + it('responds to hoverdistance change from default to 0 (part 1)', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 475, ypx: 155 }; + Fx.hover('graph', evt, 'xy'); + + var hoverTrace = gd._hoverdata[0]; + + expect(hoverTrace.curveNumber).toEqual(0); + expect(hoverTrace.pointNumber).toEqual(1); + expect(hoverTrace.x).toEqual(2); + expect(hoverTrace.y).toEqual(3); + + assertHoverLabelContent({ + nums: '3', + axis: '2', + name: 'trace 0' + }); + }); + + it('responds to hoverdistance change from default to 0 (part 2)', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 475, ypx: 155 }; + Plotly.relayout(gd, 'hoverdistance', 0); + + Fx.hover('graph', evt, 'xy'); + + expect(gd._hoverdata).toEqual(undefined); + }); + + it('responds to setting the hoverdistance to -1 by increasing ' + + 'the range of search for points to hover to Infinity (part 1)', function() { + var gd = document.getElementById('graph'); + var evt = { xpx: 450, ypx: 155 }; + Fx.hover('graph', evt, 'xy'); + + expect(gd._hoverdata).toEqual(undefined); + }); + + it('responds to setting the hoverdistance to -1 by increasing ' + + 'the range of search for points to hover to Infinity (part 2)', function() { var gd = document.getElementById('graph'); var evt = { xpx: 450, ypx: 155 }; - gd._fullLayout.hoverdistance = 0; + Plotly.relayout(gd, 'hoverdistance', -1); Fx.hover('graph', evt, 'xy'); diff --git a/test/jasmine/tests/hover_spikeline_test.js b/test/jasmine/tests/hover_spikeline_test.js index 618f4b4b61d..c9655f8269a 100644 --- a/test/jasmine/tests/hover_spikeline_test.js +++ b/test/jasmine/tests/hover_spikeline_test.js @@ -34,11 +34,11 @@ describe('spikeline', function() { } function _set_hovermode(hovermode) { - gd._fullLayout.hovermode = hovermode; + return Plotly.relayout(gd, 'hovermode', hovermode); } function _set_spikedistance(spikedistance) { - gd._fullLayout.spikedistance = spikedistance; + return Plotly.relayout(gd, 'spikedistance', spikedistance); } function _assert(lineExpect, circleExpect) { @@ -243,8 +243,8 @@ describe('spikeline', function() { .then(done); }); - it('correctly responds to setting the hoverdistance to 0 by increasing ' + - 'the range of search for points to hover to Infinity', function(done) { + it('correctly responds to setting the spikedistance to -1 by increasing ' + + 'the range of search for points to draw the spikelines to Infinity', function(done) { gd = createGraphDiv(); var _mock = makeMock('toaxis', 'closest'); @@ -263,7 +263,7 @@ describe('spikeline', function() { ); }) .then(function() { - _set_spikedistance(0); + _set_spikedistance(-1); }) .then(function() { _hover({xval: 1.6, yval: 2.6}, 'xy'); @@ -282,5 +282,45 @@ describe('spikeline', function() { .catch(fail) .then(done); }); + + it('correctly responds to setting the spikedistance to 0 by disabling ' + + 'the search for points to draw the spikelines', function(done) { + gd = createGraphDiv(); + var _mock = makeMock('toaxis', 'closest'); + + Plotly.plot(gd, _mock).then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [[557, 401, 557, 250], [557, 401, 557, 250], [80, 250, 557, 250], [80, 250, 557, 250]], + [[83, 250]] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [[820, 220, 820, 167], [820, 220, 820, 167]], + [] + ); + }) + .then(function() { + _set_spikedistance(0); + }) + .then(function() { + _hover({xval: 2, yval: 3}, 'xy'); + _assert( + [], + [] + ); + }) + .then(function() { + _hover({xval: 30, yval: 40}, 'x2y2'); + _assert( + [], + [] + ); + }) + .catch(fail) + .then(done); + }); }); }); From f56413a20721f850ec6ebea4bcb0ccd4b5f82240 Mon Sep 17 00:00:00 2001 From: Andrei Palchys Date: Wed, 17 Jan 2018 19:09:08 +0300 Subject: [PATCH 12/12] restore and update spikeline tests --- test/jasmine/tests/modebar_test.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/jasmine/tests/modebar_test.js b/test/jasmine/tests/modebar_test.js index ccda5983a18..e798ff369c4 100644 --- a/test/jasmine/tests/modebar_test.js +++ b/test/jasmine/tests/modebar_test.js @@ -861,6 +861,14 @@ describe('ModeBar', function() { }); describe('button toggleSpikelines', function() { + it('should not change layout hovermode', function() { + expect(gd._fullLayout.hovermode).toBe('x'); + assertActive(hovermodeButtons, buttonCompare); + + buttonToggle.click(); + expect(gd._fullLayout.hovermode).toBe('x'); + assertActive(hovermodeButtons, buttonCompare); + }); it('should makes spikelines visible', function() { buttonToggle.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); @@ -868,6 +876,26 @@ describe('ModeBar', function() { buttonToggle.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); }); + it('should not become disabled when hovermode is switched off closest', function() { + buttonToggle.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); + + buttonCompare.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); + }); + it('should keep the state on changing the hovermode', function() { + buttonToggle.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); + + buttonCompare.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); + + buttonToggle.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); + + buttonClosest.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); + }); }); });