From 6347b0d0eb2dec631e76dd4495da5946ec1f388c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 10 Oct 2019 14:59:40 -0400 Subject: [PATCH 1/4] do not attempt to re-hover on exiting subplots --- src/plots/cartesian/graph_interact.js | 2 +- test/jasmine/tests/hover_label_test.js | 50 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index 716fc1b120d..8013a697d8d 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -59,7 +59,7 @@ exports.initInteractions = function initInteractions(gd) { // This is on `gd._fullLayout`, *not* fullLayout because the reference // changes by the time this is called again. gd._fullLayout._rehover = function() { - if(gd._fullLayout._hoversubplot === subplot) { + if((gd._fullLayout._hoversubplot === subplot) && gd._fullLayout._plots[subplot]) { Fx.hover(gd, evt, subplot); } }; diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 9f11b03b07b..281ce864302 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -2607,6 +2607,56 @@ describe('hover updates', function() { }) .then(done); }); + + it('should not attempt to rehover over exiting subplots', function(done) { + spyOn(Fx, 'hover').and.callThrough(); + + var data = [ + {y: [1], hoverinfo: 'y'}, + {y: [2], hoverinfo: 'y', xaxis: 'x2', yaxis: 'y2'} + ]; + + var data2 = [ + {y: [1], hoverinfo: 'y'} + ]; + + var layout = { + xaxis: {domain: [0, 0.5]}, + xaxis2: {anchor: 'y2', domain: [0.5, 1]}, + yaxis2: {anchor: 'x2'}, + width: 400, + height: 200, + margin: {l: 0, t: 0, r: 0, b: 0}, + showlegend: false + }; + + var gd = createGraphDiv(); + var onPt2 = [300, 100]; + var offPt2 = [250, 100]; + + Plotly.react(gd, data, layout) + .then(function() { + assertLabelsCorrect(onPt2, [303, 100], '2', 'after 1st draw [on-pt]'); + assertLabelsCorrect(offPt2, null, null, 'after 1st draw [off-pt]'); + expect(Fx.hover).toHaveBeenCalledTimes(2); + }) + .then(function() { + var promise = Plotly.react(gd, data2, layout); + assertLabelsCorrect(onPt2, null, null, '2', 'before react() resolves [on-pt]'); + assertLabelsCorrect(offPt2, null, null, 'before react() resolves [off-pt]'); + // N.B. no calls from Plots.rehover() as x2y2 subplot got removed! + expect(Fx.hover).toHaveBeenCalledTimes(2); + return promise; + }) + .then(function() { + expect(Fx.hover).toHaveBeenCalledTimes(2); + assertLabelsCorrect(onPt2, null, null, '2', 'after react() resolves [on-pt]'); + assertLabelsCorrect(offPt2, null, null, 'after react() resolves [off-pt]'); + expect(Fx.hover).toHaveBeenCalledTimes(2); + }) + .catch(failTest) + .then(done); + }); }); describe('Test hover label custom styling:', function() { From 89e52f71eb663bbaa0e045757c78c38454991569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 10 Oct 2019 17:49:48 -0400 Subject: [PATCH 2/4] warn + exit early when Fx.hover is called w/ unrecognized subplot id --- src/components/fx/hover.js | 14 +++++----- test/jasmine/tests/hover_label_test.js | 36 +++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index a108240b916..97469111884 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -235,13 +235,15 @@ function _hover(gd, evt, subplot, noHoverEvent) { xaArray[i] = Axes.getFromId(gd, plotObj.xaxis._id); yaArray[i] = Axes.getFromId(gd, plotObj.yaxis._id); - continue; + } else if(fullLayout[spId] && fullLayout[spId]._subplot) { + // other subplot types + var _subplot = fullLayout[spId]._subplot; + xaArray[i] = _subplot.xaxis; + yaArray[i] = _subplot.yaxis; + } else { + Lib.warn('Unrecognized subplot: ' + spId); + return; } - - // other subplot types - var _subplot = fullLayout[spId]._subplot; - xaArray[i] = _subplot.xaxis; - yaArray[i] = _subplot.yaxis; } var hovermode = evt.hovermode || fullLayout.hovermode; diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 281ce864302..1f2dfd9464b 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -36,6 +36,41 @@ function touch(path, options) { return; } +describe('Fx.hover:', function() { + var gd; + + beforeEach(function() { gd = createGraphDiv(); }); + + afterEach(destroyGraphDiv); + + it('should warn when passing subplot ids that are not part of the graph', function(done) { + spyOn(Lib, 'warn'); + + var data = [ + {y: [1], hoverinfo: 'y'} + ]; + + var layout = { + xaxis: {domain: [0, 0.5]}, + xaxis2: {anchor: 'y2', domain: [0.5, 1]}, + yaxis2: {anchor: 'x2'}, + width: 400, + height: 200, + margin: {l: 0, t: 0, r: 0, b: 0}, + showlegend: false + }; + + Plotly.plot(gd, data, layout) + .then(function() { + Fx.hover(gd, {xpx: 300, ypx: 100}, 'x2y2'); + expect(gd._hoverdata).toBe(undefined, 'did not generate hoverdata'); + expect(Lib.warn).toHaveBeenCalledWith('Unrecognized subplot: x2y2'); + }) + .catch(failTest) + .then(done); + }); +}); + describe('hover info', function() { 'use strict'; @@ -2150,7 +2185,6 @@ describe('hover info on stacked subplots', function() { }); }); - describe('hover on many lines+bars', function() { 'use strict'; From d8a05126144df4bbe4477221b09071de05b04ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 10 Oct 2019 17:57:20 -0400 Subject: [PATCH 3/4] try to :hocho: (very) old sync/out-of-sync logic --- src/components/fx/hover.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 97469111884..7d76f671c54 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -224,17 +224,11 @@ function _hover(gd, evt, subplot, noHoverEvent) { for(var i = 0; i < len; i++) { var spId = subplots[i]; - // 'cartesian' case - var plotObj = plots[spId]; - if(plotObj) { + if(plots[spId]) { + // 'cartesian' case supportsCompare = true; - - // TODO make sure that fullLayout_plots axis refs - // get updated properly so that we don't have - // to use Axes.getFromId in general. - - xaArray[i] = Axes.getFromId(gd, plotObj.xaxis._id); - yaArray[i] = Axes.getFromId(gd, plotObj.yaxis._id); + xaArray[i] = plots[spId].xaxis; + yaArray[i] = plots[spId].yaxis; } else if(fullLayout[spId] && fullLayout[spId]._subplot) { // other subplot types var _subplot = fullLayout[spId]._subplot; From f6a47b2a74f64fee86543f7dc72dbddd341b7525 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 11 Oct 2019 10:57:55 -0400 Subject: [PATCH 4/4] do not attempt to re-hover on exiting mapbox subplots --- src/plots/mapbox/mapbox.js | 2 +- test/jasmine/tests/mapbox_test.js | 33 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index dbc6e866549..bc798ca3ae2 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -475,7 +475,7 @@ proto.initFx = function(calcData, fullLayout) { self.yaxis.p2c = function() { return evt.lngLat.lat; }; gd._fullLayout._rehover = function() { - if(gd._fullLayout._hoversubplot === self.id) { + if(gd._fullLayout._hoversubplot === self.id && gd._fullLayout[self.id]) { Fx.hover(gd, evt, self.id); } }; diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index 4981fc37bf2..b92e8ad1318 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -1,5 +1,6 @@ var Plotly = require('@lib'); var Lib = require('@src/lib'); +var Fx = require('@src/components/fx'); var constants = require('@src/plots/mapbox/constants'); var supplyLayoutDefaults = require('@src/plots/mapbox/layout_defaults'); @@ -1155,6 +1156,38 @@ describe('@noCI, mapbox plots', function() { .then(done); }, LONG_TIMEOUT_INTERVAL); + it('@gl should not attempt to rehover over exiting subplots', function(done) { + spyOn(Fx, 'hover').and.callThrough(); + + function countHoverLabels() { + return d3.select('.hoverlayer').selectAll('g').size(); + } + + Promise.resolve() + .then(function() { + return _mouseEvent('mousemove', pointPos, function() { + expect(countHoverLabels()).toEqual(1); + expect(Fx.hover).toHaveBeenCalledTimes(1); + expect(Fx.hover.calls.argsFor(0)[2]).toBe('mapbox'); + Fx.hover.calls.reset(); + }); + }) + .then(function() { return Plotly.deleteTraces(gd, [0, 1]); }) + .then(delay(10)) + .then(function() { + return _mouseEvent('mousemove', pointPos, function() { + expect(countHoverLabels()).toEqual(0); + // N.B. no additional calls from Plots.rehover() + // (as 'mapbox' subplot is gone), + // just one on the fallback xy subplot + expect(Fx.hover).toHaveBeenCalledTimes(1); + expect(Fx.hover.calls.argsFor(0)[2]).toBe('xy'); + }); + }) + .catch(failTest) + .then(done); + }, LONG_TIMEOUT_INTERVAL); + it('@gl should respond drag / scroll / double-click interactions', function(done) { var relayoutCnt = 0; var doubleClickCnt = 0;