From becceceb9ef66977fcd91503da3198bc25b65fbc Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Tue, 19 Apr 2016 21:27:49 +0100 Subject: [PATCH 01/11] Add zoomlayer Fixes #359 --- src/plot_api/plot_api.js | 1 + src/plots/cartesian/dragbox.js | 13 +++++++++---- src/plots/cartesian/select.js | 6 +++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 24f06adc781..0bc40c84f9b 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2654,6 +2654,7 @@ function makePlotFramework(gd) { // these are in a different svg element normally, but get collapsed into a single // svg when exporting (after inserting 3D) fullLayout._infolayer = fullLayout._toppaper.append('g').classed('infolayer', true); + fullLayout._zoomlayer = fullLayout._toppaper.append('g').classed('zoomlayer', true); fullLayout._hoverlayer = fullLayout._toppaper.append('g').classed('hoverlayer', true); gd.emit('plotly_framework'); diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 76e5976aabd..6f9dd42b868 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -139,7 +139,10 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { dragElement.init(dragOptions); - var x0, + var zoomlayer = gd._fullLayout._zoomlayer, + xs = plotinfo.x()._offset, + ys = plotinfo.y()._offset, + x0, y0, box, lum, @@ -161,15 +164,16 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { dimmed = false; zoomMode = 'xy'; - zb = plotinfo.plot.append('path') + zb = zoomlayer.append('path') .attr('class', 'zoombox') .style({ 'fill': lum>0.2 ? 'rgba(0,0,0,0)' : 'rgba(255,255,255,0)', 'stroke-width': 0 }) + .attr('transform','translate(' + xs + ' ' + ys + ')') .attr('d', path0 + 'Z'); - corners = plotinfo.plot.append('path') + corners = zoomlayer.append('path') .attr('class', 'zoombox-corners') .style({ fill: Color.background, @@ -177,6 +181,7 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { 'stroke-width': 1, opacity: 0 }) + .attr('transform','translate(' + xs + ' ' + ys + ')') .attr('d','M0,0Z'); clearSelect(); @@ -187,7 +192,7 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { // until we get around to persistent selections, remove the outline // here. The selection itself will be removed when the plot redraws // at the end. - plotinfo.plot.selectAll('.select-outline').remove(); + zoomlayer.selectAll('.select-outline').remove(); } function zoomMove(dx0, dy0) { diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js index 3c82d1ccdff..64179d0fbab 100644 --- a/src/plots/cartesian/select.js +++ b/src/plots/cartesian/select.js @@ -22,8 +22,10 @@ var MINSELECT = constants.MINSELECT; function getAxId(ax) { return ax._id; } module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { - var plot = dragOptions.plotinfo.plot, + var plot = dragOptions.gd._fullLayout._zoomlayer, dragBBox = dragOptions.element.getBoundingClientRect(), + xs = dragOptions.plotinfo.x()._offset, + ys = dragOptions.plotinfo.y()._offset, x0 = startX - dragBBox.left, y0 = startY - dragBBox.top, x1 = x0, @@ -45,6 +47,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { outlines.enter() .append('path') .attr('class', function(d) { return 'select-outline select-outline-' + d; }) + .attr('transform','translate(' + xs + ' ' + ys + ')') .attr('d', path0 + 'Z'); var corners = plot.append('path') @@ -54,6 +57,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { stroke: color.defaultLine, 'stroke-width': 1 }) + .attr('transform','translate(' + xs + ' ' + ys + ')') .attr('d','M0,0Z'); From 5df31d2a6646b37f6b736a6741e8b3eac3b85f66 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Tue, 5 Apr 2016 20:58:04 +0100 Subject: [PATCH 02/11] Add more tests to select_test.js * Test whether select elements are appended to the zoom layer. * Test whether lasso elements are appended to the zoom layer. --- test/jasmine/tests/select_test.js | 76 +++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index 6eb5b27231e..c0b027216cc 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -1,3 +1,5 @@ +var d3 = require('d3'); + var Plotly = require('@lib/index'); var Lib = require('@src/lib'); var DBLCLICKDELAY = require('@src/plots/cartesian/constants').DBLCLICKDELAY; @@ -54,6 +56,80 @@ describe('select box and lasso', function() { expect(actual.y).toBeCloseToArray(expected.y, PRECISION); } + describe('select elements', function() { + var mockCopy = Lib.extendDeep({}, mock); + mockCopy.layout.dragmode = 'select'; + + var gd; + beforeEach(function(done) { + gd = createGraphDiv(); + + Plotly.plot(gd, mockCopy.data, mockCopy.layout) + .then(done); + }); + + it('should be appended to the zoom layer', function() { + var x0 = 100; + var y0 = 200; + var x1 = 150; + var y1 = 200; + + mouseEvent('mousemove', x0, y0); + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(0); + + mouseEvent('mousedown', x0, y0); + mouseEvent('mousemove', x1, y1); + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(1); + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(2); + + mouseEvent('mouseup', x1, y1); + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(0); + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(2); + }); + }); + + describe('lasso elements', function() { + var mockCopy = Lib.extendDeep({}, mock); + mockCopy.layout.dragmode = 'lasso'; + + var gd; + beforeEach(function(done) { + gd = createGraphDiv(); + + Plotly.plot(gd, mockCopy.data, mockCopy.layout) + .then(done); + }); + + it('should be appended to the the shape layer', function() { + var x0 = 100; + var y0 = 200; + var x1 = 150; + var y1 = 200; + + mouseEvent('mousemove', x0, y0); + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(0); + + mouseEvent('mousedown', x0, y0); + mouseEvent('mousemove', x1, y1); + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(1); + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(2); + + mouseEvent('mouseup', x1, y1); + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(0); + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(2); + }); + }); + describe('select events', function() { var mockCopy = Lib.extendDeep({}, mock); mockCopy.layout.dragmode = 'select'; From 2fa50c03eb6e20a320651643da50a06104363fee Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Tue, 5 Apr 2016 21:28:14 +0100 Subject: [PATCH 03/11] Add zoom tests --- test/jasmine/tests/zoom_test.js | 51 +++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 test/jasmine/tests/zoom_test.js diff --git a/test/jasmine/tests/zoom_test.js b/test/jasmine/tests/zoom_test.js new file mode 100644 index 00000000000..38c09608cef --- /dev/null +++ b/test/jasmine/tests/zoom_test.js @@ -0,0 +1,51 @@ +var d3 = require('d3'); + +var Plotly = require('@lib/index'); +var Lib = require('@src/lib'); + +var createGraphDiv = require('../assets/create_graph_div'); +var destroyGraphDiv = require('../assets/destroy_graph_div'); +var mouseEvent = require('../assets/mouse_event'); + + +describe('zoom box element', function() { + var mock = require('@mocks/14.json'); + + var gd; + beforeEach(function(done) { + gd = createGraphDiv(); + + var mockCopy = Lib.extendDeep({}, mock); + mockCopy.layout.dragmode = 'zoom'; + + Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(done); + }); + + afterEach(destroyGraphDiv); + + it('should be appended to the zoom layer', function() { + var x0 = 100; + var y0 = 200; + var x1 = 150; + var y1 = 200; + + mouseEvent('mousemove', x0, y0); + expect(d3.selectAll('.zoomlayer > .zoombox').size()) + .toEqual(0); + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(0); + + mouseEvent('mousedown', x0, y0); + mouseEvent('mousemove', x1, y1); + expect(d3.selectAll('.zoomlayer > .zoombox').size()) + .toEqual(1); + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(1); + + mouseEvent('mouseup', x1, y1); + expect(d3.selectAll('.zoomlayer > .zoombox').size()) + .toEqual(0); + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(0); + }); +}); From 319a83bcdd93068ed41f44db842f14b1b06e122b Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 20 Apr 2016 09:09:40 +0100 Subject: [PATCH 04/11] Fix test description in select_test.js --- test/jasmine/tests/select_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index c0b027216cc..ef924e3f833 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -105,7 +105,7 @@ describe('select box and lasso', function() { .then(done); }); - it('should be appended to the the shape layer', function() { + it('should be appended to the zoom layer', function() { var x0 = 100; var y0 = 200; var x1 = 150; From 78effe6a0ec32d6c2a45fcefea8ca71df4daa67f Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 20 Apr 2016 10:23:58 +0100 Subject: [PATCH 05/11] Rename zoom_test.js to cartesian_test.js --- test/jasmine/tests/{zoom_test.js => cartesian_test.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/jasmine/tests/{zoom_test.js => cartesian_test.js} (100%) diff --git a/test/jasmine/tests/zoom_test.js b/test/jasmine/tests/cartesian_test.js similarity index 100% rename from test/jasmine/tests/zoom_test.js rename to test/jasmine/tests/cartesian_test.js From b7de50fc3b2380c6df5876a3c5eceb725da0162c Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 20 Apr 2016 10:30:29 +0100 Subject: [PATCH 06/11] dragBox: remove corners before emitting an event * Ensured the zoombox corners have been removed before emitting 'plotly_deselect' or 'plotly_selected'. --- src/plots/cartesian/select.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js index 64179d0fbab..5f22ea67d8a 100644 --- a/src/plots/cartesian/select.js +++ b/src/plots/cartesian/select.js @@ -180,6 +180,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { }; dragOptions.doneFn = function(dragged, numclicks) { + corners.remove(); if(!dragged && numclicks === 2) { // clear selection on doubleclick outlines.remove(); @@ -193,6 +194,5 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { else { dragOptions.gd.emit('plotly_selected', eventData); } - corners.remove(); }; }; From 105bed9ea037c7cd6ae743b240dff23dd3f1d398 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 20 Apr 2016 10:40:02 +0100 Subject: [PATCH 07/11] Update select_test.js * Test that a double click removes a selection. --- test/jasmine/tests/select_test.js | 88 ++++++++++++++++++++----------- 1 file changed, 56 insertions(+), 32 deletions(-) diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js index ef924e3f833..5cf94265cea 100644 --- a/test/jasmine/tests/select_test.js +++ b/test/jasmine/tests/select_test.js @@ -68,28 +68,40 @@ describe('select box and lasso', function() { .then(done); }); - it('should be appended to the zoom layer', function() { - var x0 = 100; - var y0 = 200; - var x1 = 150; - var y1 = 200; + it('should be appended to the zoom layer', function(done) { + var x0 = 100, + y0 = 200, + x1 = 150, + y1 = 250, + x2 = 50, + y2 = 50; + + gd.once('plotly_selecting', function() { + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(1); + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(2); + }); + + gd.once('plotly_selected', function() { + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(0); + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(2); + }); + + gd.once('plotly_deselect', function() { + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(0); + }); mouseEvent('mousemove', x0, y0); expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) .toEqual(0); - mouseEvent('mousedown', x0, y0); - mouseEvent('mousemove', x1, y1); - expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) - .toEqual(1); - expect(d3.selectAll('.zoomlayer > .select-outline').size()) - .toEqual(2); + drag([[x0, y0], [x1, y1]]); - mouseEvent('mouseup', x1, y1); - expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) - .toEqual(0); - expect(d3.selectAll('.zoomlayer > .select-outline').size()) - .toEqual(2); + doubleClick(x2, y2, done); }); }); @@ -105,28 +117,40 @@ describe('select box and lasso', function() { .then(done); }); - it('should be appended to the zoom layer', function() { - var x0 = 100; - var y0 = 200; - var x1 = 150; - var y1 = 200; + it('should be appended to the zoom layer', function(done) { + var x0 = 100, + y0 = 200, + x1 = 150, + y1 = 250, + x2 = 50, + y2 = 50; + + gd.once('plotly_selecting', function() { + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(1); + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(2); + }); + + gd.once('plotly_selected', function() { + expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) + .toEqual(0); + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(2); + }); + + gd.once('plotly_deselect', function() { + expect(d3.selectAll('.zoomlayer > .select-outline').size()) + .toEqual(0); + }); mouseEvent('mousemove', x0, y0); expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) .toEqual(0); - mouseEvent('mousedown', x0, y0); - mouseEvent('mousemove', x1, y1); - expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) - .toEqual(1); - expect(d3.selectAll('.zoomlayer > .select-outline').size()) - .toEqual(2); + drag([[x0, y0], [x1, y1]]); - mouseEvent('mouseup', x1, y1); - expect(d3.selectAll('.zoomlayer > .zoombox-corners').size()) - .toEqual(0); - expect(d3.selectAll('.zoomlayer > .select-outline').size()) - .toEqual(2); + doubleClick(x2, y2, done); }); }); From a360992b1e3c3229868a1d8ef4cad6603bf34ef3 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 20 Apr 2016 17:01:14 +0100 Subject: [PATCH 08/11] Use comma with translate transforms --- src/plots/cartesian/dragbox.js | 4 ++-- src/plots/cartesian/select.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 6f9dd42b868..a539e3d7e1b 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -170,7 +170,7 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { 'fill': lum>0.2 ? 'rgba(0,0,0,0)' : 'rgba(255,255,255,0)', 'stroke-width': 0 }) - .attr('transform','translate(' + xs + ' ' + ys + ')') + .attr('transform','translate(' + xs + ', ' + ys + ')') .attr('d', path0 + 'Z'); corners = zoomlayer.append('path') @@ -181,7 +181,7 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { 'stroke-width': 1, opacity: 0 }) - .attr('transform','translate(' + xs + ' ' + ys + ')') + .attr('transform','translate(' + xs + ', ' + ys + ')') .attr('d','M0,0Z'); clearSelect(); diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js index 5f22ea67d8a..b88f1176f6f 100644 --- a/src/plots/cartesian/select.js +++ b/src/plots/cartesian/select.js @@ -47,7 +47,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { outlines.enter() .append('path') .attr('class', function(d) { return 'select-outline select-outline-' + d; }) - .attr('transform','translate(' + xs + ' ' + ys + ')') + .attr('transform','translate(' + xs + ', ' + ys + ')') .attr('d', path0 + 'Z'); var corners = plot.append('path') @@ -57,7 +57,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { stroke: color.defaultLine, 'stroke-width': 1 }) - .attr('transform','translate(' + xs + ' ' + ys + ')') + .attr('transform','translate(' + xs + ', ' + ys + ')') .attr('d','M0,0Z'); From 9032cb78b948641a5ef484bb22a02643d4f08a57 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 20 Apr 2016 11:23:07 +0100 Subject: [PATCH 09/11] Pause before starting the drag-interaction tests * The drag interaction tests in click_test.js may fail when run by `npm run test-jasmine` while they succeed if run by `npm run test-jasmine -- tests/click_test.js`. * These failures disappear when a pause of 200ms is introduced in the `beforeEach` after creating the plot. * Shorter pauses of 0, 10, 20, 50 and 100 ms fail. --- test/jasmine/tests/click_test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/click_test.js b/test/jasmine/tests/click_test.js index 34bf0d71f24..6d1aaa26e40 100644 --- a/test/jasmine/tests/click_test.js +++ b/test/jasmine/tests/click_test.js @@ -180,7 +180,9 @@ describe('Test click interactions:', function() { describe('drag interactions', function() { beforeEach(function(done) { - Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(done); + Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() { + setTimeout(done, 200); + }); }); it('on nw dragbox should update the axis ranges', function(done) { From 03cad13084ec05a873be76d38f90d5f8a1168d1f Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 27 Apr 2016 09:45:18 +0100 Subject: [PATCH 10/11] Revert "Pause before starting the drag-interaction tests" This reverts commit 9032cb78b948641a5ef484bb22a02643d4f08a57. --- test/jasmine/tests/click_test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/jasmine/tests/click_test.js b/test/jasmine/tests/click_test.js index 6d1aaa26e40..34bf0d71f24 100644 --- a/test/jasmine/tests/click_test.js +++ b/test/jasmine/tests/click_test.js @@ -180,9 +180,7 @@ describe('Test click interactions:', function() { describe('drag interactions', function() { beforeEach(function(done) { - Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() { - setTimeout(done, 200); - }); + Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(done); }); it('on nw dragbox should update the axis ranges', function(done) { From 90700da4429496fe6b6ddde1dafcb42202ac6c46 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 27 Apr 2016 13:33:52 +0100 Subject: [PATCH 11/11] Fix failure in `click_test.js` * In the drag interactions tests, make sure the notifier doesn't hide the drag elements. --- test/jasmine/tests/click_test.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/click_test.js b/test/jasmine/tests/click_test.js index 34bf0d71f24..4e0d5795d8f 100644 --- a/test/jasmine/tests/click_test.js +++ b/test/jasmine/tests/click_test.js @@ -180,7 +180,13 @@ describe('Test click interactions:', function() { describe('drag interactions', function() { beforeEach(function(done) { - Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(done); + Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() { + // Do not let the notifier hide the drag elements + var tooltip = document.querySelector('.notifier-note'); + if(tooltip) tooltip.style.display = 'None'; + + done(); + }); }); it('on nw dragbox should update the axis ranges', function(done) {