Skip to content

Commit

Permalink
Merge pull request #4269 from plotly/fix-rehover-on-exiting-subplots
Browse files Browse the repository at this point in the history
Do not attempt to re-hover on exiting subplots
  • Loading branch information
etpinard authored Oct 15, 2019
2 parents b3c1e7a + f6a47b2 commit 8230da5
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 19 deletions.
28 changes: 12 additions & 16 deletions src/components/fx/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,24 +224,20 @@ 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);
continue;
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;
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;
Expand Down
2 changes: 1 addition & 1 deletion src/plots/cartesian/graph_interact.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/plots/mapbox/mapbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};
Expand Down
86 changes: 85 additions & 1 deletion test/jasmine/tests/hover_label_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -2150,7 +2185,6 @@ describe('hover info on stacked subplots', function() {
});
});


describe('hover on many lines+bars', function() {
'use strict';

Expand Down Expand Up @@ -2607,6 +2641,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() {
Expand Down
33 changes: 33 additions & 0 deletions test/jasmine/tests/mapbox_test.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 8230da5

Please sign in to comment.