From 0d3fe541e05ca3f6c5b905ebfabe56fc0f197313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 8 Jul 2016 14:23:47 -0400 Subject: [PATCH 1/6] lint: flattern Fx test suite --- test/jasmine/tests/fx_test.js | 96 +++++++++++++++++------------------ 1 file changed, 47 insertions(+), 49 deletions(-) diff --git a/test/jasmine/tests/fx_test.js b/test/jasmine/tests/fx_test.js index 59ac3a146fc..e7572b8ad83 100644 --- a/test/jasmine/tests/fx_test.js +++ b/test/jasmine/tests/fx_test.js @@ -2,68 +2,66 @@ var Fx = require('@src/plots/cartesian/graph_interact'); var Plots = require('@src/plots/plots'); -describe('Test FX', function() { +describe('FX defaults', function() { 'use strict'; - describe('defaults', function() { + var layoutIn, layoutOut, fullData; - var layoutIn, layoutOut, fullData; - - beforeEach(function() { - layoutIn = {}; - layoutOut = { - _has: Plots._hasPlotType - }; - fullData = [{}]; - }); + beforeEach(function() { + layoutIn = {}; + layoutOut = { + _has: Plots._hasPlotType + }; + fullData = [{}]; + }); - it('should default (blank version)', function() { - Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut.hovermode).toBe('closest', 'hovermode to closest'); - expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); - }); + it('should default (blank version)', function() { + Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); + expect(layoutOut.hovermode).toBe('closest', 'hovermode to closest'); + expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); + }); - it('should default (cartesian version)', function() { - layoutOut._basePlotModules = [{ name: 'cartesian' }]; + it('should default (cartesian version)', function() { + layoutOut._basePlotModules = [{ name: 'cartesian' }]; - Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut.hovermode).toBe('x', 'hovermode to x'); - expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); - expect(layoutOut._isHoriz).toBe(false, 'isHoriz to false'); - }); + Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); + expect(layoutOut.hovermode).toBe('x', 'hovermode to x'); + expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); + expect(layoutOut._isHoriz).toBe(false, 'isHoriz to false'); + }); - it('should default (cartesian horizontal version)', function() { - layoutOut._basePlotModules = [{ name: 'cartesian' }]; - fullData[0] = { orientation: 'h' }; + it('should default (cartesian horizontal version)', function() { + layoutOut._basePlotModules = [{ name: 'cartesian' }]; + fullData[0] = { orientation: 'h' }; - Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut.hovermode).toBe('y', 'hovermode to y'); - expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); - expect(layoutOut._isHoriz).toBe(true, 'isHoriz to true'); - }); + Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); + expect(layoutOut.hovermode).toBe('y', 'hovermode to y'); + expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); + expect(layoutOut._isHoriz).toBe(true, 'isHoriz to true'); + }); - it('should default (gl3d version)', function() { - layoutOut._basePlotModules = [{ name: 'gl3d' }]; + it('should default (gl3d version)', function() { + layoutOut._basePlotModules = [{ name: 'gl3d' }]; - Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut.hovermode).toBe('closest', 'hovermode to closest'); - expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); - }); + Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); + expect(layoutOut.hovermode).toBe('closest', 'hovermode to closest'); + expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); + }); - it('should default (geo version)', function() { - layoutOut._basePlotModules = [{ name: 'geo' }]; + it('should default (geo version)', function() { + layoutOut._basePlotModules = [{ name: 'geo' }]; - Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut.hovermode).toBe('closest', 'hovermode to closest'); - expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); - }); + Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); + expect(layoutOut.hovermode).toBe('closest', 'hovermode to closest'); + expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); + }); - it('should default (multi plot type version)', function() { - layoutOut._basePlotModules = [{ name: 'cartesian' }, { name: 'gl3d' }]; + it('should default (multi plot type version)', function() { + layoutOut._basePlotModules = [{ name: 'cartesian' }, { name: 'gl3d' }]; - Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut.hovermode).toBe('x', 'hovermode to x'); - expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); - }); + Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData); + expect(layoutOut.hovermode).toBe('x', 'hovermode to x'); + expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); }); }); +}); From fdb0c3cc8a48d84ab1b2dbc87d6e8838c9f33e4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 8 Jul 2016 15:38:09 -0400 Subject: [PATCH 2/6] update 'dragmode' cursor in relayout NOT modebar --- src/components/modebar/buttons.js | 21 +-------------------- src/plot_api/plot_api.js | 3 +++ 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/src/components/modebar/buttons.js b/src/components/modebar/buttons.js index 9635b30cc37..23b6ae8a72c 100644 --- a/src/components/modebar/buttons.js +++ b/src/components/modebar/buttons.js @@ -11,7 +11,6 @@ var Plotly = require('../../plotly'); var Lib = require('../../lib'); -var setCursor = require('../../lib/setcursor'); var downloadImage = require('../../snapshot/download'); var Icons = require('../../../build/ploticon'); @@ -171,13 +170,6 @@ modeBarButtons.hoverCompareCartesian = { click: handleCartesian }; -var DRAGCURSORS = { - pan: 'move', - zoom: 'crosshair', - select: 'crosshair', - lasso: 'crosshair' -}; - function handleCartesian(gd, ev) { var button = ev.currentTarget, astr = button.getAttribute('data-attr'), @@ -227,18 +219,7 @@ function handleCartesian(gd, ev) { aobj[astr] = val; } - Plotly.relayout(gd, aobj).then(function() { - if(astr === 'dragmode') { - if(fullLayout._has('cartesian')) { - setCursor( - fullLayout._paper.select('.nsewdrag'), - DRAGCURSORS[val] - ); - } - Plotly.Fx.supplyLayoutDefaults(gd.layout, fullLayout, gd._fullData); - Plotly.Fx.init(gd); - } - }); + Plotly.relayout(gd, aobj); } modeBarButtons.zoom3d = { diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 8b3cd559c4b..2dfbd3bf1dd 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2429,6 +2429,9 @@ Plotly.relayout = function relayout(gd, astr, val) { var subplotIds; manageModeBar(gd); + Plotly.Fx.supplyLayoutDefaults(gd.layout, fullLayout, gd._fullData); + Plotly.Fx.init(gd); + subplotIds = Plots.getSubplotIds(fullLayout, 'gl3d'); for(i = 0; i < subplotIds.length; i++) { scene = fullLayout[subplotIds[i]]._scene; From 5f80b6e5618067aac4f6873a0a0063d4b849815e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 8 Jul 2016 15:39:00 -0400 Subject: [PATCH 3/6] dry: check for if dragbox is main-drag only once --- src/plots/cartesian/dragbox.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 508fa6a2d51..17a98bd7d9d 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -52,7 +52,8 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { pw = xa[0]._length, ph = ya[0]._length, MINDRAG = constants.MINDRAG, - MINZOOM = constants.MINZOOM; + MINZOOM = constants.MINZOOM, + isMainDrag = (ns + ew === 'nsew'); for(var i = 1; i < subplots.length; i++) { var subplotXa = subplots[i].x(), @@ -89,7 +90,7 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { // and stop there if(!yActive && !xActive) { dragger.onmousedown = null; - dragger.style.pointerEvents = (ns + ew === 'nsew') ? 'all' : 'none'; + dragger.style.pointerEvents = isMainDrag ? 'all' : 'none'; return dragger; } @@ -107,7 +108,8 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { doubleclick: doubleClick, prepFn: function(e, startX, startY) { var dragModeNow = gd._fullLayout.dragmode; - if(ns + ew === 'nsew') { + + if(isMainDrag) { // main dragger handles all drag modes, and changes // to pan (or to zoom if it already is pan) on shift if(e.shiftKey) { From 57d69804bc89934d4e23440219bed88a40100d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 8 Jul 2016 15:39:18 -0400 Subject: [PATCH 4/6] fixup flatten --- test/jasmine/tests/fx_test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/fx_test.js b/test/jasmine/tests/fx_test.js index e7572b8ad83..31cc07e0ed1 100644 --- a/test/jasmine/tests/fx_test.js +++ b/test/jasmine/tests/fx_test.js @@ -2,7 +2,8 @@ var Fx = require('@src/plots/cartesian/graph_interact'); var Plots = require('@src/plots/plots'); -describe('FX defaults', function() { + +describe('Fx defaults', function() { 'use strict'; var layoutIn, layoutOut, fullData; From f0110eeb2d480bba41b5fb883d10ca373d188aae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 8 Jul 2016 15:40:31 -0400 Subject: [PATCH 5/6] fx: make mainDrag active if 'lasso' / 'select' even w/ fixed ranges --- src/plots/cartesian/dragbox.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 17a98bd7d9d..ff6f7bc52bf 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -76,19 +76,22 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { dragClass = ns + ew + 'drag'; var dragger3 = plotinfo.draglayer.selectAll('.' + dragClass).data([0]); + dragger3.enter().append('rect') .classed('drag', true) .classed(dragClass, true) .style({fill: 'transparent', 'stroke-width': 0}) .attr('data-subplot', plotinfo.id); + dragger3.call(Drawing.setRect, x, y, w, h) .call(setCursor, cursor); + var dragger = dragger3.node(); // still need to make the element if the axes are disabled // but nuke its events (except for maindrag which needs them for hover) // and stop there - if(!yActive && !xActive) { + if(!yActive && !xActive && !isSelectOrLasso(fullLayout.dragmode)) { dragger.onmousedown = null; dragger.style.pointerEvents = isMainDrag ? 'all' : 'none'; return dragger; @@ -133,7 +136,7 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { dragOptions.doneFn = dragDone; clearSelect(); } - else if(dragModeNow === 'select' || dragModeNow === 'lasso') { + else if(isSelectOrLasso(dragModeNow)) { prepSelect(e, startX, startY, dragOptions, dragModeNow); } } @@ -670,3 +673,9 @@ function removeZoombox(gd) { .selectAll('.zoombox,.js-zoombox-backdrop,.js-zoombox-menu,.zoombox-corners') .remove(); } + +function isSelectOrLasso(dragmode) { + var modes = ['lasso', 'select']; + + return modes.indexOf(dragmode) !== -1; +} From 7f9bc9c2d18afe31552089b743e8c11706b98b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 8 Jul 2016 15:40:49 -0400 Subject: [PATCH 6/6] test: check main drag update on relayout --- test/jasmine/tests/fx_test.js | 69 ++++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/fx_test.js b/test/jasmine/tests/fx_test.js index 31cc07e0ed1..4a89b547472 100644 --- a/test/jasmine/tests/fx_test.js +++ b/test/jasmine/tests/fx_test.js @@ -1,6 +1,11 @@ -var Fx = require('@src/plots/cartesian/graph_interact'); +var Plotly = require('@lib/index'); var Plots = require('@src/plots/plots'); +var Fx = require('@src/plots/cartesian/graph_interact'); + +var d3 = require('d3'); +var createGraphDiv = require('../assets/create_graph_div'); +var destroyGraphDiv = require('../assets/destroy_graph_div'); describe('Fx defaults', function() { @@ -65,4 +70,66 @@ describe('Fx defaults', function() { expect(layoutOut.dragmode).toBe('zoom', 'dragmode to zoom'); }); }); + +describe('relayout', function() { + 'use strict'; + + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + it('should update main drag with correct', function(done) { + + function assertMainDrag(cursor, isActive) { + expect(d3.selectAll('rect.nsewdrag').size()).toEqual(1, 'number of nodes'); + var mainDrag = d3.select('rect.nsewdrag'), + node = mainDrag.node(); + + expect(mainDrag.classed('cursor-' + cursor)).toBe(true, 'cursor ' + cursor); + expect(mainDrag.style('pointer-events')).toEqual('all', 'pointer event'); + expect(!!node.onmousedown).toBe(isActive, 'mousedown handler'); + } + + Plotly.plot(gd, [{ + y: [2, 1, 2] + }]).then(function() { + assertMainDrag('crosshair', true); + + return Plotly.relayout(gd, 'dragmode', 'pan'); + }).then(function() { + assertMainDrag('move', true); + + return Plotly.relayout(gd, 'dragmode', 'drag'); + }).then(function() { + assertMainDrag('crosshair', true); + + return Plotly.relayout(gd, 'xaxis.fixedrange', true); + }).then(function() { + assertMainDrag('ns-resize', true); + + return Plotly.relayout(gd, 'yaxis.fixedrange', true); + }).then(function() { + assertMainDrag('pointer', false); + + return Plotly.relayout(gd, 'dragmode', 'drag'); + }).then(function() { + assertMainDrag('pointer', false); + + return Plotly.relayout(gd, 'dragmode', 'lasso'); + }).then(function() { + assertMainDrag('pointer', true); + + return Plotly.relayout(gd, 'dragmode', 'select'); + }).then(function() { + assertMainDrag('pointer', true); + + return Plotly.relayout(gd, 'xaxis.fixedrange', false); + }).then(function() { + assertMainDrag('ew-resize', true); + }).then(done); + }); });