add generalUpdatePerTraceModule method in Plots

- so that the same logic can be share for geo and ternary
  subplots (and maybe cartesian subplots down the road)
- fix bug occurring when the first trace on a given subplot
  had `visible: false` (which yielded an exception)

Author: etpinard

src/plots/geo/geo.js

@@ -15,6 +15,7 @@ var d3 = require('d3');
 
 var Color = require('../../components/color');
 var Drawing = require('../../components/drawing');
+var Plots = require('../plots');
 var Axes = require('../cartesian/axes');
 var Fx = require('../cartesian/graph_interact');
 
@@ -170,66 +171,13 @@ proto.plot = function(geoCalcData, fullLayout, promises) {
 };
 
 proto.onceTopojsonIsLoaded = function(geoCalcData, geoLayout) {
-    var i;
-
     this.drawLayout(geoLayout);
 
-    var traceHashOld = this.traceHash;
-    var traceHash = {};
-
-    for(i = 0; i < geoCalcData.length; i++) {
-        var calcData = geoCalcData[i],
-            trace = calcData[0].trace;
-
-        traceHash[trace.type] = traceHash[trace.type] || [];
-        traceHash[trace.type].push(calcData);
-    }
-
-    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 fakeCalcTrace = traceHashOld[moduleName][0],
-                fakeTrace = fakeCalcTrace[0].trace;
-
-            fakeTrace.visible = false;
-            traceHash[moduleName] = [fakeCalcTrace];
-        }
-    }
-
-    moduleNames = Object.keys(traceHash);
-
-    for(i = 0; i < moduleNames.length; i++) {
-        var moduleCalcData = traceHash[moduleNames[i]],
-            _module = moduleCalcData[0][0].trace._module;
-
-        _module.plot(this, filterVisible(moduleCalcData), geoLayout);
-    }
-
-    this.traceHash = traceHash;
+    Plots.generalUpdatePerTraceModule(this, geoCalcData, geoLayout);
 
     this.render();
 };
 
-function filterVisible(calcDataIn) {
-    var calcDataOut = [];
-
-    for(var i = 0; i < calcDataIn.length; i++) {
-        var calcTrace = calcDataIn[i],
-            trace = calcTrace[0].trace;
-
-        if(trace.visible === true) calcDataOut.push(calcTrace);
-    }
-
-    return calcDataOut;
-}
-
 proto.updateFx = function(hovermode) {
     this.showHover = (hovermode !== false);
 

src/plots/plots.js

@@ -2052,3 +2052,67 @@ plots.doCalcdata = function(gd, traces) {
         calcdata[i] = cd;
     }
 };
+
+plots.generalUpdatePerTraceModule = function(subplot, subplotCalcData, subplotLayout) {
+    var traceHashOld = subplot.traceHash,
+        traceHash = {},
+        i;
+
+    function filterVisible(calcDataIn) {
+        var calcDataOut = [];
+
+        for(var i = 0; i < calcDataIn.length; i++) {
+            var calcTrace = calcDataIn[i],
+                trace = calcTrace[0].trace;
+
+            if(trace.visible === true) calcDataOut.push(calcTrace);
+        }
+
+        return calcDataOut;
+    }
+
+    // build up moduleName -> calcData hash
+    for(i = 0; i < subplotCalcData.length; i++) {
+        var calcTraces = subplotCalcData[i],
+            trace = calcTraces[0].trace;
+
+        // skip over visible === false traces
+        // as they don't have `_module` ref
+        if(trace.visible) {
+            traceHash[trace.type] = traceHash[trace.type] || [];
+            traceHash[trace.type].push(calcTraces);
+        }
+    }
+
+    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 fakeCalcTrace = traceHashOld[moduleName][0],
+                fakeTrace = fakeCalcTrace[0].trace;
+
+            fakeTrace.visible = false;
+            traceHash[moduleName] = [fakeCalcTrace];
+        }
+    }
+
+    // update list of module names to include 'fake' traces added above
+    moduleNames = Object.keys(traceHash);
+
+    // call module plot method
+    for(i = 0; i < moduleNames.length; i++) {
+        var moduleCalcData = traceHash[moduleNames[i]],
+            _module = moduleCalcData[0][0].trace._module;
+
+        _module.plot(subplot, filterVisible(moduleCalcData), subplotLayout);
+    }
+
+    // update moduleName -> calcData hash
+    subplot.traceHash = traceHash;
+};

test/jasmine/tests/plots_test.js

@@ -582,4 +582,114 @@ describe('Test Plots', function() {
         });
     });
 
+    describe('Plots.generalUpdatePerTraceModule', function() {
+
+        function _update(subplotCalcData, traceHashOld) {
+            var subplot = { traceHash: traceHashOld || {} };
+            var calcDataPerModule = [];
+
+            var plot = function(_, moduleCalcData) {
+                calcDataPerModule.push(moduleCalcData);
+            };
+
+            subplotCalcData.forEach(function(calcTrace) {
+                calcTrace[0].trace._module = { plot: plot };
+            });
+
+            Plots.generalUpdatePerTraceModule(subplot, subplotCalcData, {});
+
+            return {
+                traceHash: subplot.traceHash,
+                calcDataPerModule: calcDataPerModule
+            };
+        }
+
+        it('should update subplot trace hash and call module plot method with correct calcdata traces', function() {
+            var out = _update([
+                [ { trace: { type: 'A', visible: false } } ],
+                [ { trace: { type: 'A', visible: true } } ],
+                [ { trace: { type: 'B', visible: false } } ],
+                [ { trace: { type: 'C', visible: true } } ]
+            ]);
+
+            expect(Object.keys(out.traceHash)).toEqual(['A', 'C']);
+            expect(out.traceHash.A.length).toEqual(1);
+            expect(out.traceHash.C.length).toEqual(1);
+
+            expect(out.calcDataPerModule.length).toEqual(2);
+            expect(out.calcDataPerModule[0].length).toEqual(1);
+            expect(out.calcDataPerModule[1].length).toEqual(1);
+
+            var out2 = _update([
+                [ { trace: { type: 'A', visible: false } } ],
+                [ { trace: { type: 'A', visible: false } } ],
+                [ { trace: { type: 'B', visible: true } } ],
+                [ { trace: { type: 'C', visible: false } } ]
+            ], out.traceHash);
+
+            expect(Object.keys(out2.traceHash)).toEqual(['B', 'A', 'C']);
+            expect(out2.traceHash.B.length).toEqual(1);
+            expect(out2.traceHash.A.length).toEqual(1);
+            expect(out2.traceHash.A[0][0].trace.visible).toBe(false);
+            expect(out2.traceHash.C.length).toEqual(1);
+            expect(out2.traceHash.C[0][0].trace.visible).toBe(false);
+
+            expect(out2.calcDataPerModule.length).toEqual(1);
+            expect(out2.calcDataPerModule[0].length).toEqual(1);
+
+            var out3 = _update([
+                [ { trace: { type: 'A', visible: false } } ],
+                [ { trace: { type: 'A', visible: false } } ],
+                [ { trace: { type: 'B', visible: false } } ],
+                [ { trace: { type: 'C', visible: false } } ]
+            ], out2.traceHash);
+
+            expect(Object.keys(out3.traceHash)).toEqual(['B', 'A', 'C']);
+            expect(out3.traceHash.B.length).toEqual(1);
+            expect(out3.traceHash.B[0][0].trace.visible).toBe(false);
+            expect(out3.traceHash.A.length).toEqual(1);
+            expect(out3.traceHash.A[0][0].trace.visible).toBe(false);
+            expect(out3.traceHash.C.length).toEqual(1);
+            expect(out3.traceHash.C[0][0].trace.visible).toBe(false);
+
+            expect(out3.calcDataPerModule.length).toEqual(0);
+
+            var out4 = _update([
+                [ { trace: { type: 'A', visible: true } } ],
+                [ { trace: { type: 'A', visible: true } } ],
+                [ { trace: { type: 'B', visible: true } } ],
+                [ { trace: { type: 'C', visible: true } } ]
+            ], out3.traceHash);
+
+            expect(Object.keys(out4.traceHash)).toEqual(['A', 'B', 'C']);
+            expect(out4.traceHash.A.length).toEqual(2);
+            expect(out4.traceHash.B.length).toEqual(1);
+            expect(out4.traceHash.C.length).toEqual(1);
+
+            expect(out4.calcDataPerModule.length).toEqual(3);
+            expect(out4.calcDataPerModule[0].length).toEqual(2);
+            expect(out4.calcDataPerModule[1].length).toEqual(1);
+            expect(out4.calcDataPerModule[2].length).toEqual(1);
+        });
+
+        it('should handle cases when module plot is not set (geo case)', function(done) {
+            Plotly.plot(createGraphDiv(), [{
+                type: 'scattergeo',
+                visible: false,
+                lon: [10, 20],
+                lat: [20, 10]
+            }, {
+                type: 'scattergeo',
+                lon: [10, 20],
+                lat: [20, 10]
+            }])
+            .then(function() {
+                expect(d3.selectAll('g.trace.scattergeo').size()).toEqual(1);
+
+                destroyGraphDiv();
+                done();
+            });
+        });
+
+    });
 });