From c4f1edc3570a4505591f63685a1d1a68648dd97f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 20 Jul 2018 17:18:45 -0400 Subject: [PATCH 1/6] fixup doTicks docstring --- src/plots/cartesian/axes.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 38d56e912ca..ce7ad408695 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1500,7 +1500,8 @@ axes.makeClipPaths = function(gd) { }); }; -/** Main multi-axis drawing routine! +/** + * Main multi-axis drawing routine! * * @param {DOM element} gd : graph div * @param {string or array of strings} arg : polymorphic argument @@ -1556,14 +1557,15 @@ axes.doTicks = function(gd, arg, skipTitle) { })); }; -/** Per axis drawing routine! +/** + * Per-axis drawing routine! * * This routine draws axis ticks and much more (... grids, labels, title etc.) * Supports multiple argument signatures. * N.B. this thing is async in general (because of MathJax rendering) * * @param {DOM element} gd : graph div - * @param {string or array of strings} arg : polymorphic argument + * @param {string or object} arg : polymorphic argument * @param {boolean} skipTitle : optional flag to skip axis title draw/update * @return {promise} * From a4d3597a491dcca8c0aa48cd4f8a04c08cbb3905 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 20 Jul 2018 17:20:57 -0400 Subject: [PATCH 2/6] rm old hack that got selection gl2d subplots working - this was only used for scattergl and now scattergl is plotted on 'cartesian' subplots. --- src/plots/cartesian/graph_interact.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index f872ddef16e..2ed10621c1c 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -27,7 +27,7 @@ exports.initInteractions = function initInteractions(gd) { return; } - if(!fullLayout._has('cartesian') && !fullLayout._has('gl2d') && !fullLayout._has('splom')) return; + if(!fullLayout._has('cartesian') && !fullLayout._has('splom')) return; var subplots = Object.keys(fullLayout._plots || {}).sort(function(a, b) { // sort overlays last, then by x axis number, then y axis number From adc85144ff5b67f16e660a2e283b2eb3a1db1e2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 20 Jul 2018 17:25:02 -0400 Subject: [PATCH 3/6] include info about things that reset subplot layers in d3-data - to clean what need to be cleaned up in exit selection, instead of using granular and brittle old vs new plotinfo logic - axis.layer, overlays are currently include, we might want to include more things in it eventually, --- src/plots/cartesian/index.js | 60 +++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 0ccbc57a4c4..114a9172757 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -358,7 +358,7 @@ exports.drawFramework = function(gd) { var subplotData = makeSubplotData(gd); var subplotLayers = fullLayout._cartesianlayer.selectAll('.subplot') - .data(subplotData, Lib.identity); + .data(subplotData, String); subplotLayers.enter().append('g') .attr('class', function(name) { return 'subplot ' + name; }); @@ -371,19 +371,9 @@ exports.drawFramework = function(gd) { subplotLayers.each(function(name) { var plotinfo = fullLayout._plots[name]; - // keep ref to plot group plotinfo.plotgroup = d3.select(this); - - // initialize list of overlay subplots - plotinfo.overlays = []; - makeSubplotLayer(gd, plotinfo); - // fill in list of overlay subplots - if(plotinfo.mainplot) { - var mainplot = fullLayout._plots[plotinfo.mainplot]; - mainplot.overlays.push(plotinfo); - } // make separate drag layers for each subplot, // but append them to paper rather than the plot groups, @@ -400,27 +390,49 @@ exports.rangePlot = function(gd, plotinfo, cdSubplot) { function makeSubplotData(gd) { var fullLayout = gd._fullLayout; - var subplotData = []; - var overlays = []; - - for(var k in fullLayout._plots) { - var plotinfo = fullLayout._plots[k]; - var xa2 = plotinfo.xaxis._mainAxis; - var ya2 = plotinfo.yaxis._mainAxis; + var ids = fullLayout._subplots.cartesian; + var len = ids.length; + var subplotData = new Array(len); + var i, j, id, plotinfo, xa, ya; + + for(i = 0; i < len; i++) { + id = ids[i]; + plotinfo = fullLayout._plots[id]; + xa = plotinfo.xaxis; + ya = plotinfo.yaxis; + + var xa2 = xa._mainAxis; + var ya2 = ya._mainAxis; var mainplot = xa2._id + ya2._id; + var mainplotinfo = fullLayout._plots[mainplot]; + plotinfo.overlays = []; - if(mainplot !== k && fullLayout._plots[mainplot]) { + if(mainplot !== id && mainplotinfo) { + // link 'main plot' ref in overlaying plotinfo plotinfo.mainplot = mainplot; - plotinfo.mainplotinfo = fullLayout._plots[mainplot]; - overlays.push(k); + plotinfo.mainplotinfo = mainplotinfo; + // fill in list of overlaying subplots in 'main plot' + mainplotinfo.overlays.push(plotinfo); } else { - subplotData.push(k); plotinfo.mainplot = undefined; + plotinfo.mainPlotinfo = undefined; } } - // main subplots before overlays - subplotData = subplotData.concat(overlays); + // use info about axis layer and overlaying pattern + // to clean what need to be cleaned up in exit selection + for(i = 0; i < len; i++) { + id = ids[i]; + plotinfo = fullLayout._plots[id]; + xa = plotinfo.xaxis; + ya = plotinfo.yaxis; + + var d = [id, xa.layer, ya.layer, xa.overlaying || '', ya.overlaying || '']; + for(j = 0; j < plotinfo.overlays.length; j++) { + d.push(plotinfo.overlays[j].id); + } + subplotData[i] = d; + } return subplotData; } From 66c75db103117975e258e4d1e7a758352c623732 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 20 Jul 2018 17:26:02 -0400 Subject: [PATCH 4/6] adapt selectAll('g.subplot').each(d) data structure - now 'd' is an array, and the subplot id is in d[0]. --- src/plot_api/subroutines.js | 6 ++++-- src/plots/cartesian/axes.js | 5 +++-- src/plots/cartesian/index.js | 22 ++++++++++++---------- test/jasmine/tests/splom_test.js | 2 +- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index d45aa6683d9..a054b6173ad 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -118,7 +118,8 @@ function lsInner(gd) { // to put them var lowerBackgroundIDs = []; var lowerDomains = []; - subplotSelection.each(function(subplot) { + subplotSelection.each(function(d) { + var subplot = d[0]; var plotinfo = fullLayout._plots[subplot]; if(plotinfo.mainplot) { @@ -161,7 +162,8 @@ function lsInner(gd) { fullLayout._plots[subplot].bg = d3.select(this); }); - subplotSelection.each(function(subplot) { + subplotSelection.each(function(d) { + var subplot = d[0]; var plotinfo = fullLayout._plots[subplot]; var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index ce7ad408695..e47f23b97ac 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1526,8 +1526,9 @@ axes.doTicks = function(gd, arg, skipTitle) { var fullLayout = gd._fullLayout; if(arg === 'redraw') { - fullLayout._paper.selectAll('g.subplot').each(function(subplot) { - var plotinfo = fullLayout._plots[subplot]; + fullLayout._paper.selectAll('g.subplot').each(function(d) { + var id = d[0]; + var plotinfo = fullLayout._plots[id]; var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 114a9172757..6c05c3d3bcf 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -361,24 +361,24 @@ exports.drawFramework = function(gd) { .data(subplotData, String); subplotLayers.enter().append('g') - .attr('class', function(name) { return 'subplot ' + name; }); + .attr('class', function(d) { return 'subplot ' + d[0]; }); subplotLayers.order(); subplotLayers.exit() .call(purgeSubplotLayers, fullLayout); - subplotLayers.each(function(name) { - var plotinfo = fullLayout._plots[name]; + subplotLayers.each(function(d) { + var id = d[0]; + var plotinfo = fullLayout._plots[id]; plotinfo.plotgroup = d3.select(this); makeSubplotLayer(gd, plotinfo); - // make separate drag layers for each subplot, // but append them to paper rather than the plot groups, // so they end up on top of the rest - plotinfo.draglayer = ensureSingle(fullLayout._draggers, 'g', name); + plotinfo.draglayer = ensureSingle(fullLayout._draggers, 'g', id); }); }; @@ -529,7 +529,9 @@ function makeSubplotLayer(gd, plotinfo) { if(!hasOnlyLargeSploms) { ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.xaxis._id); ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.yaxis._id); - plotinfo.gridlayer.selectAll('g').sort(axisIds.idSort); + plotinfo.gridlayer.selectAll('g') + .map(function(d) { return d[0]; }) + .sort(axisIds.idSort); } plotinfo.xlines @@ -546,13 +548,13 @@ function purgeSubplotLayers(layers, fullLayout) { var overlayIdsToRemove = {}; - layers.each(function(subplotId) { + layers.each(function(d) { + var id = d[0]; var plotgroup = d3.select(this); plotgroup.remove(); - removeSubplotExtras(subplotId, fullLayout); - - overlayIdsToRemove[subplotId] = true; + removeSubplotExtras(id, fullLayout); + overlayIdsToRemove[id] = true; // do not remove individual axis s here // as other subplots may need them diff --git a/test/jasmine/tests/splom_test.js b/test/jasmine/tests/splom_test.js index e894ab7fe88..2b29fa6d5cc 100644 --- a/test/jasmine/tests/splom_test.js +++ b/test/jasmine/tests/splom_test.js @@ -457,7 +457,7 @@ describe('@gl Test splom interactions:', function() { subplots.each(function(d, i) { var actual = this.children.length; var expected = typeof exp.innerSubplotNodeCnt === 'function' ? - exp.innerSubplotNodeCnt(d, i) : + exp.innerSubplotNodeCnt(d[0], i) : exp.innerSubplotNodeCnt; if(actual !== expected) { failedSubplots.push([d, actual, 'vs', expected].join(' ')); From efc9fafcdc09f37bdf3805ff658f12e99c6319e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 20 Jul 2018 17:27:13 -0400 Subject: [PATCH 5/6] rm now obsolete 'clean axis layer' code in Plots.linkSubplots - this should never have been part of Plots.supplyDefaults pipeline. --- src/plots/plots.js | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 15fddfc844c..fdc08e0bb1b 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -757,6 +757,8 @@ plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayou }; plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { + var i, j; + var oldSubplots = oldFullLayout._plots || {}; var newSubplots = newFullLayout._plots = {}; var newSubplotList = newFullLayout._subplots; @@ -768,32 +770,22 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa var ids = newSubplotList.cartesian.concat(newSubplotList.gl2d || []); - var i, j, id, ax; - for(i = 0; i < ids.length; i++) { - id = ids[i]; + var id = ids[i]; var oldSubplot = oldSubplots[id]; var xaxis = axisIDs.getFromId(mockGd, id, 'x'); var yaxis = axisIDs.getFromId(mockGd, id, 'y'); var plotinfo; + // link or create subplot object if(oldSubplot) { plotinfo = newSubplots[id] = oldSubplot; - - if(plotinfo.xaxis.layer !== xaxis.layer) { - plotinfo.xlines.attr('d', null); - plotinfo.xaxislayer.selectAll('*').remove(); - } - - if(plotinfo.yaxis.layer !== yaxis.layer) { - plotinfo.ylines.attr('d', null); - plotinfo.yaxislayer.selectAll('*').remove(); - } } else { plotinfo = newSubplots[id] = {}; plotinfo.id = id; } + // update x and y axis layout object refs plotinfo.xaxis = xaxis; plotinfo.yaxis = yaxis; @@ -821,7 +813,7 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa // anchored axes to the axes they're anchored to var axList = axisIDs.list(mockGd, null, true); for(i = 0; i < axList.length; i++) { - ax = axList[i]; + var ax = axList[i]; var mainAx = null; if(ax.overlaying) { From 5f2f7ce292ebb85cd953820cbecc1e115119fe4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 20 Jul 2018 17:27:41 -0400 Subject: [PATCH 6/6] add test :lock:ing overlaying axis configuration updates --- test/jasmine/tests/cartesian_test.js | 82 ++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/test/jasmine/tests/cartesian_test.js b/test/jasmine/tests/cartesian_test.js index 3cb326a940f..1eed4bd0529 100644 --- a/test/jasmine/tests/cartesian_test.js +++ b/test/jasmine/tests/cartesian_test.js @@ -659,6 +659,88 @@ describe('subplot creation / deletion:', function() { .then(done); }); + it('should clear obsolete content out of axis layers when changing overlaying configuation', function(done) { + function data() { + return [ + {x: [1, 2], y: [1, 2]}, + {x: [1, 2], y: [1, 2], xaxis: 'x2', yaxis: 'y2'} + ]; + } + + function fig0() { + return { + data: data(), + layout: { + xaxis2: {side: 'top', overlaying: 'x'}, + yaxis2: {side: 'right', overlaying: 'y'} + } + }; + } + + function fig1() { + return { + data: data(), + layout: { + xaxis2: {side: 'top', domain: [0.37, 1]}, + yaxis2: {side: 'right', overlaying: 'y'} + } + }; + } + + function getParentClassName(query, level) { + level = level || 1; + var cl = gd.querySelector('g.cartesianlayer'); + var node = cl.querySelector(query); + while(level--) node = node.parentNode; + return node.getAttribute('class'); + } + + function _assert(msg, exp) { + expect(getParentClassName('.xtick')) + .toBe(exp.xtickParent, 'xitck parent - ' + msg); + expect(getParentClassName('.x2tick')) + .toBe(exp.x2tickParent, 'x2tick parent - ' + msg); + expect(getParentClassName('.trace' + gd._fullData[0].uid, 2)) + .toBe(exp.trace0Parent, 'data[0] parent - ' + msg); + expect(getParentClassName('.trace' + gd._fullData[1].uid, 2)) + .toBe(exp.trace1Parent, 'data[1] parent - ' + msg); + } + + Plotly.react(gd, fig0()) + .then(function() { + _assert('x2/y2 both overlays', { + xtickParent: 'xaxislayer-above', + x2tickParent: 'x2y2-x', + trace0Parent: 'plot', + trace1Parent: 'x2y2' + }); + }) + .then(function() { + return Plotly.react(gd, fig1()); + }) + .then(function() { + _assert('x2 free / y2 overlaid', { + xtickParent: 'xaxislayer-above', + x2tickParent: 'xaxislayer-above', + trace0Parent: 'plot', + trace1Parent: 'plot' + }); + }) + .then(function() { + return Plotly.react(gd, fig0()); + }) + .then(function() { + _assert('back to x2/y2 both overlays', { + xtickParent: 'xaxislayer-above', + x2tickParent: 'x2y2-x', + trace0Parent: 'plot', + trace1Parent: 'x2y2' + }); + }) + .catch(failTest) + .then(done); + }); + it('clear axis ticks, labels and title when relayout an axis to `*visible:false*', function(done) { function _assert(xaxis, yaxis) { var g = d3.select('.subplot.xy');