From fb85e65dcfa316874b2876cb11bd959ef9bbfaf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 3 Jan 2019 18:00:34 -0500 Subject: [PATCH 1/4] fix uirevision behavior on gl3d subplots --- src/plots/gl3d/scene.js | 12 +++-- test/jasmine/tests/plot_api_react_test.js | 65 +++++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/plots/gl3d/scene.js b/src/plots/gl3d/scene.js index fac8e904ebe..bb3dea88c8c 100644 --- a/src/plots/gl3d/scene.js +++ b/src/plots/gl3d/scene.js @@ -702,6 +702,7 @@ proto.setCamera = function setCamera(cameraData) { // save camera to user layout (i.e. gd.layout) proto.saveCamera = function saveCamera(layout) { + var fullLayout = this.fullLayout; var cameraData = this.getCamera(); var cameraNestedProp = Lib.nestedProperty(layout, this.id + '.camera'); var cameraDataLastSave = cameraNestedProp.get(); @@ -713,8 +714,9 @@ proto.saveCamera = function saveCamera(layout) { return y[vectors[i]] && (x[vectors[i]][components[j]] === y[vectors[i]][components[j]]); } - if(cameraDataLastSave === undefined) hasChanged = true; - else { + if(cameraDataLastSave === undefined) { + hasChanged = true; + } else { for(var i = 0; i < 3; i++) { for(var j = 0; j < 3; j++) { if(!same(cameraData, cameraDataLastSave, i, j)) { @@ -726,12 +728,14 @@ proto.saveCamera = function saveCamera(layout) { } if(hasChanged) { + var preGUI = {}; + preGUI[this.id + '.camera'] = cameraDataLastSave; + Registry.call('_storeDirectGUIEdit', layout, fullLayout._preGUI, preGUI); + cameraNestedProp.set(cameraData); - var fullLayout = this.fullLayout; var cameraFullNP = Lib.nestedProperty(fullLayout, this.id + '.camera'); cameraFullNP.set(cameraData); - Registry.call('_storeDirectGUIEdit', layout, fullLayout._preGUI, cameraData); } return hasChanged; diff --git a/test/jasmine/tests/plot_api_react_test.js b/test/jasmine/tests/plot_api_react_test.js index 4dbf48ddc8a..0a32be329c2 100644 --- a/test/jasmine/tests/plot_api_react_test.js +++ b/test/jasmine/tests/plot_api_react_test.js @@ -13,6 +13,7 @@ var destroyGraphDiv = require('../assets/destroy_graph_div'); var failTest = require('../assets/fail_test'); var supplyAllDefaults = require('../assets/supply_defaults'); var mockLists = require('../assets/mock_lists'); +var mouseEvent = require('../assets/mouse_event'); var drag = require('../assets/drag'); var MAPBOX_ACCESS_TOKEN = require('@build/credentials.json').MAPBOX_ACCESS_TOKEN; @@ -1806,3 +1807,67 @@ describe('Plotly.react and uirevision attributes', function() { _run(fig, editComponents, checkInitial, checkEdited).then(done); }); }); + +describe('Test Plotly.react + interactions under uirevision:', function() { + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(function() { + Plotly.purge(gd); + destroyGraphDiv(); + }); + + it('@gl gl3d subplots preserve camera changes on interactions', function(done) { + function _react() { + return Plotly.react(gd, [{ + type: 'surface', + z: [[1, 2, 3], [3, 1, 2], [2, 3, 1]] + }], { + width: 500, + height: 500, + uirevision: true + }); + } + + // mocking panning/scrolling is brittle, + // this here is enough to to trigger the relayoutCallback + function _mouseup() { + var target = gd.querySelector('.svg-container .gl-container #scene canvas'); + return new Promise(function(resolve) { + mouseEvent('mouseup', 200, 200, {element: target}); + setTimeout(resolve, 0); + }); + } + + // should be same before & after 2nd react() + function _assertGUI(msg) { + var preGUI = gd._fullLayout._preGUI; + expect(preGUI['scene.camera']).toBe(null, msg); + } + + _react() + .then(function() { + expect(gd.layout.scene).toEqual(jasmine.objectContaining({ + aspectratio: {x: 1, y: 1, z: 1}, + aspectmode: 'auto' + })); + expect(gd.layout.scene.camera).toBe(undefined); + + var fullEye = gd._fullLayout.scene.camera.eye; + expect(fullEye.x).toBe(1.25); + expect(fullEye.y).toBe(1.25); + expect(fullEye.z).toBe(1.25); + + expect(gd._fullLayout._preGUI).toEqual({}); + }) + .then(function() { return _mouseup(); }) + .then(function() { _assertGUI('before'); }) + .then(_react) + .then(function() { _assertGUI('after'); }) + .catch(failTest) + .then(done); + }); +}); From 81511972bb3a260dc8dc702bee375f019382db13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 3 Jan 2019 18:01:09 -0500 Subject: [PATCH 2/4] fix uirevision behavior on geo subplots --- src/plots/geo/zoom.js | 6 ++- test/jasmine/tests/plot_api_react_test.js | 60 +++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/plots/geo/zoom.js b/src/plots/geo/zoom.js index efe9a91a2de..293ffed0127 100644 --- a/src/plots/geo/zoom.js +++ b/src/plots/geo/zoom.js @@ -53,11 +53,14 @@ function sync(geo, projection, cb) { var fullLayout = gd._fullLayout; var fullOpts = fullLayout[id]; + var preGUI = {}; var eventData = {}; function set(propStr, val) { - var fullNp = Lib.nestedProperty(fullOpts, propStr); + preGUI[id + '.' + propStr] = Lib.nestedProperty(userOpts, propStr).get(); + Registry.call('_storeDirectGUIEdit', layout, fullLayout._preGUI, preGUI); + var fullNp = Lib.nestedProperty(fullOpts, propStr); if(fullNp.get() !== val) { fullNp.set(val); Lib.nestedProperty(userOpts, propStr).set(val); @@ -67,7 +70,6 @@ function sync(geo, projection, cb) { cb(set); set('projection.scale', projection.scale() / geo.fitScale); - Registry.call('_storeDirectGUIEdit', layout, fullLayout._preGUI, eventData); gd.emit('plotly_relayout', eventData); } diff --git a/test/jasmine/tests/plot_api_react_test.js b/test/jasmine/tests/plot_api_react_test.js index 0a32be329c2..f514199eb76 100644 --- a/test/jasmine/tests/plot_api_react_test.js +++ b/test/jasmine/tests/plot_api_react_test.js @@ -1870,4 +1870,64 @@ describe('Test Plotly.react + interactions under uirevision:', function() { .catch(failTest) .then(done); }); + + it('geo subplots should preserve viewport changes after panning', function(done) { + function _react() { + return Plotly.react(gd, [{ + type: 'scattergeo', + lon: [3, 1, 2], + lat: [2, 3, 1] + }], { + width: 500, + height: 500, + uirevision: true + }); + } + + function _drag(x0, y0, dx, dy) { + mouseEvent('mousemove', x0, y0); + mouseEvent('mousedown', x0, y0); + mouseEvent('mousemove', x0 + dx, y0 + dy); + mouseEvent('mouseup', x0 + dx, y0 + dy); + } + + // should be same before & after 2nd react() + function _assertGUI(msg) { + var TOL = 2; + + var geo = gd.layout.geo || {}; + expect(((geo.projection || {}).rotation || {}).lon).toBeCloseTo(-52.94, TOL, msg); + expect((geo.center || {}).lon).toBeCloseTo(-52.94, TOL, msg); + expect((geo.center || {}).lat).toBeCloseTo(52.94, TOL, msg); + + var fullGeo = gd._fullLayout.geo; + expect(fullGeo.projection.rotation.lon).toBeCloseTo(-52.94, TOL, msg); + expect(fullGeo.center.lon).toBeCloseTo(-52.94, TOL, msg); + expect(fullGeo.center.lat).toBeCloseTo(52.94, TOL, msg); + + var preGUI = gd._fullLayout._preGUI; + expect(preGUI['geo.projection.rotation.lon']).toBe(null, msg); + expect(preGUI['geo.center.lon']).toBe(null, msg); + expect(preGUI['geo.center.lat']).toBe(null, msg); + expect(preGUI['geo.projection.scale']).toBe(null, msg); + } + + _react() + .then(function() { + expect(gd.layout.geo).toEqual({}); + + var fullGeo = gd._fullLayout.geo; + expect(fullGeo.projection.rotation.lon).toBe(0); + expect(fullGeo.center.lon).toBe(0); + expect(fullGeo.center.lat).toBe(0); + + expect(gd._fullLayout._preGUI).toEqual({}); + }) + .then(function() { return _drag(200, 200, 50, 50); }) + .then(function() { _assertGUI('before'); }) + .then(_react) + .then(function() { _assertGUI('after'); }) + .catch(failTest) + .then(done); + }); }); From 4b8a05b81fe788442eac01f6c02759a0468d4b0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 3 Jan 2019 18:03:16 -0500 Subject: [PATCH 3/4] fix uirevision behavior on mapbox subplots ... and remove `self.opts` use `self.gd._fullLayout[self.id]` instead, which always points to the correct container even on redraw-less updates --- src/plots/mapbox/mapbox.js | 74 +++++++++++------------ test/jasmine/tests/plot_api_react_test.js | 74 +++++++++++++++++++++++ 2 files changed, 111 insertions(+), 37 deletions(-) diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index 78ae2e4974d..78509140a2c 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -32,9 +32,6 @@ function Mapbox(opts) { // unique id for this Mapbox instance this.uid = fullLayout._uid + '-' + this.id; - // full mapbox options (N.B. needs to be updated on every updates) - this.opts = fullLayout[this.id]; - // create framework on instantiation for a smoother first plot call this.div = null; this.xaxis = null; @@ -57,9 +54,7 @@ module.exports = function createMapbox(opts) { proto.plot = function(calcData, fullLayout, promises) { var self = this; - - // feed in new mapbox options - var opts = self.opts = fullLayout[this.id]; + var opts = fullLayout[self.id]; // remove map and create a new map if access token has change if(self.map && (opts.accesstoken !== self.accessToken)) { @@ -88,7 +83,7 @@ proto.plot = function(calcData, fullLayout, promises) { proto.createMap = function(calcData, fullLayout, resolve, reject) { var self = this; var gd = self.gd; - var opts = self.opts; + var opts = fullLayout[self.id]; // store style id and URL or object var styleObj = self.styleObj = getStyleObj(opts.style); @@ -147,14 +142,16 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { // duplicate 'plotly_relayout' events. if(eventData.originalEvent || wheeling) { - var view = self.getView(); + var optsNow = gd._fullLayout[self.id]; + Registry.call('_storeDirectGUIEdit', gd.layout, gd._fullLayout._preGUI, self.getViewEdits(optsNow)); - opts._input.center = opts.center = view.center; - opts._input.zoom = opts.zoom = view.zoom; - opts._input.bearing = opts.bearing = view.bearing; - opts._input.pitch = opts.pitch = view.pitch; + var viewNow = self.getView(); + optsNow._input.center = optsNow.center = viewNow.center; + optsNow._input.zoom = optsNow.zoom = viewNow.zoom; + optsNow._input.bearing = optsNow.bearing = viewNow.bearing; + optsNow._input.pitch = optsNow.pitch = viewNow.pitch; - emitRelayoutFromView(view); + gd.emit('plotly_relayout', self.getViewEdits(viewNow)); } wheeling = false; }); @@ -186,35 +183,25 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { map.on('zoomstart', unhover); map.on('dblclick', function() { - gd.emit('plotly_doubleclick', null); + var optsNow = gd._fullLayout[self.id]; + Registry.call('_storeDirectGUIEdit', gd.layout, gd._fullLayout._preGUI, self.getViewEdits(optsNow)); var viewInitial = self.viewInitial; - map.setCenter(convertCenter(viewInitial.center)); map.setZoom(viewInitial.zoom); map.setBearing(viewInitial.bearing); map.setPitch(viewInitial.pitch); var viewNow = self.getView(); + optsNow._input.center = optsNow.center = viewNow.center; + optsNow._input.zoom = optsNow.zoom = viewNow.zoom; + optsNow._input.bearing = optsNow.bearing = viewNow.bearing; + optsNow._input.pitch = optsNow.pitch = viewNow.pitch; - opts._input.center = opts.center = viewNow.center; - opts._input.zoom = opts.zoom = viewNow.zoom; - opts._input.bearing = opts.bearing = viewNow.bearing; - opts._input.pitch = opts.pitch = viewNow.pitch; - - emitRelayoutFromView(viewNow); + gd.emit('plotly_doubleclick', null); + gd.emit('plotly_relayout', self.getViewEdits(viewNow)); }); - function emitRelayoutFromView(view) { - var id = self.id; - var evtData = {}; - for(var k in view) { - evtData[id + '.' + k] = view[k]; - } - Registry.call('_storeDirectGUIEdit', gd.layout, gd._fullLayout._preGUI, evtData); - gd.emit('plotly_relayout', evtData); - } - // define event handlers on map creation, to keep one ref per map, // so that map.on / map.off in updateFx works as expected self.clearSelect = function() { @@ -248,10 +235,11 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { proto.updateMap = function(calcData, fullLayout, resolve, reject) { var self = this; var map = self.map; + var opts = fullLayout[this.id]; self.rejectOnError(reject); - var styleObj = getStyleObj(self.opts.style); + var styleObj = getStyleObj(opts.style); if(self.styleObj.id !== styleObj.id) { self.styleObj = styleObj; @@ -309,14 +297,14 @@ proto.updateData = function(calcData) { proto.updateLayout = function(fullLayout) { var map = this.map; - var opts = this.opts; + var opts = fullLayout[this.id]; map.setCenter(convertCenter(opts.center)); map.setZoom(opts.zoom); map.setBearing(opts.bearing); map.setPitch(opts.pitch); - this.updateLayers(); + this.updateLayers(fullLayout); this.updateFramework(fullLayout); this.updateFx(fullLayout); this.map.resize(); @@ -463,8 +451,8 @@ proto.updateFramework = function(fullLayout) { this.yaxis._length = size.h * (domain.y[1] - domain.y[0]); }; -proto.updateLayers = function() { - var opts = this.opts; +proto.updateLayers = function(fullLayout) { + var opts = fullLayout[this.id]; var layers = opts.layers; var layerList = this.layerList; var i; @@ -519,7 +507,6 @@ proto.project = function(v) { // get map's current view values in plotly.js notation proto.getView = function() { var map = this.map; - var mapCenter = map.getCenter(); var center = { lon: mapCenter.lng, lat: mapCenter.lat }; @@ -531,6 +518,19 @@ proto.getView = function() { }; }; +proto.getViewEdits = function(cont) { + var id = this.id; + var keys = ['center', 'zoom', 'bearing', 'pitch']; + var obj = {}; + + for(var i = 0; i < keys.length; i++) { + var k = keys[i]; + obj[id + '.' + k] = cont[k]; + } + + return obj; +}; + function getStyleObj(val) { var styleValues = layoutAttributes.style.values; var styleDflt = layoutAttributes.style.dflt; diff --git a/test/jasmine/tests/plot_api_react_test.js b/test/jasmine/tests/plot_api_react_test.js index f514199eb76..747d410c237 100644 --- a/test/jasmine/tests/plot_api_react_test.js +++ b/test/jasmine/tests/plot_api_react_test.js @@ -1930,4 +1930,78 @@ describe('Test Plotly.react + interactions under uirevision:', function() { .catch(failTest) .then(done); }); + + it('@gl mapbox subplots should preserve viewport changes after panning', function(done) { + Plotly.setPlotConfig({ + mapboxAccessToken: MAPBOX_ACCESS_TOKEN + }); + + function _react() { + return Plotly.react(gd, [{ + type: 'scattermapbox', + lon: [3, 1, 2], + lat: [2, 3, 1] + }], { + width: 500, + height: 500, + uirevision: true + }); + } + + // see mapbox_test.js for rationale + function _mouseEvent(type, pos) { + return new Promise(function(resolve) { + mouseEvent(type, pos[0], pos[1]); + setTimeout(resolve, 100); + }); + } + + // see mapbox_test.js for rationale + function _drag(p0, p1) { + return _mouseEvent('mousemove', p0) + .then(function() { return _mouseEvent('mousedown', p0); }) + .then(function() { return _mouseEvent('mousemove', p1); }) + .then(function() { return _mouseEvent('mousemove', p1); }) + .then(function() { return _mouseEvent('mouseup', p1); }) + .then(function() { return _mouseEvent('mouseup', p1); }); + } + + // should be same before & after 2nd react() + function _assertGUI(msg) { + var TOL = 2; + + var mapbox = gd.layout.mapbox || {}; + expect((mapbox.center || {}).lon).toBeCloseTo(-17.578, TOL, msg); + expect((mapbox.center || {}).lat).toBeCloseTo(17.308, TOL, msg); + expect(mapbox.zoom).toBe(1); + + var fullMapbox = gd._fullLayout.mapbox || {}; + expect(fullMapbox.center.lon).toBeCloseTo(-17.578, TOL, msg); + expect(fullMapbox.center.lat).toBeCloseTo(17.308, TOL, msg); + expect(fullMapbox.zoom).toBe(1); + + var preGUI = gd._fullLayout._preGUI; + expect(preGUI['mapbox.center.lon']).toBe(null, msg); + expect(preGUI['mapbox.center.lat']).toBe(null, msg); + expect(preGUI['mapbox.zoom']).toBe(null, msg); + } + + _react() + .then(function() { + expect(gd.layout.mapbox).toEqual({}); + + var fullMapbox = gd._fullLayout.mapbox; + expect(fullMapbox.center.lon).toBe(0); + expect(fullMapbox.center.lat).toBe(0); + expect(fullMapbox.zoom).toBe(1); + + expect(gd._fullLayout._preGUI).toEqual({}); + }) + .then(function() { return _drag([200, 200], [250, 250]); }) + .then(function() { _assertGUI('before'); }) + .then(_react) + .then(function() { _assertGUI('after'); }) + .catch(failTest) + .then(done); + }); }); From b880717ba8b86602a62d8010a8028695556c0103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 4 Jan 2019 09:30:35 -0500 Subject: [PATCH 4/4] improve gl3d uirevision test --- test/jasmine/tests/plot_api_react_test.js | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/plot_api_react_test.js b/test/jasmine/tests/plot_api_react_test.js index 747d410c237..3ff29357131 100644 --- a/test/jasmine/tests/plot_api_react_test.js +++ b/test/jasmine/tests/plot_api_react_test.js @@ -1832,9 +1832,17 @@ describe('Test Plotly.react + interactions under uirevision:', function() { }); } - // mocking panning/scrolling is brittle, + // mocking panning/scrolling with mouse events is brittle, // this here is enough to to trigger the relayoutCallback function _mouseup() { + var sceneLayout = gd._fullLayout.scene; + var cameraOld = sceneLayout.camera; + sceneLayout._scene.setCamera({ + eye: {x: 2, y: 2, z: 2}, + center: cameraOld.center, + up: cameraOld.up + }); + var target = gd.querySelector('.svg-container .gl-container #scene canvas'); return new Promise(function(resolve) { mouseEvent('mouseup', 200, 200, {element: target}); @@ -1844,6 +1852,18 @@ describe('Test Plotly.react + interactions under uirevision:', function() { // should be same before & after 2nd react() function _assertGUI(msg) { + var TOL = 2; + + var eye = ((gd.layout.scene || {}).camera || {}).eye || {}; + expect(eye.x).toBeCloseTo(2, TOL, msg); + expect(eye.y).toBeCloseTo(2, TOL, msg); + expect(eye.z).toBeCloseTo(2, TOL, msg); + + var fullEye = gd._fullLayout.scene.camera.eye; + expect(fullEye.x).toBeCloseTo(2, TOL, msg); + expect(fullEye.y).toBeCloseTo(2, TOL, msg); + expect(fullEye.z).toBeCloseTo(2, TOL, msg); + var preGUI = gd._fullLayout._preGUI; expect(preGUI['scene.camera']).toBe(null, msg); }