Skip to content

Commit 6c2f368

Browse files
apalchysapalchys
committed
fix issues after PR code review
1 parent f562832 commit 6c2f368

File tree

9 files changed

+178
-80
lines changed

9 files changed

+178
-80
lines changed

‎src/components/fx/hover.js

+40-34
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,8 @@ function _hover(gd, evt, subplot, noHoverEvent) {
208208
return dragElement.unhoverRaw(gd, evt);
209209
}
210210

211-
var hoverdistance = fullLayout.hoverdistance === 0 ? Infinity : fullLayout.hoverdistance;
212-
var spikedistance = fullLayout.spikedistance === 0 ? Infinity : fullLayout.spikedistance;
211+
var hoverdistance = fullLayout.hoverdistance === -1 ? Infinity : fullLayout.hoverdistance;
212+
var spikedistance = fullLayout.spikedistance === -1 ? Infinity : fullLayout.spikedistance;
213213

214214
// hoverData: the set of candidate points we've found to highlight
215215
var hoverData = [],
@@ -268,17 +268,18 @@ function _hover(gd, evt, subplot, noHoverEvent) {
268268

269269
// [x|y]px: the pixels (from top left) of the mouse location
270270
// on the currently selected plot area
271+
// add pointerX|Y property for drawing the spikes in spikesnap 'cursor' situation
271272
var hasUserCalledHover = !evt.target,
272273
xpx, ypx;
273274

274275
if(hasUserCalledHover) {
275276
if('xpx' in evt) xpx = evt.xpx;
276277
else xpx = xaArray[0]._length / 2;
277-
if(!('offsetX' in evt)) evt.offsetX = xpx + xaArray[0]._offset;
278+
evt.pointerX = xpx + xaArray[0]._offset;
278279

279280
if('ypx' in evt) ypx = evt.ypx;
280281
else ypx = yaArray[0]._length / 2;
281-
if(!('offsetY' in evt)) evt.offsetY = ypx + yaArray[0]._offset;
282+
evt.pointerY = ypx + yaArray[0]._offset;
282283
}
283284
else {
284285
// fire the beforehover event and quit if it returns false
@@ -298,6 +299,8 @@ function _hover(gd, evt, subplot, noHoverEvent) {
298299
if(xpx < 0 || xpx > dbb.width || ypx < 0 || ypx > dbb.height) {
299300
return dragElement.unhoverRaw(gd, evt);
300301
}
302+
evt.pointerX = evt.offsetX;
303+
evt.pointerY = evt.offsetY;
301304
}
302305

303306
if('xval' in evt) xvalArray = helpers.flat(subplots, evt.xval);
@@ -391,21 +394,23 @@ function _hover(gd, evt, subplot, noHoverEvent) {
391394
yval = yvalArray[subploti];
392395
}
393396

394-
// Now find the points.
395-
if(trace._module && trace._module.hoverPoints) {
396-
var newPoints = trace._module.hoverPoints(pointData, xval, yval, mode, fullLayout._hoverlayer);
397-
if(newPoints) {
398-
var newPoint;
399-
for(var newPointNum = 0; newPointNum < newPoints.length; newPointNum++) {
400-
newPoint = newPoints[newPointNum];
401-
if(isNumeric(newPoint.x0) && isNumeric(newPoint.y0)) {
402-
hoverData.push(cleanPoint(newPoint, hovermode));
397+
// Now if there is range to look in, find the points to hover.
398+
if(hoverdistance !== 0) {
399+
if(trace._module && trace._module.hoverPoints) {
400+
var newPoints = trace._module.hoverPoints(pointData, xval, yval, mode, fullLayout._hoverlayer);
401+
if(newPoints) {
402+
var newPoint;
403+
for(var newPointNum = 0; newPointNum < newPoints.length; newPointNum++) {
404+
newPoint = newPoints[newPointNum];
405+
if(isNumeric(newPoint.x0) && isNumeric(newPoint.y0)) {
406+
hoverData.push(cleanPoint(newPoint, hovermode));
407+
}
403408
}
404409
}
405410
}
406-
}
407-
else {
408-
Lib.log('Unrecognized trace type in hover:', trace);
411+
else {
412+
Lib.log('Unrecognized trace type in hover:', trace);
413+
}
409414
}
410415

411416
// in closest mode, remove any existing (farther) points
@@ -415,9 +420,9 @@ function _hover(gd, evt, subplot, noHoverEvent) {
415420
distance = hoverData[0].distance;
416421
}
417422

418-
// Now look for the points to draw the spikelines
423+
// Now if there is range to look in, find the points to draw the spikelines
419424
// Do it only if there is no hoverData
420-
if(fullLayout._has('cartesian')) {
425+
if(fullLayout._has('cartesian') && (spikedistance !== 0)) {
421426
if(hoverData.length === 0) {
422427
pointData.distance = spikedistance;
423428
pointData.index = false;
@@ -430,7 +435,7 @@ function _hover(gd, evt, subplot, noHoverEvent) {
430435
if(closestVPoints.length) {
431436
var closestVPt = closestVPoints[0];
432437
if(isNumeric(closestVPt.x0) && isNumeric(closestVPt.y0)) {
433-
tmpPoint = clearClosestPoint(closestVPt);
438+
tmpPoint = fillClosestPoint(closestVPt);
434439
if(!spikePoints.vLinePoint || (spikePoints.vLinePoint.distance > tmpPoint.distance)) {
435440
spikePoints.vLinePoint = tmpPoint;
436441
}
@@ -443,7 +448,7 @@ function _hover(gd, evt, subplot, noHoverEvent) {
443448
if(closestHPoints.length) {
444449
var closestHPt = closestHPoints[0];
445450
if(isNumeric(closestHPt.x0) && isNumeric(closestHPt.y0)) {
446-
tmpPoint = clearClosestPoint(closestHPt);
451+
tmpPoint = fillClosestPoint(closestHPt);
447452
if(!spikePoints.hLinePoint || (spikePoints.hLinePoint.distance > tmpPoint.distance)) {
448453
spikePoints.hLinePoint = tmpPoint;
449454
}
@@ -463,7 +468,7 @@ function _hover(gd, evt, subplot, noHoverEvent) {
463468
xpx = xa.c2p(xval),
464469
ypx = ya.c2p(yval),
465470
dxy = function(point) {
466-
var rad = Math.max(3, point.mrc || 0),
471+
var rad = point.kink,
467472
dx = (point.x1 + point.x0) / 2 - xpx,
468473
dy = (point.y1 + point.y0) / 2 - ypx;
469474
return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - 3 / rad);
@@ -486,7 +491,7 @@ function _hover(gd, evt, subplot, noHoverEvent) {
486491
return resultPoint;
487492
}
488493

489-
function clearClosestPoint(point) {
494+
function fillClosestPoint(point) {
490495
if(!point) return null;
491496
return {
492497
xa: point.xa,
@@ -502,7 +507,6 @@ function _hover(gd, evt, subplot, noHoverEvent) {
502507
};
503508
}
504509

505-
// if hoverData is empty check for the spikes to draw and quit if there are none
506510
var spikelineOpts = {
507511
fullLayout: fullLayout,
508512
container: fullLayout._hoverlayer,
@@ -516,22 +520,24 @@ function _hover(gd, evt, subplot, noHoverEvent) {
516520
};
517521
gd._spikepoints = newspikepoints;
518522

519-
if(fullLayout._has('cartesian')) {
523+
// Now if it is not restricted by spikedistance option, set the points to draw the spikelines
524+
if(fullLayout._has('cartesian') && (spikedistance !== 0)) {
520525
if(hoverData.length !== 0) {
521526
var tmpHPointData = hoverData.filter(function(point) {
522527
return point.ya.showspikes;
523528
});
524529
var tmpHPoint = selectClosestPoint(tmpHPointData, spikedistance);
525-
spikePoints.hLinePoint = clearClosestPoint(tmpHPoint);
530+
spikePoints.hLinePoint = fillClosestPoint(tmpHPoint);
526531

527532
var tmpVPointData = hoverData.filter(function(point) {
528533
return point.xa.showspikes;
529534
});
530535
var tmpVPoint = selectClosestPoint(tmpVPointData, spikedistance);
531-
spikePoints.vLinePoint = clearClosestPoint(tmpVPoint);
536+
spikePoints.vLinePoint = fillClosestPoint(tmpVPoint);
532537
}
533538
}
534539

540+
// if hoverData is empty check for the spikes to draw and quit if there are none
535541
if(hoverData.length === 0) {
536542
var result = dragElement.unhoverRaw(gd, evt);
537543
if(fullLayout._has('cartesian') && ((spikePoints.hLinePoint !== null) || (spikePoints.vLinePoint !== null))) {
@@ -1228,8 +1234,8 @@ function createSpikelines(closestPoints, opts) {
12281234
var xa,
12291235
ya;
12301236

1231-
var showY = closestPoints.hLinePoint ? true : false;
1232-
var showX = closestPoints.vLinePoint ? true : false;
1237+
var showY = !!closestPoints.hLinePoint;
1238+
var showX = !!closestPoints.vLinePoint;
12331239

12341240
// Remove old spikeline items
12351241
container.selectAll('.spikeline').remove();
@@ -1248,11 +1254,11 @@ function createSpikelines(closestPoints, opts) {
12481254
var ySnap = ya.spikesnap;
12491255

12501256
if(ySnap === 'cursor') {
1251-
hLinePointY = evt.offsetY;
1252-
hLinePointX = evt.offsetX;
1257+
hLinePointX = evt.pointerX;
1258+
hLinePointY = evt.pointerY;
12531259
} else {
1254-
hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2;
12551260
hLinePointX = xa._offset + (hLinePoint.x0 + hLinePoint.x1) / 2;
1261+
hLinePointY = ya._offset + (hLinePoint.y0 + hLinePoint.y1) / 2;
12561262
}
12571263
var dfltHLineColor = tinycolor.readability(hLinePoint.color, contrastColor) < 1.5 ?
12581264
Color.contrast(contrastColor) : hLinePoint.color;
@@ -1324,8 +1330,8 @@ function createSpikelines(closestPoints, opts) {
13241330
var xSnap = xa.spikesnap;
13251331

13261332
if(xSnap === 'cursor') {
1327-
vLinePointX = evt.offsetX;
1328-
vLinePointY = evt.offsetY;
1333+
vLinePointX = evt.pointerX;
1334+
vLinePointY = evt.pointerY;
13291335
} else {
13301336
vLinePointX = xa._offset + (vLinePoint.x0 + vLinePoint.x1) / 2;
13311337
vLinePointY = ya._offset + (vLinePoint.y0 + vLinePoint.y1) / 2;
@@ -1408,7 +1414,7 @@ function hoverChanged(gd, evt, oldhoverdata) {
14081414
}
14091415

14101416
function spikesChanged(gd, oldspikepoints) {
1411-
// don't emit any events if nothing changed
1417+
// don't relayout the plot because of new spikelines if spikelines points didn't change
14121418
if(!oldspikepoints) return true;
14131419
if(oldspikepoints.vLinePoint !== gd._spikepoints.vLinePoint ||
14141420
oldspikepoints.hLinePoint !== gd._spikepoints.hLinePoint

‎src/components/fx/layout_attributes.js

+8-8
Original file line numberDiff line numberDiff line change
@@ -40,24 +40,24 @@ module.exports = {
4040
},
4141
hoverdistance: {
4242
valType: 'integer',
43-
min: 0,
43+
min: -1,
4444
dflt: 20,
45-
role: 'style',
45+
role: 'info',
4646
editType: 'none',
4747
description: [
48-
'Sets the default distance (in points) to look for data',
49-
'to add hover labels (zero means no cutoff)'
48+
'Sets the default distance (in pixels) to look for data',
49+
'to add hover labels (-1 means no cutoff, 0 means no looking for data)'
5050
].join(' ')
5151
},
5252
spikedistance: {
5353
valType: 'integer',
54-
min: 0,
54+
min: -1,
5555
dflt: 20,
56-
role: 'style',
56+
role: 'info',
5757
editType: 'none',
5858
description: [
59-
'Sets the default distance (in points) to look for data to draw',
60-
'spikelines to (zero means no cutoff).'
59+
'Sets the default distance (in pixels) to look for data to draw',
60+
'spikelines to (-1 means no cutoff, 0 means no looking for data).'
6161
].join(' ')
6262
},
6363
hoverlabel: {

‎src/components/fx/layout_defaults.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
2727
}
2828
else hovermodeDflt = 'closest';
2929

30-
coerce('hovermode', hovermodeDflt);
31-
coerce('hoverdistance');
32-
coerce('spikedistance');
30+
var hoverMode = coerce('hovermode', hovermodeDflt);
31+
if(hoverMode) {
32+
coerce('hoverdistance');
33+
coerce('spikedistance');
34+
}
3335

3436
// if only mapbox or geo subplots is present on graph,
3537
// reset 'zoom' dragmode to 'pan' until 'zoom' is implemented,

‎src/components/modebar/buttons.js

+8-8
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ function handleCartesian(gd, ev) {
181181
var fullLayout = gd._fullLayout;
182182
var aobj = {};
183183
var axList = axisIds.list(gd, null, true);
184-
var allEnabled = 'on';
184+
var allSpikesEnabled = 'on';
185185

186186
var ax, i;
187187

@@ -209,8 +209,8 @@ function handleCartesian(gd, ev) {
209209
}
210210
if(ax._showSpikeInitial !== undefined) {
211211
aobj[axName + '.showspikes'] = ax._showSpikeInitial;
212-
if(allEnabled === 'on' && !ax._showSpikeInitial) {
213-
allEnabled = 'off';
212+
if(allSpikesEnabled === 'on' && !ax._showSpikeInitial) {
213+
allSpikesEnabled = 'off';
214214
}
215215
}
216216
}
@@ -230,7 +230,7 @@ function handleCartesian(gd, ev) {
230230
}
231231
}
232232
}
233-
fullLayout._cartesianSpikesEnabled = allEnabled;
233+
fullLayout._cartesianSpikesEnabled = allSpikesEnabled;
234234
}
235235
else {
236236
// if ALL traces have orientation 'h', 'hovermode': 'x' otherwise: 'y'
@@ -240,11 +240,11 @@ function handleCartesian(gd, ev) {
240240
} else if(astr === 'hovermode' && val === 'closest') {
241241
for(i = 0; i < axList.length; i++) {
242242
ax = axList[i];
243-
if(allEnabled === 'on' && !ax.showspikes) {
244-
allEnabled = 'off';
243+
if(allSpikesEnabled === 'on' && !ax.showspikes) {
244+
allSpikesEnabled = 'off';
245245
}
246246
}
247-
fullLayout._cartesianSpikesEnabled = allEnabled;
247+
fullLayout._cartesianSpikesEnabled = allSpikesEnabled;
248248
}
249249

250250
aobj[astr] = val;
@@ -566,7 +566,7 @@ function setSpikelineVisibility(gd) {
566566
for(var i = 0; i < axList.length; i++) {
567567
ax = axList[i];
568568
axName = ax._name;
569-
aobj[axName + '.showspikes'] = fullLayout._cartesianSpikesEnabled === 'on' ? true : false;
569+
aobj[axName + '.showspikes'] = fullLayout._cartesianSpikesEnabled === 'on' ? true : ax._showSpikeInitial;
570570
}
571571

572572
return aobj;

‎src/plots/cartesian/axes.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ axes.saveRangeInitial = function(gd, overwrite) {
398398
axes.saveShowSpikeInitial = function(gd, overwrite) {
399399
var axList = axes.list(gd, '', true),
400400
hasOneAxisChanged = false,
401-
allEnabled = 'on';
401+
allSpikesEnabled = 'on';
402402

403403
for(var i = 0; i < axList.length; i++) {
404404
var ax = axList[i];
@@ -415,11 +415,11 @@ axes.saveShowSpikeInitial = function(gd, overwrite) {
415415
hasOneAxisChanged = true;
416416
}
417417

418-
if(allEnabled === 'on' && !ax.showspikes) {
419-
allEnabled = 'off';
418+
if(allSpikesEnabled === 'on' && !ax.showspikes) {
419+
allSpikesEnabled = 'off';
420420
}
421421
}
422-
gd._fullLayout._cartesianSpikesEnabled = allEnabled;
422+
gd._fullLayout._cartesianSpikesEnabled = allSpikesEnabled;
423423
return hasOneAxisChanged;
424424
};
425425

‎src/plots/cartesian/layout_defaults.js

+17-7
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
8989
return Lib.coerce(axLayoutIn, axLayoutOut, layoutAttributes, attr, dflt);
9090
}
9191

92+
function coerce2(attr, dflt) {
93+
return Lib.coerce2(axLayoutIn, axLayoutOut, layoutAttributes, attr, dflt);
94+
}
95+
9296
function getCounterAxes(axLetter) {
9397
return (axLetter === 'x') ? yIds : xIds;
9498
}
@@ -139,13 +143,19 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
139143

140144
handleAxisDefaults(axLayoutIn, axLayoutOut, coerce, defaultOptions, layoutOut);
141145

142-
var showSpikes = coerce('showspikes');
143-
if(showSpikes) {
144-
coerce('spikecolor');
145-
coerce('spikethickness');
146-
coerce('spikedash');
147-
coerce('spikemode');
148-
coerce('spikesnap');
146+
var spikecolor = coerce2('spikecolor'),
147+
spikethickness = coerce2('spikethickness'),
148+
spikedash = coerce2('spikedash'),
149+
spikemode = coerce2('spikemode'),
150+
spikesnap = coerce2('spikesnap'),
151+
showSpikes = coerce('showspikes', !!spikecolor || !!spikethickness || !!spikedash || !!spikemode || !!spikesnap);
152+
153+
if(!showSpikes) {
154+
delete axLayoutOut.spikecolor;
155+
delete axLayoutOut.spikethickness;
156+
delete axLayoutOut.spikedash;
157+
delete axLayoutOut.spikemode;
158+
delete axLayoutOut.spikesnap;
149159
}
150160

151161
var positioningOptions = {

‎src/traces/scatter/hover.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
7979

8080
y0: yc - rad,
8181
y1: yc + rad,
82-
yLabelVal: di.y
82+
yLabelVal: di.y,
83+
84+
kink: Math.max(minRad, di.mrc || 0)
8385
});
8486

8587
fillHoverText(di, trace, pointData);

0 commit comments

Comments
 (0)