From 23815107506653bbbbf147411dc13f4020c19c3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 1 Aug 2018 17:51:47 -0400 Subject: [PATCH 1/4] generalize setPositions for `barpolar` --- src/plot_api/plot_api.js | 4 ++++ src/plots/plots.js | 34 +++++++++++++++++++++++++--------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 1ee438d5f0b..22a4a938680 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -329,9 +329,13 @@ exports.plot = function(gd, data, layout, config) { marginPushers, marginPushersAgain ]; + if(hasCartesian) seq.push(positionAndAutorange); + else seq.push(Plots.doSetPositions); + seq.push(subroutines.layoutStyles); if(hasCartesian) seq.push(drawAxes); + seq.push( subroutines.drawData, subroutines.finalDraw, diff --git a/src/plots/plots.js b/src/plots/plots.js index 6557d3ccf20..62267e4348f 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -2566,24 +2566,40 @@ function clearAxesCalc(axList) { plots.doSetPositions = function(gd) { var fullLayout = gd._fullLayout; - var subplots = fullLayout._subplots.cartesian; var modules = fullLayout._visibleModules; - var methods = []; - var i, j; + var hash = {}; + var i, j, k; // position and range calculations for traces that // depend on each other ie bars (stacked or grouped) // and boxes (grouped) push each other out of the way for(j = 0; j < modules.length; j++) { - Lib.pushUnique(methods, modules[j].setPositions); + var _module = modules[j]; + var fn = _module.setPositions; + if(fn) { + var spType = _module.basePlotModule.name; + if(hash[spType]) { + Lib.pushUnique(hash[spType], fn); + } else { + hash[spType] = [fn]; + } + } } - if(!methods.length) return; - for(i = 0; i < subplots.length; i++) { - var subplotInfo = fullLayout._plots[subplots[i]]; - for(j = 0; j < methods.length; j++) { - methods[j](gd, subplotInfo); + for(k in hash) { + var methods = hash[k]; + var subplots = fullLayout._subplots[k]; + + for(i = 0; i < subplots.length; i++) { + var sp = subplots[i]; + var spInfo = k === 'cartesian' ? + fullLayout._plots[sp] : + fullLayout[sp]; + + for(j = 0; j < methods.length; j++) { + methods[j](gd, spInfo); + } } } }; From 6226cea6c34c813942d47120cb65918932636d76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 3 Aug 2018 15:12:22 -0400 Subject: [PATCH 2/4] rename setPositions -> crossTraceCalc ... and set_positions.js files -> cross_trace_calc.js --- src/plots/plots.js | 2 +- .../{set_positions.js => cross_trace_calc.js} | 2 +- src/traces/bar/index.js | 2 +- .../{set_positions.js => cross_trace_calc.js} | 4 ++-- src/traces/box/index.js | 2 +- src/traces/candlestick/index.js | 2 +- src/traces/histogram/index.js | 4 ++-- .../{set_positions.js => cross_trace_calc.js} | 4 ++-- src/traces/violin/index.js | 2 +- test/jasmine/tests/bar_test.js | 18 +++++++++--------- test/jasmine/tests/finance_test.js | 2 +- test/jasmine/tests/histogram_test.js | 2 +- 12 files changed, 23 insertions(+), 23 deletions(-) rename src/traces/bar/{set_positions.js => cross_trace_calc.js} (99%) rename src/traces/box/{set_positions.js => cross_trace_calc.js} (97%) rename src/traces/violin/{set_positions.js => cross_trace_calc.js} (90%) diff --git a/src/plots/plots.js b/src/plots/plots.js index 62267e4348f..4685b30b1a4 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -2576,7 +2576,7 @@ plots.doSetPositions = function(gd) { for(j = 0; j < modules.length; j++) { var _module = modules[j]; - var fn = _module.setPositions; + var fn = _module.crossTraceCalc; if(fn) { var spType = _module.basePlotModule.name; if(hash[spType]) { diff --git a/src/traces/bar/set_positions.js b/src/traces/bar/cross_trace_calc.js similarity index 99% rename from src/traces/bar/set_positions.js rename to src/traces/bar/cross_trace_calc.js index 1021a400a16..e00e9e8779f 100644 --- a/src/traces/bar/set_positions.js +++ b/src/traces/bar/cross_trace_calc.js @@ -24,7 +24,7 @@ var Sieve = require('./sieve.js'); * now doing this one subplot at a time */ -module.exports = function setPositions(gd, plotinfo) { +module.exports = function crossTraceCalc(gd, plotinfo) { var xa = plotinfo.xaxis, ya = plotinfo.yaxis; diff --git a/src/traces/bar/index.js b/src/traces/bar/index.js index b05ddd8b79d..91836d265c6 100644 --- a/src/traces/bar/index.js +++ b/src/traces/bar/index.js @@ -16,7 +16,7 @@ Bar.layoutAttributes = require('./layout_attributes'); Bar.supplyDefaults = require('./defaults'); Bar.supplyLayoutDefaults = require('./layout_defaults'); Bar.calc = require('./calc'); -Bar.setPositions = require('./set_positions'); +Bar.crossTraceCalc = require('./cross_trace_calc'); Bar.colorbar = require('../scatter/marker_colorbar'); Bar.arraysToCalcdata = require('./arrays_to_calcdata'); Bar.plot = require('./plot'); diff --git a/src/traces/box/set_positions.js b/src/traces/box/cross_trace_calc.js similarity index 97% rename from src/traces/box/set_positions.js rename to src/traces/box/cross_trace_calc.js index c10f511ce83..9a4aadd8d56 100644 --- a/src/traces/box/set_positions.js +++ b/src/traces/box/cross_trace_calc.js @@ -13,7 +13,7 @@ var Lib = require('../../lib'); var orientations = ['v', 'h']; -function setPositions(gd, plotinfo) { +function crossTraceCalc(gd, plotinfo) { var calcdata = gd.calcdata; var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; @@ -109,6 +109,6 @@ function setPositionOffset(traceType, gd, boxList, posAxis, pad) { } module.exports = { - setPositions: setPositions, + crossTraceCalc: crossTraceCalc, setPositionOffset: setPositionOffset }; diff --git a/src/traces/box/index.js b/src/traces/box/index.js index ef67721f8eb..3ad049e1701 100644 --- a/src/traces/box/index.js +++ b/src/traces/box/index.js @@ -15,7 +15,7 @@ Box.layoutAttributes = require('./layout_attributes'); Box.supplyDefaults = require('./defaults').supplyDefaults; Box.supplyLayoutDefaults = require('./layout_defaults').supplyLayoutDefaults; Box.calc = require('./calc'); -Box.setPositions = require('./set_positions').setPositions; +Box.crossTraceCalc = require('./cross_trace_calc').crossTraceCalc; Box.plot = require('./plot').plot; Box.style = require('./style').style; Box.styleOnSelect = require('./style').styleOnSelect; diff --git a/src/traces/candlestick/index.js b/src/traces/candlestick/index.js index ff568688ebb..3ec580002f0 100644 --- a/src/traces/candlestick/index.js +++ b/src/traces/candlestick/index.js @@ -32,7 +32,7 @@ module.exports = { attributes: require('./attributes'), layoutAttributes: require('../box/layout_attributes'), supplyLayoutDefaults: require('../box/layout_defaults').supplyLayoutDefaults, - setPositions: require('../box/set_positions').setPositions, + crossTraceCalc: require('../box/cross_trace_calc').crossTraceCalc, supplyDefaults: require('./defaults'), calc: require('./calc'), plot: require('../box/plot').plot, diff --git a/src/traces/histogram/index.js b/src/traces/histogram/index.js index c20e31d6e55..fc84b85e98a 100644 --- a/src/traces/histogram/index.js +++ b/src/traces/histogram/index.js @@ -12,7 +12,7 @@ /** * Histogram has its own attribute, defaults and calc steps, * but uses bar's plot to display - * and bar's setPositions for stacking and grouping + * and bar's crossTraceCalc (formally known as setPositions) for stacking and grouping */ /** @@ -30,7 +30,7 @@ Histogram.layoutAttributes = require('../bar/layout_attributes'); Histogram.supplyDefaults = require('./defaults'); Histogram.supplyLayoutDefaults = require('../bar/layout_defaults'); Histogram.calc = require('./calc'); -Histogram.setPositions = require('../bar/set_positions'); +Histogram.crossTraceCalc = require('../bar/cross_trace_calc'); Histogram.plot = require('../bar/plot'); Histogram.layerName = 'barlayer'; Histogram.style = require('../bar/style').style; diff --git a/src/traces/violin/set_positions.js b/src/traces/violin/cross_trace_calc.js similarity index 90% rename from src/traces/violin/set_positions.js rename to src/traces/violin/cross_trace_calc.js index d2ebb04c4b8..8a4483b92ec 100644 --- a/src/traces/violin/set_positions.js +++ b/src/traces/violin/cross_trace_calc.js @@ -8,10 +8,10 @@ 'use strict'; -var setPositionOffset = require('../box/set_positions').setPositionOffset; +var setPositionOffset = require('../box/cross_trace_calc').setPositionOffset; var orientations = ['v', 'h']; -module.exports = function setPositions(gd, plotinfo) { +module.exports = function crossTraceCalc(gd, plotinfo) { var calcdata = gd.calcdata; var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; diff --git a/src/traces/violin/index.js b/src/traces/violin/index.js index 9c058ef998d..4885709ceea 100644 --- a/src/traces/violin/index.js +++ b/src/traces/violin/index.js @@ -14,7 +14,7 @@ module.exports = { supplyDefaults: require('./defaults'), supplyLayoutDefaults: require('./layout_defaults'), calc: require('./calc'), - setPositions: require('./set_positions'), + crossTraceCalc: require('./cross_trace_calc'), plot: require('./plot'), style: require('./style'), styleOnSelect: require('../scatter/style').styleOnSelect, diff --git a/test/jasmine/tests/bar_test.js b/test/jasmine/tests/bar_test.js index 10ff65139d5..8c0df2d46ec 100644 --- a/test/jasmine/tests/bar_test.js +++ b/test/jasmine/tests/bar_test.js @@ -171,7 +171,7 @@ describe('Bar.supplyDefaults', function() { }); }); -describe('bar calc / setPositions', function() { +describe('bar calc / crossTraceCalc (formally known as setPositions)', function() { 'use strict'; it('should fill in calc pt fields (stack case)', function() { @@ -337,7 +337,7 @@ describe('Bar.calc', function() { }); }); -describe('Bar.setPositions', function() { +describe('Bar.crossTraceCalc (formally known as setPositions)', function() { 'use strict'; it('should guard against invalid offset items', function() { @@ -1347,9 +1347,9 @@ describe('bar visibility toggling:', function() { expect(fullLayout.xaxis.range).toBeCloseToArray(xrng, 2, msg + ' xrng'); expect(fullLayout.yaxis.range).toBeCloseToArray(yrng, 2, msg + ' yrng'); - var setPositions = gd._fullData[0]._module.setPositions; - expect(setPositions).toHaveBeenCalledTimes(calls); - setPositions.calls.reset(); + var crossTraceCalc = gd._fullData[0]._module.crossTraceCalc; + expect(crossTraceCalc).toHaveBeenCalledTimes(calls); + crossTraceCalc.calls.reset(); } it('should update axis range according to visible edits (group case)', function(done) { @@ -1358,7 +1358,7 @@ describe('bar visibility toggling:', function() { {type: 'bar', x: [1, 2, 3], y: [-1, -2, -1]} ]) .then(function() { - spyOn(gd._fullData[0]._module, 'setPositions').and.callThrough(); + spyOn(gd._fullData[0]._module, 'crossTraceCalc').and.callThrough(); _assert('base', [0.5, 3.5], [-2.222, 2.222], 0); return Plotly.restyle(gd, 'visible', false, [1]); @@ -1388,7 +1388,7 @@ describe('bar visibility toggling:', function() { {type: 'bar', x: [1, 2, 3], y: [2, 3, 2]} ], {barmode: 'stack'}) .then(function() { - spyOn(gd._fullData[0]._module, 'setPositions').and.callThrough(); + spyOn(gd._fullData[0]._module, 'crossTraceCalc').and.callThrough(); _assert('base', [0.5, 3.5], [0, 5.263], 0); return Plotly.restyle(gd, 'visible', false, [1]); @@ -1806,8 +1806,8 @@ function mockBarPlot(dataWithoutTraceType, layout) { yaxis: gd._fullLayout.yaxis }; - // call Bar.setPositions - Bar.setPositions(gd, plotinfo); + // call Bar.crossTraceCalc + Bar.crossTraceCalc(gd, plotinfo); return gd; } diff --git a/test/jasmine/tests/finance_test.js b/test/jasmine/tests/finance_test.js index df5e8614f02..edb3c45514c 100644 --- a/test/jasmine/tests/finance_test.js +++ b/test/jasmine/tests/finance_test.js @@ -380,7 +380,7 @@ describe('finance charts calc', function() { supplyAllDefaults(gd); Plots.doCalcdata(gd); gd.calcdata.forEach(function(cd) { - // fill in some stuff that happens during setPositions or plot + // fill in some stuff that happens during crossTraceCalc or plot if(cd[0].trace.type === 'candlestick') { var diff = cd[1].pos - cd[0].pos; cd[0].t.wHover = diff / 2; diff --git a/test/jasmine/tests/histogram_test.js b/test/jasmine/tests/histogram_test.js index d6728eddb5f..327b4d2feb7 100644 --- a/test/jasmine/tests/histogram_test.js +++ b/test/jasmine/tests/histogram_test.js @@ -237,7 +237,7 @@ describe('Test histogram', function() { var d73 = Date.UTC(1973, 0, 1); expect(out).toEqual([ // full calcdata has x and y too (and t in the first one), - // but those come later from setPositions. + // but those come later from crossTraceCalc. {i: 0, b: 0, p: d70, s: 2, pts: [0, 1], p0: d70, p1: d70}, {i: 1, b: 0, p: d71, s: 1, pts: [2], p0: d71, p1: d71}, {i: 2, b: 0, p: d72, s: 0, pts: [], p0: d72, p1: d72}, From 44b767778f2b03516bc45eda935064b92cd0de4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 3 Aug 2018 15:12:43 -0400 Subject: [PATCH 3/4] rename Plots.doSetPositions -> Plots.doCrossTraceCalc --- src/plot_api/plot_api.js | 4 ++-- src/plots/plots.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 22a4a938680..023edda5c54 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -292,7 +292,7 @@ exports.plot = function(gd, data, layout, config) { return; } - Plots.doSetPositions(gd); + Plots.doCrossTraceCalc(gd); // calc and autorange for errorbars Registry.getComponentMethod('errorbars', 'calc')(gd); @@ -331,7 +331,7 @@ exports.plot = function(gd, data, layout, config) { ]; if(hasCartesian) seq.push(positionAndAutorange); - else seq.push(Plots.doSetPositions); + else seq.push(Plots.doCrossTraceCalc); seq.push(subroutines.layoutStyles); if(hasCartesian) seq.push(drawAxes); diff --git a/src/plots/plots.js b/src/plots/plots.js index 4685b30b1a4..6243ca422fe 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -2236,7 +2236,7 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) plots.supplyDefaults(gd); plots.doCalcdata(gd); - plots.doSetPositions(gd); + plots.doCrossTraceCalc(gd); Registry.getComponentMethod('errorbars', 'calc')(gd); return Promise.resolve(); @@ -2564,7 +2564,7 @@ function clearAxesCalc(axList) { } } -plots.doSetPositions = function(gd) { +plots.doCrossTraceCalc = function(gd) { var fullLayout = gd._fullLayout; var modules = fullLayout._visibleModules; var hash = {}; From e2360382c4e13dc22b8cce928ad4e1bfa7c4ad87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 3 Aug 2018 17:53:22 -0400 Subject: [PATCH 4/4] fix typo in comments --- src/traces/histogram/index.js | 2 +- test/jasmine/tests/bar_test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/traces/histogram/index.js b/src/traces/histogram/index.js index fc84b85e98a..50abc6f24b7 100644 --- a/src/traces/histogram/index.js +++ b/src/traces/histogram/index.js @@ -12,7 +12,7 @@ /** * Histogram has its own attribute, defaults and calc steps, * but uses bar's plot to display - * and bar's crossTraceCalc (formally known as setPositions) for stacking and grouping + * and bar's crossTraceCalc (formerly known as setPositions) for stacking and grouping */ /** diff --git a/test/jasmine/tests/bar_test.js b/test/jasmine/tests/bar_test.js index 8c0df2d46ec..b6f30ceedd0 100644 --- a/test/jasmine/tests/bar_test.js +++ b/test/jasmine/tests/bar_test.js @@ -171,7 +171,7 @@ describe('Bar.supplyDefaults', function() { }); }); -describe('bar calc / crossTraceCalc (formally known as setPositions)', function() { +describe('bar calc / crossTraceCalc (formerly known as setPositions)', function() { 'use strict'; it('should fill in calc pt fields (stack case)', function() { @@ -337,7 +337,7 @@ describe('Bar.calc', function() { }); }); -describe('Bar.crossTraceCalc (formally known as setPositions)', function() { +describe('Bar.crossTraceCalc (formerly known as setPositions)', function() { 'use strict'; it('should guard against invalid offset items', function() {