make scatter{ternary,polar,carpet} Scatter.style reuse straight-up

- instead of wrapping Scatter.style - make Plots.style detect if some trace module reuse the same style module.
plotly · Dec 15, 2017 · 17503cb · 17503cb
1 parent 54b3f1c
commit 17503cb
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 89 deletions.
diff --git a/src/plots/plots.js b/src/plots/plots.js
@@ -1421,11 +1421,21 @@ plots.purge = function(gd) {
 
 plots.style = function(gd) {
     var _modules = gd._fullLayout._modules;
+    var styleModules = [];
+    var i;
+
+    // some trace modules reuse the same style method,
+    // make sure to not unnecessary call them multiple times.
 
-    for(var i = 0; i < _modules.length; i++) {
+    for(i = 0; i < _modules.length; i++) {
         var _module = _modules[i];
+        if(_module.style) {
+            Lib.pushUnique(styleModules, _module.style);
+        }
+    }
 
-        if(_module.style) _module.style(gd);
+    for(i = 0; i < styleModules.length; i++) {
+        styleModules[i](gd);
     }
 };
 

diff --git a/src/traces/scattercarpet/index.js b/src/traces/scattercarpet/index.js
@@ -15,7 +15,7 @@ ScatterCarpet.supplyDefaults = require('./defaults');
 ScatterCarpet.colorbar = require('../scatter/colorbar');
 ScatterCarpet.calc = require('./calc');
 ScatterCarpet.plot = require('./plot');
-ScatterCarpet.style = require('./style');
+ScatterCarpet.style = require('../scatter/style').style;
 ScatterCarpet.hoverPoints = require('./hover');
 ScatterCarpet.selectPoints = require('../scatter/select');
 ScatterCarpet.eventData = require('./event_data');

diff --git a/src/traces/scattercarpet/style.js b/src/traces/scattercarpet/style.js
diff --git a/src/traces/scatterpolar/index.js b/src/traces/scatterpolar/index.js
@@ -18,7 +18,7 @@ module.exports = {
     supplyDefaults: require('./defaults'),
     calc: require('./calc'),
     plot: require('./plot'),
-    style: require('./style'),
+    style: require('../scatter/style').style,
     hoverPoints: require('./hover'),
     selectPoints: require('../scatter/select'),
 

diff --git a/src/traces/scatterpolar/style.js b/src/traces/scatterpolar/style.js
diff --git a/src/traces/scatterternary/index.js b/src/traces/scatterternary/index.js
@@ -15,7 +15,7 @@ ScatterTernary.supplyDefaults = require('./defaults');
 ScatterTernary.colorbar = require('../scatter/colorbar');
 ScatterTernary.calc = require('./calc');
 ScatterTernary.plot = require('./plot');
-ScatterTernary.style = require('./style');
+ScatterTernary.style = require('../scatter/style').style;
 ScatterTernary.hoverPoints = require('./hover');
 ScatterTernary.selectPoints = require('../scatter/select');
 ScatterTernary.eventData = require('./event_data');

diff --git a/src/traces/scatterternary/style.js b/src/traces/scatterternary/style.js
diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js
@@ -5,9 +5,9 @@ var Lib = require('@src/lib');
 var d3 = require('d3');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
+var fail = require('../assets/fail_test');
 var supplyAllDefaults = require('../assets/supply_defaults');
 
-
 describe('Test Plots', function() {
     'use strict';
 
@@ -737,4 +737,53 @@ describe('Test Plots', function() {
             });
         });
     });
+
+    describe('Plots.style', function() {
+        var gd;
+
+        beforeEach(function() {
+            gd = createGraphDiv();
+        });
+
+        afterEach(destroyGraphDiv);
+
+        it('should call reused style modules only once per graph', function(done) {
+            var Drawing = require('@src/components/drawing');
+
+            Plotly.plot(gd, [{
+                mode: 'markers',
+                y: [1, 2, 1]
+            }, {
+                type: 'scatterternary',
+                mode: 'markers',
+                a: [1, 2, 3],
+                b: [2, 1, 3],
+                c: [3, 2, 1]
+            }, {
+                type: 'scatterpolar',
+                mode: 'markers',
+                r: [1, 2, 3],
+                theta: [0, 90, 120]
+            }])
+            .then(function() {
+                expect(gd._fullLayout._modules.length).toBe(3);
+
+                // A routine that gets called inside Scatter.style,
+                // once per trace.
+                //
+                // Start spying on it here, so that calls outside of
+                // Plots.style are ignored.
+                spyOn(Drawing, 'pointStyle');
+
+                return Plots.style(gd);
+            })
+            .then(function() {
+                // N.B. Drawing.pointStyle would be called 9 times w/o
+                // some special Plots.style logic.
+                expect(Drawing.pointStyle).toHaveBeenCalledTimes(3);
+            })
+            .catch(fail)
+            .then(done);
+        });
+    });
 });