From 0a4feb388a498ef2562f3a72f165b7c303e7fb14 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 17 Oct 2019 12:20:24 -0400 Subject: [PATCH 1/4] implement hovergaps false option for heatmap and contour traces --- src/traces/contour/attributes.js | 2 +- src/traces/contour/defaults.js | 1 + src/traces/heatmap/attributes.js | 11 +++++ src/traces/heatmap/defaults.js | 1 + src/traces/heatmap/hover.js | 40 ++++++++++--------- test/jasmine/tests/contour_test.js | 64 +++++++++++++++++++++++++++++- test/jasmine/tests/heatmap_test.js | 25 ++++++++++++ 7 files changed, 123 insertions(+), 21 deletions(-) diff --git a/src/traces/contour/attributes.js b/src/traces/contour/attributes.js index ea200b51b5a..0914aba111f 100644 --- a/src/traces/contour/attributes.js +++ b/src/traces/contour/attributes.js @@ -38,7 +38,7 @@ module.exports = extendFlat({ ytype: heatmapAttrs.ytype, zhoverformat: heatmapAttrs.zhoverformat, hovertemplate: heatmapAttrs.hovertemplate, - + hovergaps: heatmapAttrs.hovergaps, connectgaps: extendFlat({}, heatmapAttrs.connectgaps, { description: [ 'Determines whether or not gaps', diff --git a/src/traces/contour/defaults.js b/src/traces/contour/defaults.js index 62c904c99a1..31066584145 100644 --- a/src/traces/contour/defaults.js +++ b/src/traces/contour/defaults.js @@ -35,6 +35,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout coerce('text'); coerce('hovertext'); coerce('hovertemplate'); + coerce('hovergaps'); var isConstraint = (coerce('contours.type') === 'constraint'); coerce('connectgaps', Lib.isArray1D(traceOut.z)); diff --git a/src/traces/heatmap/attributes.js b/src/traces/heatmap/attributes.js index 7aa7e9da0a8..177af98354a 100644 --- a/src/traces/heatmap/attributes.js +++ b/src/traces/heatmap/attributes.js @@ -79,6 +79,17 @@ module.exports = extendFlat({ 'Picks a smoothing algorithm use to smooth `z` data.' ].join(' ') }, + hovergaps: { + valType: 'boolean', + dflt: true, + role: 'style', + editType: 'none', + description: [ + 'Determines whether or not gaps', + '(i.e. {nan} or missing values)', + 'in the `z` data are hovered on.' + ].join(' ') + }, connectgaps: { valType: 'boolean', role: 'info', diff --git a/src/traces/heatmap/defaults.js b/src/traces/heatmap/defaults.js index a245af88d95..f499ad8391d 100644 --- a/src/traces/heatmap/defaults.js +++ b/src/traces/heatmap/defaults.js @@ -34,6 +34,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout handleStyleDefaults(traceIn, traceOut, coerce, layout); + coerce('hovergaps'); coerce('connectgaps', Lib.isArray1D(traceOut.z) && (traceOut.zsmooth !== false)); colorscaleDefaults(traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'z'}); diff --git a/src/traces/heatmap/hover.js b/src/traces/heatmap/hover.js index 7c0deec8e0a..24409dfbc5e 100644 --- a/src/traces/heatmap/hover.js +++ b/src/traces/heatmap/hover.js @@ -88,9 +88,6 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, hoverLay } } - var zVal = z[ny][nx]; - if(zmask && !zmask[ny][nx]) zVal = undefined; - var text; if(Array.isArray(cd0.hovertext) && Array.isArray(cd0.hovertext[ny])) { text = cd0.hovertext[ny][nx]; @@ -98,18 +95,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, hoverLay text = cd0.text[ny][nx]; } - // dummy axis for formatting the z value - var cOpts = extractOpts(trace); - var dummyAx = { - type: 'linear', - range: [cOpts.min, cOpts.max], - hoverformat: zhoverformat, - _separators: xa._separators, - _numFormat: xa._numFormat - }; - var zLabel = Axes.tickText(dummyAx, zVal, 'hover').text; - - return [Lib.extendFlat(pointData, { + var obj = { index: [ny, nx], // never let a 2D override 1D type as closest point distance: pointData.maxHoverDistance, @@ -120,8 +106,26 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, hoverLay y1: y1, xLabelVal: xl, yLabelVal: yl, - zLabelVal: zVal, - zLabel: zLabel, text: text - })]; + }; + + var zVal = z[ny][nx]; + if(zmask && !zmask[ny][nx]) zVal = undefined; + + if(zVal !== undefined || trace.hovergaps) { + // dummy axis for formatting the z value + var cOpts = extractOpts(trace); + var dummyAx = { + type: 'linear', + range: [cOpts.min, cOpts.max], + hoverformat: zhoverformat, + _separators: xa._separators, + _numFormat: xa._numFormat + }; + + obj.zLabelVal = zVal; + obj.zLabel = Axes.tickText(dummyAx, zVal, 'hover').text; + } + + return [Lib.extendFlat(pointData, obj)]; }; diff --git a/test/jasmine/tests/contour_test.js b/test/jasmine/tests/contour_test.js index a4d6aa9d153..4e98952d72a 100644 --- a/test/jasmine/tests/contour_test.js +++ b/test/jasmine/tests/contour_test.js @@ -66,7 +66,7 @@ describe('contour defaults', function() { it('should default connectgaps to false if `z` is not a one dimensional array', function() { traceIn = { - type: 'heatmap', + type: 'contour', z: [[0, null], [1, 2]] }; @@ -76,7 +76,7 @@ describe('contour defaults', function() { it('should default connectgaps to true if `z` is a one dimensional array', function() { traceIn = { - type: 'heatmap', + type: 'contour', x: [0, 1, 0, 1], y: [0, 0, 1, 1], z: [0, null, 1, 2] @@ -591,3 +591,63 @@ describe('contour plotting and editing', function() { .then(done); }); }); + +describe('contour hover', function() { + 'use strict'; + + var gd; + + function _hover(gd, xval, yval) { + var fullLayout = gd._fullLayout; + var calcData = gd.calcdata; + var hoverData = []; + + for(var i = 0; i < calcData.length; i++) { + var pointData = { + index: false, + distance: 20, + cd: calcData[i], + trace: calcData[i][0].trace, + xa: fullLayout.xaxis, + ya: fullLayout.yaxis + }; + + var hoverPoint = Contour.hoverPoints(pointData, xval, yval); + if(hoverPoint) hoverData.push(hoverPoint[0]); + } + + return hoverData; + } + + function assertLabels(hoverPoint, xLabel, yLabel, zLabel, text) { + expect(hoverPoint.xLabelVal).toBe(xLabel, 'have correct x label'); + expect(hoverPoint.yLabelVal).toBe(yLabel, 'have correct y label'); + expect(hoverPoint.zLabelVal).toBe(zLabel, 'have correct z label'); + expect(hoverPoint.text).toBe(text, 'have correct text label'); + } + + describe('missing data', function() { + beforeAll(function(done) { + gd = createGraphDiv(); + + Plotly.plot(gd, { + data: [{ + type: 'contour', + x: [10, 11, 10, 11], + y: [100, 100, 101, 101], + z: [null, 1, 2, 3], + connectgaps: false, + hovergaps: false + }] + }).then(done); + }); + afterAll(destroyGraphDiv); + + it('should not create zLabels when hovering on missing data and hovergaps is disabled', function() { + var pt = _hover(gd, 10, 100)[0]; + + expect(pt.index).toEqual([0, 0], 'have correct index'); + assertLabels(pt, 10, 100, undefined); + }); + }); +}); diff --git a/test/jasmine/tests/heatmap_test.js b/test/jasmine/tests/heatmap_test.js index 91422d1ff00..ba94d1e4009 100644 --- a/test/jasmine/tests/heatmap_test.js +++ b/test/jasmine/tests/heatmap_test.js @@ -976,4 +976,29 @@ describe('heatmap hover', function() { .then(done); }); }); + + describe('missing data', function() { + beforeAll(function(done) { + gd = createGraphDiv(); + + Plotly.plot(gd, { + data: [{ + type: 'heatmap', + x: [10, 11, 10, 11], + y: [100, 100, 101, 101], + z: [null, 1, 2, 3], + connectgaps: false, + hovergaps: false + }] + }).then(done); + }); + afterAll(destroyGraphDiv); + + it('should not create zLabels when hovering on missing data and hovergaps is disabled', function() { + var pt = _hover(gd, 10, 100)[0]; + + expect(pt.index).toEqual([0, 0], 'have correct index'); + assertLabels(pt, 10, 100, undefined); + }); + }); }); From dd3cfaf7984f5678a886f9ff9dc89c3d55e563e0 Mon Sep 17 00:00:00 2001 From: archmoj Date: Mon, 21 Oct 2019 17:03:09 -0400 Subject: [PATCH 2/4] rename hovergaps to hoverongaps --- src/traces/contour/attributes.js | 2 +- src/traces/contour/defaults.js | 2 +- src/traces/heatmap/attributes.js | 2 +- src/traces/heatmap/defaults.js | 2 +- src/traces/heatmap/hover.js | 2 +- test/jasmine/tests/contour_test.js | 4 ++-- test/jasmine/tests/heatmap_test.js | 4 ++-- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/traces/contour/attributes.js b/src/traces/contour/attributes.js index 0914aba111f..3e9431b4e25 100644 --- a/src/traces/contour/attributes.js +++ b/src/traces/contour/attributes.js @@ -38,7 +38,7 @@ module.exports = extendFlat({ ytype: heatmapAttrs.ytype, zhoverformat: heatmapAttrs.zhoverformat, hovertemplate: heatmapAttrs.hovertemplate, - hovergaps: heatmapAttrs.hovergaps, + hoverongaps: heatmapAttrs.hoverongaps, connectgaps: extendFlat({}, heatmapAttrs.connectgaps, { description: [ 'Determines whether or not gaps', diff --git a/src/traces/contour/defaults.js b/src/traces/contour/defaults.js index 31066584145..fbc91a2c8a5 100644 --- a/src/traces/contour/defaults.js +++ b/src/traces/contour/defaults.js @@ -35,7 +35,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout coerce('text'); coerce('hovertext'); coerce('hovertemplate'); - coerce('hovergaps'); + coerce('hoverongaps'); var isConstraint = (coerce('contours.type') === 'constraint'); coerce('connectgaps', Lib.isArray1D(traceOut.z)); diff --git a/src/traces/heatmap/attributes.js b/src/traces/heatmap/attributes.js index 177af98354a..6bcacb08a49 100644 --- a/src/traces/heatmap/attributes.js +++ b/src/traces/heatmap/attributes.js @@ -79,7 +79,7 @@ module.exports = extendFlat({ 'Picks a smoothing algorithm use to smooth `z` data.' ].join(' ') }, - hovergaps: { + hoverongaps: { valType: 'boolean', dflt: true, role: 'style', diff --git a/src/traces/heatmap/defaults.js b/src/traces/heatmap/defaults.js index f499ad8391d..0c2c5705254 100644 --- a/src/traces/heatmap/defaults.js +++ b/src/traces/heatmap/defaults.js @@ -34,7 +34,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout handleStyleDefaults(traceIn, traceOut, coerce, layout); - coerce('hovergaps'); + coerce('hoverongaps'); coerce('connectgaps', Lib.isArray1D(traceOut.z) && (traceOut.zsmooth !== false)); colorscaleDefaults(traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'z'}); diff --git a/src/traces/heatmap/hover.js b/src/traces/heatmap/hover.js index 24409dfbc5e..b706541f0c1 100644 --- a/src/traces/heatmap/hover.js +++ b/src/traces/heatmap/hover.js @@ -112,7 +112,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, hoverLay var zVal = z[ny][nx]; if(zmask && !zmask[ny][nx]) zVal = undefined; - if(zVal !== undefined || trace.hovergaps) { + if(zVal !== undefined || trace.hoverongaps) { // dummy axis for formatting the z value var cOpts = extractOpts(trace); var dummyAx = { diff --git a/test/jasmine/tests/contour_test.js b/test/jasmine/tests/contour_test.js index 4e98952d72a..617bd043bd7 100644 --- a/test/jasmine/tests/contour_test.js +++ b/test/jasmine/tests/contour_test.js @@ -637,13 +637,13 @@ describe('contour hover', function() { y: [100, 100, 101, 101], z: [null, 1, 2, 3], connectgaps: false, - hovergaps: false + hoverongaps: false }] }).then(done); }); afterAll(destroyGraphDiv); - it('should not create zLabels when hovering on missing data and hovergaps is disabled', function() { + it('should not create zLabels when hovering on missing data and hoverongaps is disabled', function() { var pt = _hover(gd, 10, 100)[0]; expect(pt.index).toEqual([0, 0], 'have correct index'); diff --git a/test/jasmine/tests/heatmap_test.js b/test/jasmine/tests/heatmap_test.js index ba94d1e4009..8f9fa666042 100644 --- a/test/jasmine/tests/heatmap_test.js +++ b/test/jasmine/tests/heatmap_test.js @@ -988,13 +988,13 @@ describe('heatmap hover', function() { y: [100, 100, 101, 101], z: [null, 1, 2, 3], connectgaps: false, - hovergaps: false + hoverongaps: false }] }).then(done); }); afterAll(destroyGraphDiv); - it('should not create zLabels when hovering on missing data and hovergaps is disabled', function() { + it('should not create zLabels when hovering on missing data and hoverongaps is disabled', function() { var pt = _hover(gd, 10, 100)[0]; expect(pt.index).toEqual([0, 0], 'have correct index'); From fbaf63656983db2759345038a0365147ad702b06 Mon Sep 17 00:00:00 2001 From: archmoj Date: Mon, 21 Oct 2019 17:25:29 -0400 Subject: [PATCH 3/4] modify hoverongaps description --- src/traces/heatmap/attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/traces/heatmap/attributes.js b/src/traces/heatmap/attributes.js index 6bcacb08a49..f63fb40a1c9 100644 --- a/src/traces/heatmap/attributes.js +++ b/src/traces/heatmap/attributes.js @@ -87,7 +87,7 @@ module.exports = extendFlat({ description: [ 'Determines whether or not gaps', '(i.e. {nan} or missing values)', - 'in the `z` data are hovered on.' + 'in the `z` data have hover labels associated with them.' ].join(' ') }, connectgaps: { From 92693525664f8d067385b410f268d2b36b3f058e Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 22 Oct 2019 11:38:48 -0400 Subject: [PATCH 4/4] revert heatmap hover and only return early when z is undefined and hoverongaps is false - adjust new tests to make sure plotly_hover is not triggered on gaps when hoverongaps is false --- src/traces/heatmap/hover.js | 42 ++++++++++++++---------------- test/jasmine/tests/contour_test.js | 18 ++++++------- test/jasmine/tests/heatmap_test.js | 11 +++++--- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/traces/heatmap/hover.js b/src/traces/heatmap/hover.js index b706541f0c1..b29fe1498ff 100644 --- a/src/traces/heatmap/hover.js +++ b/src/traces/heatmap/hover.js @@ -88,6 +88,11 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, hoverLay } } + var zVal = z[ny][nx]; + if(zmask && !zmask[ny][nx]) zVal = undefined; + + if(zVal === undefined && !trace.hoverongaps) return; + var text; if(Array.isArray(cd0.hovertext) && Array.isArray(cd0.hovertext[ny])) { text = cd0.hovertext[ny][nx]; @@ -95,7 +100,18 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, hoverLay text = cd0.text[ny][nx]; } - var obj = { + // dummy axis for formatting the z value + var cOpts = extractOpts(trace); + var dummyAx = { + type: 'linear', + range: [cOpts.min, cOpts.max], + hoverformat: zhoverformat, + _separators: xa._separators, + _numFormat: xa._numFormat + }; + var zLabel = Axes.tickText(dummyAx, zVal, 'hover').text; + + return [Lib.extendFlat(pointData, { index: [ny, nx], // never let a 2D override 1D type as closest point distance: pointData.maxHoverDistance, @@ -106,26 +122,8 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, hoverLay y1: y1, xLabelVal: xl, yLabelVal: yl, + zLabelVal: zVal, + zLabel: zLabel, text: text - }; - - var zVal = z[ny][nx]; - if(zmask && !zmask[ny][nx]) zVal = undefined; - - if(zVal !== undefined || trace.hoverongaps) { - // dummy axis for formatting the z value - var cOpts = extractOpts(trace); - var dummyAx = { - type: 'linear', - range: [cOpts.min, cOpts.max], - hoverformat: zhoverformat, - _separators: xa._separators, - _numFormat: xa._numFormat - }; - - obj.zLabelVal = zVal; - obj.zLabel = Axes.tickText(dummyAx, zVal, 'hover').text; - } - - return [Lib.extendFlat(pointData, obj)]; + })]; }; diff --git a/test/jasmine/tests/contour_test.js b/test/jasmine/tests/contour_test.js index 617bd043bd7..a8ee906f110 100644 --- a/test/jasmine/tests/contour_test.js +++ b/test/jasmine/tests/contour_test.js @@ -619,13 +619,6 @@ describe('contour hover', function() { return hoverData; } - function assertLabels(hoverPoint, xLabel, yLabel, zLabel, text) { - expect(hoverPoint.xLabelVal).toBe(xLabel, 'have correct x label'); - expect(hoverPoint.yLabelVal).toBe(yLabel, 'have correct y label'); - expect(hoverPoint.zLabelVal).toBe(zLabel, 'have correct z label'); - expect(hoverPoint.text).toBe(text, 'have correct text label'); - } - describe('missing data', function() { beforeAll(function(done) { gd = createGraphDiv(); @@ -643,11 +636,16 @@ describe('contour hover', function() { }); afterAll(destroyGraphDiv); - it('should not create zLabels when hovering on missing data and hoverongaps is disabled', function() { + it('should not display hover on missing data and hoverongaps is disabled', function() { var pt = _hover(gd, 10, 100)[0]; - expect(pt.index).toEqual([0, 0], 'have correct index'); - assertLabels(pt, 10, 100, undefined); + var hoverData; + gd.on('plotly_hover', function(data) { + hoverData = data; + }); + + expect(hoverData).toEqual(undefined); + expect(pt).toEqual(undefined); }); }); }); diff --git a/test/jasmine/tests/heatmap_test.js b/test/jasmine/tests/heatmap_test.js index 8f9fa666042..ce464809af4 100644 --- a/test/jasmine/tests/heatmap_test.js +++ b/test/jasmine/tests/heatmap_test.js @@ -994,11 +994,16 @@ describe('heatmap hover', function() { }); afterAll(destroyGraphDiv); - it('should not create zLabels when hovering on missing data and hoverongaps is disabled', function() { + it('should not display hover on missing data and hoverongaps is disabled', function() { var pt = _hover(gd, 10, 100)[0]; - expect(pt.index).toEqual([0, 0], 'have correct index'); - assertLabels(pt, 10, 100, undefined); + var hoverData; + gd.on('plotly_hover', function(data) { + hoverData = data; + }); + + expect(hoverData).toEqual(undefined); + expect(pt).toEqual(undefined); }); }); });