Merge pull request #289 from plotly/delete-last-trace2

A slightly less ambitious take on orphan subplots and 'delete last trace'

Author: etpinard

src/plot_api/plot_api.js

@@ -50,16 +50,13 @@ var xmlnsNamespaces = require('../constants/xmlns_namespaces');
 Plotly.plot = function(gd, data, layout, config) {
     Lib.markTime('in plot');
 
-
     gd = getGraphDiv(gd);
 
-    /*
-     * Events.init is idempotent and bails early if gd has already been init'd
-     */
+    // Events.init is idempotent and bails early if gd has already been init'd
     Events.init(gd);
 
     var okToPlot = Events.triggerHandler(gd, 'plotly_beforeplot', [data, layout, config]);
-    if(okToPlot===false) return Promise.reject();
+    if(okToPlot === false) return Promise.reject();
 
     // if there's no data or layout, and this isn't yet a plotly plot
     // container, log a warning to help plotly.js users debug
@@ -89,22 +86,23 @@ Plotly.plot = function(gd, data, layout, config) {
     // complete, and empty out the promise list again.
     gd._promises = [];
 
+    var graphWasEmpty = ((gd.data || []).length === 0 && Array.isArray(data));
+
     // if there is already data on the graph, append the new data
     // if you only want to redraw, pass a non-array for data
-    var graphwasempty = ((gd.data||[]).length===0 && Array.isArray(data));
     if(Array.isArray(data)) {
         cleanData(data, gd.data);
 
-        if(graphwasempty) gd.data=data;
-        else gd.data.push.apply(gd.data,data);
+        if(graphWasEmpty) gd.data = data;
+        else gd.data.push.apply(gd.data, data);
 
         // for routines outside graph_obj that want a clean tab
         // (rather than appending to an existing one) gd.empty
         // is used to determine whether to make a new tab
-        gd.empty=false;
+        gd.empty = false;
     }
 
-    if(!gd.layout || graphwasempty) gd.layout = cleanLayout(layout);
+    if(!gd.layout || graphWasEmpty) gd.layout = cleanLayout(layout);
 
     // if the user is trying to drag the axes, allow new data and layout
     // to come in but don't allow a replot.
@@ -126,23 +124,28 @@ Plotly.plot = function(gd, data, layout, config) {
     // so we don't try to re-call Plotly.plot from inside
     // legend and colorbar, if margins changed
     gd._replotting = true;
-    var hasData = gd._fullData.length>0;
+    var hasData = gd._fullData.length > 0;
+
+    var subplots = Plotly.Axes.getSubplots(gd).join(''),
+        oldSubplots = Object.keys(gd._fullLayout._plots || {}).join(''),
+        hasSameSubplots = (oldSubplots === subplots);
 
     // Make or remake the framework (ie container and axes) if we need to
     // note: if they container already exists and has data,
     //  the new layout gets ignored (as it should)
     //  but if there's no data there yet, it's just a placeholder...
     //  then it should destroy and remake the plot
-    if (hasData) {
-        var subplots = Plotly.Axes.getSubplots(gd).join(''),
-            oldSubplots = Object.keys(gd._fullLayout._plots || {}).join('');
-
-        if(gd.framework!==makePlotFramework || graphwasempty || (oldSubplots!==subplots)) {
+    if(hasData) {
+        if(gd.framework !== makePlotFramework || graphWasEmpty || !hasSameSubplots) {
             gd.framework = makePlotFramework;
             makePlotFramework(gd);
         }
     }
-    else if(graphwasempty) makePlotFramework(gd);
+    else if(!hasSameSubplots) {
+        gd.framework = makePlotFramework;
+        makePlotFramework(gd);
+    }
+    else if(graphWasEmpty) makePlotFramework(gd);
 
     var fullLayout = gd._fullLayout;
 
@@ -160,7 +163,7 @@ Plotly.plot = function(gd, data, layout, config) {
     }
 
     // in case it has changed, attach fullData traces to calcdata
-    for (var i = 0; i < gd.calcdata.length; i++) {
+    for(var i = 0; i < gd.calcdata.length; i++) {
         gd.calcdata[i][0].trace = gd._fullData[i];
     }
 
@@ -2144,8 +2147,12 @@ Plotly.relayout = function relayout(gd, astr, val) {
         undoit[ai] = (pleaf === 'reverse') ? vi : p.get();
 
         // check autosize or autorange vs size and range
-        if(hw.indexOf(ai)!==-1) { doextra('autosize', false); }
-        else if(ai==='autosize') { doextra(hw, undefined); }
+        if(hw.indexOf(ai) !== -1) {
+            doextra('autosize', false);
+        }
+        else if(ai === 'autosize') {
+            doextra(hw, undefined);
+        }
         else if(pleafPlus.match(/^[xyz]axis[0-9]*\.range(\[[0|1]\])?$/)) {
             doextra(ptrunk+'.autorange', false);
         }
@@ -2165,6 +2172,10 @@ Plotly.relayout = function relayout(gd, astr, val) {
         else if(pleaf === 'tickmode') {
             doextra([ptrunk + '.tick0', ptrunk + '.dtick'], undefined);
         }
+        else if(/[xy]axis[0-9]*?$/.test(pleaf) && !Object.keys(vi || {}).length) {
+            docalc = true;
+        }
+
         // toggling log without autorange: need to also recalculate ranges
         // logical XOR (ie are we toggling log)
         if(pleaf==='type' && ((parentFull.type === 'log') !== (vi === 'log'))) {
@@ -2318,10 +2329,12 @@ Plotly.relayout = function relayout(gd, astr, val) {
         seq.push(function layoutReplot() {
             // force plot() to redo the layout
             gd.layout = undefined;
+
             // force it to redo calcdata?
             if(docalc) gd.calcdata = undefined;
+
             // replot with the modified layout
-            return Plotly.plot(gd,'',layout);
+            return Plotly.plot(gd, '', layout);
         });
     }
     else if(ak.length) {

src/plots/cartesian/index.js

@@ -100,10 +100,8 @@ exports.plot = function(gd) {
         }
 
         // finally do all error bars at once
-        if(fullLayout._hasCartesian) {
-            ErrorBars.plot(gd, subplotInfo, cdError);
-            Lib.markTime('done ErrorBars');
-        }
+        ErrorBars.plot(gd, subplotInfo, cdError);
+        Lib.markTime('done ErrorBars');
     }
 
     // now draw stuff not on subplots (ie, only pies at the moment)
@@ -114,3 +112,9 @@ exports.plot = function(gd) {
         if(cdPie.length) Pie.plot(gd, cdPie);
     }
 };
+
+exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
+    if(oldFullLayout._hasPie && !newFullLayout._hasPie) {
+        oldFullLayout._pielayer.selectAll('g.trace').remove();
+    }
+};

src/plots/cartesian/layout_defaults.js

@@ -20,28 +20,36 @@ var axisIds = require('./axis_ids');
 
 
 module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
-    // get the full list of axes already defined
     var layoutKeys = Object.keys(layoutIn),
-        xaList = [],
-        yaList = [],
+        xaListCartesian = [],
+        yaListCartesian = [],
+        xaListGl2d = [],
+        yaListGl2d = [],
         outerTicks = {},
         noGrids = {},
         i;
 
-    for(i = 0; i < layoutKeys.length; i++) {
-        var key = layoutKeys[i];
-        if(constants.xAxisMatch.test(key)) xaList.push(key);
-        else if(constants.yAxisMatch.test(key)) yaList.push(key);
-    }
-
+    // look for axes in the data
     for(i = 0; i < fullData.length; i++) {
-        var trace = fullData[i],
-            xaName = axisIds.id2name(trace.xaxis),
+        var trace = fullData[i];
+        var listX, listY;
+
+        if(Plots.traceIs(trace, 'cartesian')) {
+            listX = xaListCartesian;
+            listY = yaListCartesian;
+        }
+        else if(Plots.traceIs(trace, 'gl2d')) {
+            listX = xaListGl2d;
+            listY = yaListGl2d;
+        }
+        else continue;
+
+        var xaName = axisIds.id2name(trace.xaxis),
             yaName = axisIds.id2name(trace.yaxis);
 
         // add axes implied by traces
-        if(xaName && xaList.indexOf(xaName) === -1) xaList.push(xaName);
-        if(yaName && yaList.indexOf(yaName) === -1) yaList.push(yaName);
+        if(xaName && listX.indexOf(xaName) === -1) listX.push(xaName);
+        if(yaName && listY.indexOf(yaName) === -1) listY.push(yaName);
 
         // check for default formatting tweaks
         if(Plots.traceIs(trace, '2dMap')) {
@@ -55,22 +63,47 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
         }
     }
 
-    function axSort(a,b) {
-        var aNum = Number(a.substr(5)||1),
-            bNum = Number(b.substr(5)||1);
-        return aNum - bNum;
+    // N.B. Ignore orphan axes (i.e. axes that have no data attached to them)
+    // if gl3d or geo is present on graph. This is retain backward compatible.
+    //
+    // TODO drop this in version 2.0
+    var ignoreOrphan = (layoutOut._hasGL3D || layoutOut._hasGeo);
+
+    if(!ignoreOrphan) {
+        for(i = 0; i < layoutKeys.length; i++) {
+            var key = layoutKeys[i];
+
+            // orphan layout axes are considered cartesian subplots
+
+            if(xaListGl2d.indexOf(key) === -1 &&
+                xaListCartesian.indexOf(key) === -1 &&
+                    constants.xAxisMatch.test(key)) {
+                xaListCartesian.push(key);
+            }
+            else if(yaListGl2d.indexOf(key) === -1 &&
+                yaListCartesian.indexOf(key) === -1 &&
+                    constants.yAxisMatch.test(key)) {
+                yaListCartesian.push(key);
+            }
+        }
     }
 
-    if(layoutOut._hasCartesian || layoutOut._hasGL2D || !fullData.length) {
-        // make sure there's at least one of each and lists are sorted
-        if(!xaList.length) xaList = ['xaxis'];
-        else xaList.sort(axSort);
+    // make sure that plots with orphan cartesian axes
+    // are considered 'cartesian'
+    if(xaListCartesian.length && yaListCartesian.length) {
+        layoutOut._hasCartesian = true;
+    }
 
-        if(!yaList.length) yaList = ['yaxis'];
-        else yaList.sort(axSort);
+    function axSort(a, b) {
+        var aNum = Number(a.substr(5) || 1),
+            bNum = Number(b.substr(5) || 1);
+        return aNum - bNum;
     }
 
-    xaList.concat(yaList).forEach(function(axName){
+    var xaList = xaListCartesian.concat(xaListGl2d).sort(axSort),
+        yaList = yaListCartesian.concat(yaListGl2d).sort(axSort);
+
+    xaList.concat(yaList).forEach(function(axName) {
         var axLetter = axName.charAt(0),
             axLayoutIn = layoutIn[axName] || {},
             axLayoutOut = {},
@@ -100,7 +133,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
 
         // so we don't have to repeat autotype unnecessarily,
         // copy an autotype back to layoutIn
-        if(!layoutIn[axName] && axLayoutIn.type!=='-') {
+        if(!layoutIn[axName] && axLayoutIn.type !== '-') {
             layoutIn[axName] = {type: axLayoutIn.type};
         }
 

src/plots/geo/geo.js

@@ -56,6 +56,8 @@ function Geo(options, fullLayout) {
 
     this.makeFramework();
     this.updateFx(fullLayout.hovermode);
+
+    this.traceHash = {};
 }
 
 module.exports = Geo;
@@ -152,25 +154,47 @@ function filterData(dataIn) {
 }
 
 proto.onceTopojsonIsLoaded = function(geoData, geoLayout) {
-    var traceData = {};
+    var i;
 
     this.drawLayout(geoLayout);
 
-    for(var i = 0; i < geoData.length; i++) {
+    var traceHashOld = this.traceHash;
+    var traceHash = {};
+
+    for(i = 0; i < geoData.length; i++) {
         var trace = geoData[i];
 
-        traceData[trace.type] = traceData[trace.type] || [];
-        traceData[trace.type].push(trace);
+        traceHash[trace.type] = traceHash[trace.type] || [];
+        traceHash[trace.type].push(trace);
     }
 
-    var traceKeys = Object.keys(traceData);
-    for(var j = 0; j < traceKeys.length; j++){
-        var moduleData = traceData[traceKeys[j]];
+    var moduleNamesOld = Object.keys(traceHashOld);
+    var moduleNames = Object.keys(traceHash);
+
+    // when a trace gets deleted, make sure that its module's
+    // plot method is called so that it is properly
+    // removed from the DOM.
+    for(i = 0; i < moduleNamesOld.length; i++) {
+        var moduleName = moduleNamesOld[i];
+
+        if(moduleNames.indexOf(moduleName) === -1) {
+            var fakeModule = traceHashOld[moduleName][0];
+            fakeModule.visible = false;
+            traceHash[moduleName] = [fakeModule];
+        }
+    }
+
+    moduleNames = Object.keys(traceHash);
+
+    for(i = 0; i < moduleNames.length; i++) {
+        var moduleData = traceHash[moduleNames[i]];
         var _module = moduleData[0]._module;
 
         _module.plot(this, filterData(moduleData), geoLayout);
     }
 
+    this.traceHash = traceHash;
+
     this.render();
 };
 

src/plots/geo/index.js

@@ -65,3 +65,16 @@ exports.plot = function plotGeo(gd) {
         geo.plot(fullGeoData, fullLayout, gd._promises);
     }
 };
+
+exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
+    var oldGeoKeys = Plots.getSubplotIds(oldFullLayout, 'geo');
+
+    for(var i = 0; i < oldGeoKeys.length; i++) {
+        var oldGeoKey = oldGeoKeys[i];
+        var oldGeo = oldFullLayout[oldGeoKey]._geo;
+
+        if(!newFullLayout[oldGeoKey] && !!oldGeo) {
+            oldGeo.geoDiv.remove();
+        }
+    }
+};

src/plots/geo/layout/defaults.js

@@ -17,7 +17,9 @@ var supplyGeoAxisLayoutDefaults = require('./axis_defaults');
 
 
 module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
-    var geos = Plots.getSubplotIdsInData(fullData, 'geo'),
+    if(!layoutOut._hasGeo) return;
+
+    var geos = Plots.findSubplotIds(fullData, 'geo'),
         geosLength = geos.length;
 
     var geoLayoutIn, geoLayoutOut;
@@ -28,7 +30,12 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
 
     for(var i = 0; i < geosLength; i++) {
         var geo = geos[i];
-        geoLayoutIn = layoutIn[geo] || {};
+
+        // geo traces get a layout geo for free!
+        if(layoutIn[geo]) geoLayoutIn = layoutIn[geo];
+        else geoLayoutIn = layoutIn[geo] = {};
+
+        geoLayoutIn = layoutIn[geo];
         geoLayoutOut = {};
 
         coerce('domain.x');

src/plots/gl3d/index.js

@@ -67,6 +67,18 @@ exports.plot = function plotGl3d(gd) {
     }
 };
 
+exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
+    var oldSceneKeys = Plots.getSubplotIds(oldFullLayout, 'gl3d');
+
+    for(var i = 0; i < oldSceneKeys.length; i++) {
+        var oldSceneKey = oldSceneKeys[i];
+
+        if(!newFullLayout[oldSceneKey] && !!oldFullLayout[oldSceneKey]._scene) {
+            oldFullLayout[oldSceneKey]._scene.destroy();
+        }
+    }
+};
+
 // clean scene ids, 'scene1' -> 'scene'
 exports.cleanId = function cleanId(id) {
     if (!id.match(/^scene[0-9]*$/)) return;