Merge pull request #2455 from plotly/legendonly-fix

fix #2452 - removing and adding scatter(gl) as not the first module

Author: alexcjohnson

src/plots/cartesian/index.js

@@ -216,35 +216,19 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout)
     var oldModules = oldFullLayout._modules || [],
         newModules = newFullLayout._modules || [];
 
-    var hadScatter, hasScatter, hadGl, hasGl, i, oldPlots, ids, subplotInfo;
+    var hadScatter, hasScatter, hadGl, hasGl, i, oldPlots, ids, subplotInfo, moduleName;
 
 
     for(i = 0; i < oldModules.length; i++) {
-        if(oldModules[i].name === 'scatter') {
-            hadScatter = true;
-        }
-        break;
-    }
-
-    for(i = 0; i < newModules.length; i++) {
-        if(newModules[i].name === 'scatter') {
-            hasScatter = true;
-            break;
-        }
-    }
-
-    for(i = 0; i < oldModules.length; i++) {
-        if(oldModules[i].name === 'scattergl') {
-            hadGl = true;
-        }
-        break;
+        moduleName = oldModules[i].name;
+        if(moduleName === 'scatter') hadScatter = true;
+        else if(moduleName === 'scattergl') hadGl = true;
     }
 
     for(i = 0; i < newModules.length; i++) {
-        if(newModules[i].name === 'scattergl') {
-            hasGl = true;
-            break;
-        }
+        moduleName = newModules[i].name;
+        if(moduleName === 'scatter') hasScatter = true;
+        else if(moduleName === 'scattergl') hasGl = true;
     }
 
     if(hadScatter && !hasScatter) {

src/traces/scattergl/index.js

@@ -610,7 +610,9 @@ function sceneUpdate(gd, subplot) {
             scene.selectBatch = null;
             scene.unselectBatch = null;
 
-            delete subplot._scene;
+            // we can't just delete _scene, because `destroy` is called in the
+            // middle of supplyDefaults, before relinkPrivateKeys which will put it back.
+            subplot._scene = null;
         };
     }
 

test/jasmine/tests/cartesian_test.js

@@ -159,6 +159,58 @@ describe('restyle', function() {
             .then(done);
 
         });
+
+        it('can legend-hide the second and only scatter trace', function(done) {
+            Plotly.plot(gd, [
+                {y: [1, 2, 3], type: 'bar'},
+                {y: [1, 2, 3], xaxis: 'x2', yaxis: 'y2', type: 'scatter'}
+            ], {
+                xaxis: {domain: [0, 0.4]},
+                xaxis2: {domain: [0.6, 1]},
+                yaxis2: {anchor: 'x2'},
+                width: 600,
+                height: 400
+            })
+            .then(function() {
+                expect(d3.select('.scatter').size()).toBe(1);
+                return Plotly.restyle(gd, {visible: 'legendonly'}, 1);
+            })
+            .then(function() {
+                expect(d3.select('.scatter').size()).toBe(0);
+                return Plotly.restyle(gd, {visible: true}, 1);
+            })
+            .then(function() {
+                expect(d3.select('.scatter').size()).toBe(1);
+            })
+            .catch(failTest)
+            .then(done);
+        });
+
+        it('@gl can legend-hide the second and only scattergl trace', function(done) {
+            Plotly.plot(gd, [
+                {y: [1, 2, 3], type: 'bar'},
+                {y: [1, 2, 3], xaxis: 'x2', yaxis: 'y2', type: 'scattergl'}
+            ], {
+                xaxis: {domain: [0, 0.4]},
+                xaxis2: {domain: [0.6, 1]},
+                yaxis2: {anchor: 'x2'},
+                width: 600,
+                height: 400
+            })
+            .then(function() {
+                expect(!!gd._fullLayout._plots.x2y2._scene).toBe(true);
+                return Plotly.restyle(gd, {visible: 'legendonly'}, 1);
+            })
+            .then(function() {
+                expect(!!gd._fullLayout._plots.x2y2._scene).toBe(false);
+                return Plotly.restyle(gd, {visible: true}, 1);
+            })
+            .then(function() {
+                expect(!!gd._fullLayout._plots.x2y2._scene).toBe(true);
+            })
+            .catch(failTest)
+            .then(done);
+        });
     });
 });
 

test/jasmine/tests/gl2d_plot_interact_test.js

@@ -39,7 +39,7 @@ describe('@gl Test removal of gl contexts', function() {
             expect(gd._fullLayout._plots.xy._scene).toBeDefined();
             Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout);
 
-            expect(gd._fullLayout._plots.xy._scene).toBeUndefined();
+            expect(!!gd._fullLayout._plots.xy._scene).toBe(false);
         })
         .then(done);
     });