Skip to content

Commit 17503cb

Browse files
committed
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.
1 parent 54b3f1c commit 17503cb

File tree

8 files changed

+65
-89
lines changed

8 files changed

+65
-89
lines changed

src/plots/plots.js

+12-2
Original file line numberDiff line numberDiff line change
@@ -1421,11 +1421,21 @@ plots.purge = function(gd) {
14211421

14221422
plots.style = function(gd) {
14231423
var _modules = gd._fullLayout._modules;
1424+
var styleModules = [];
1425+
var i;
1426+
1427+
// some trace modules reuse the same style method,
1428+
// make sure to not unnecessary call them multiple times.
14241429

1425-
for(var i = 0; i < _modules.length; i++) {
1430+
for(i = 0; i < _modules.length; i++) {
14261431
var _module = _modules[i];
1432+
if(_module.style) {
1433+
Lib.pushUnique(styleModules, _module.style);
1434+
}
1435+
}
14271436

1428-
if(_module.style) _module.style(gd);
1437+
for(i = 0; i < styleModules.length; i++) {
1438+
styleModules[i](gd);
14291439
}
14301440
};
14311441

src/traces/scattercarpet/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ ScatterCarpet.supplyDefaults = require('./defaults');
1515
ScatterCarpet.colorbar = require('../scatter/colorbar');
1616
ScatterCarpet.calc = require('./calc');
1717
ScatterCarpet.plot = require('./plot');
18-
ScatterCarpet.style = require('./style');
18+
ScatterCarpet.style = require('../scatter/style').style;
1919
ScatterCarpet.hoverPoints = require('./hover');
2020
ScatterCarpet.selectPoints = require('../scatter/select');
2121
ScatterCarpet.eventData = require('./event_data');

src/traces/scattercarpet/style.js

-27
This file was deleted.

src/traces/scatterpolar/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ module.exports = {
1818
supplyDefaults: require('./defaults'),
1919
calc: require('./calc'),
2020
plot: require('./plot'),
21-
style: require('./style'),
21+
style: require('../scatter/style').style,
2222
hoverPoints: require('./hover'),
2323
selectPoints: require('../scatter/select'),
2424

src/traces/scatterpolar/style.js

-29
This file was deleted.

src/traces/scatterternary/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ ScatterTernary.supplyDefaults = require('./defaults');
1515
ScatterTernary.colorbar = require('../scatter/colorbar');
1616
ScatterTernary.calc = require('./calc');
1717
ScatterTernary.plot = require('./plot');
18-
ScatterTernary.style = require('./style');
18+
ScatterTernary.style = require('../scatter/style').style;
1919
ScatterTernary.hoverPoints = require('./hover');
2020
ScatterTernary.selectPoints = require('../scatter/select');
2121
ScatterTernary.eventData = require('./event_data');

src/traces/scatterternary/style.js

-27
This file was deleted.

test/jasmine/tests/plots_test.js

+50-1
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ var Lib = require('@src/lib');
55
var d3 = require('d3');
66
var createGraphDiv = require('../assets/create_graph_div');
77
var destroyGraphDiv = require('../assets/destroy_graph_div');
8+
var fail = require('../assets/fail_test');
89
var supplyAllDefaults = require('../assets/supply_defaults');
910

10-
1111
describe('Test Plots', function() {
1212
'use strict';
1313

@@ -737,4 +737,53 @@ describe('Test Plots', function() {
737737
});
738738
});
739739
});
740+
741+
describe('Plots.style', function() {
742+
var gd;
743+
744+
beforeEach(function() {
745+
gd = createGraphDiv();
746+
});
747+
748+
afterEach(destroyGraphDiv);
749+
750+
it('should call reused style modules only once per graph', function(done) {
751+
var Drawing = require('@src/components/drawing');
752+
753+
Plotly.plot(gd, [{
754+
mode: 'markers',
755+
y: [1, 2, 1]
756+
}, {
757+
type: 'scatterternary',
758+
mode: 'markers',
759+
a: [1, 2, 3],
760+
b: [2, 1, 3],
761+
c: [3, 2, 1]
762+
}, {
763+
type: 'scatterpolar',
764+
mode: 'markers',
765+
r: [1, 2, 3],
766+
theta: [0, 90, 120]
767+
}])
768+
.then(function() {
769+
expect(gd._fullLayout._modules.length).toBe(3);
770+
771+
// A routine that gets called inside Scatter.style,
772+
// once per trace.
773+
//
774+
// Start spying on it here, so that calls outside of
775+
// Plots.style are ignored.
776+
spyOn(Drawing, 'pointStyle');
777+
778+
return Plots.style(gd);
779+
})
780+
.then(function() {
781+
// N.B. Drawing.pointStyle would be called 9 times w/o
782+
// some special Plots.style logic.
783+
expect(Drawing.pointStyle).toHaveBeenCalledTimes(3);
784+
})
785+
.catch(fail)
786+
.then(done);
787+
});
788+
});
740789
});

0 commit comments

Comments
 (0)