Merge pull request #2831 from plotly/subplot-overlaying-toggle-relayouts

etpinard · web-flow · commit dcaebc289b00 · 2018-07-23T09:44:23.000-04:00
Fix overlaying subplot configuration relayouts
diff --git 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
@@ -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
@@ -1525,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;
 
@@ -1556,14 +1558,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}
  *
diff --git 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
diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js
@@ -358,37 +358,27 @@ 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; });
+        .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];
 
-        // 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,
         // so they end up on top of the rest
-        plotinfo.draglayer = ensureSingle(fullLayout._draggers, 'g', name);
+        plotinfo.draglayer = ensureSingle(fullLayout._draggers, 'g', id);
     });
 };
 
@@ -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;
 }
@@ -517,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
@@ -534,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 <clipPath>s here
         // as other subplots may need them
diff --git 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) {
diff --git 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');
diff --git 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(' '));

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ exports.initInteractions = function initInteractions(gd) {`
`27`	`27`	`return;`
`28`	`28`	`}`
`29`	`29`
`30`		`- if(!fullLayout._has('cartesian') && !fullLayout._has('gl2d') && !fullLayout._has('splom')) return;`
	`30`	`+ if(!fullLayout._has('cartesian') && !fullLayout._has('splom')) return;`
`31`	`31`
`32`	`32`	`var subplots = Object.keys(fullLayout._plots \|\| {}).sort(function(a, b) {`
`33`	`33`	`// sort overlays last, then by x axis number, then y axis number`