Skip to content

Fix scattergl select -> dblclick -> pan bug #2815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 15, 2018
Merged

Fix scattergl select -> dblclick -> pan bug #2815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad06a12
pass dragbox element to mouseEvent
etpinard Jul 13, 2018
b70ebcc
fix #2767 - reset scatter2d markerOptions in scene.plot
etpinard Jul 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/traces/scattergl/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,12 @@ function plot(gd, subplot, cdata) {
}
});
}
} else {
if(scene.scatter2d) {
// reset scatter2d opts to base opts,
// thus unsetting markerUnselectedOptions from selection
scene.scatter2d.update(scene.markerOptions);
}
}

// upload viewport/range data to GPU
Expand Down
181 changes: 177 additions & 4 deletions test/jasmine/tests/gl2d_click_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var assertHoverLabelContent = customAssertions.assertHoverLabelContent;
// from the mousemove events and then simulate
// a click event on mouseup
var click = require('../assets/timed_click');
var doubleClick = require('../assets/double_click');
var hover = require('../assets/hover');
var delay = require('../assets/delay');
var mouseEvent = require('../assets/mouse_event');
Expand Down Expand Up @@ -593,17 +594,19 @@ describe('@noCI @gl Test gl2d lasso/select:', function() {

function drag(path) {
var len = path.length;
var el = d3.select(gd).select('rect.nsewdrag').node();
var opts = {element: el};

Lib.clearThrottle();
mouseEvent('mousemove', path[0][0], path[0][1]);
mouseEvent('mousedown', path[0][0], path[0][1]);
mouseEvent('mousemove', path[0][0], path[0][1], opts);
mouseEvent('mousedown', path[0][0], path[0][1], opts);

path.slice(1, len).forEach(function(pt) {
Lib.clearThrottle();
mouseEvent('mousemove', pt[0], pt[1]);
mouseEvent('mousemove', pt[0], pt[1], opts);
});

mouseEvent('mouseup', path[len - 1][0], path[len - 1][1]);
mouseEvent('mouseup', path[len - 1][0], path[len - 1][1], opts);
}

function select(path) {
Expand Down Expand Up @@ -961,4 +964,174 @@ describe('@noCI @gl Test gl2d lasso/select:', function() {
.catch(failTest)
.then(done);
});

it('should behave correctly during select+doubleclick+pan scenarios', function(done) {
gd = createGraphDiv();

// See https://github.com/plotly/plotly.js/issues/2767

function grabScene() {
return gd.calcdata[0][0].t._scene;
}

function _assert(msg, exp) {
var scene = grabScene();
var scatter2d = scene.scatter2d;

expect((scene.markerOptions || [])[0].opacity)
.toBe(1, 'marker.opacity - ' + msg);
expect((scene.markerSelectedOptions || [])[0].opacity)
.toBe(1, 'selected.marker.opacity - ' + msg);
expect((scene.markerUnselectedOptions || [])[0].opacity)
.toBe(0.2, 'unselected.marker.opacity - ' + msg);

expect(scene.selectBatch).toEqual(exp.selectBatch);
expect(scene.unselectBatch).toEqual(exp.unselectBatch);

var updateCalls = scatter2d.update.calls.all();
var drawCalls = scatter2d.draw.calls.all();

expect(updateCalls.length).toBe(
exp.updateArgs.length,
'scatter2d.update has been called the correct number of times - ' + msg
);
updateCalls.forEach(function(d, i) {
d.args.forEach(function(arg, j) {
if('range' in arg[0]) {
// no need to assert range value in detail
expect(exp.updateArgs[i][j]).toBe(
'range',
'scatter.update range update - ' + msg
);
} else {
expect(arg).toBe(
exp.updateArgs[i][j],
'scatter.update call' + i + ' arg' + j + ' - ' + msg
);
}
});
});

expect(drawCalls.length).toBe(
exp.drawArgs.length,
'scatter2d.draw has been called the correct number of times - ' + msg
);
drawCalls.forEach(function(d, i) {
d.args.forEach(function(arg, j) {
expect(arg).toBe(
exp.drawArgs[i][j],
'scatter.draw call' + i + ' arg' + j + ' - ' + msg
);
});
});

scene.scatter2d.update.calls.reset();
scene.scatter2d.draw.calls.reset();
}

var unselectBatchOld;

Plotly.newPlot('graph', [{
type: 'scattergl',
mode: 'markers',
y: [1, 2, 1],
marker: {size: 30}
}], {
dragmode: 'select',
margin: {t: 0, b: 0, l: 0, r: 0},
width: 500,
height: 500
})
.then(delay(100))
.then(function() {
var scene = grabScene();
spyOn(scene.scatter2d, 'update').and.callThrough();
spyOn(scene.scatter2d, 'draw').and.callThrough();
})
.then(function() {
_assert('base', {
selectBatch: [],
unselectBatch: [],
updateArgs: [],
drawArgs: []
});
})
.then(function() { return select([[20, 20], [480, 250]]); })
.then(function() {
var scene = grabScene();
_assert('after select', {
selectBatch: [[1]],
unselectBatch: [[0, 2]],
updateArgs: [
// N.B. scatter2d now draws unselected options
[scene.markerUnselectedOptions],
],
drawArgs: [
[scene.unselectBatch]
]
});
})
.then(function() { return doubleClick(250, 250); })
.then(function() {
var scene = grabScene();
_assert('after doubleclick', {
selectBatch: [null],
unselectBatch: [[0, 1, 2]],
updateArgs: [],
drawArgs: [
// call in no-selection loop (can we get rid of this?)
[0],
// call with unselectBatch
[scene.unselectBatch]
]
});
})
.then(function() { return Plotly.relayout(gd, 'dragmode', 'pan'); })
.then(function() {
_assert('after relayout to *pan*', {
selectBatch: [null],
unselectBatch: [[0, 1, 2]],
// nothing to do when relayouting to 'pan'
updateArgs: [],
drawArgs: []
});

// keep ref for next _assert()
var scene = grabScene();
unselectBatchOld = scene.unselectBatch;
})
.then(function() { return drag([[200, 200], [250, 250]]); })
.then(function() {
var scene = grabScene();
_assert('after pan', {
selectBatch: null,
unselectBatch: null,
// drag triggers:
// - 2 scene.update() calls, which each invoke
// + 1 scatter2d.update (updating viewport)
// + 2 scatter2d.draw (same as after double-click)
//
// replot on mouseup triggers:
// - 1 scatter2d.update updating viewport
// - 1 scatter2d.update resetting markerOptions
// - 1 (full) scene.draw()
updateArgs: [
['range'],
['range'],
// N.B. bring scatter2d back to 'base' marker options
[scene.markerOptions],
['range']
],
drawArgs: [
[0],
[unselectBatchOld],
[0],
[unselectBatchOld],
[0]
]
});
})
.catch(failTest)
.then(done);
});
});