Skip to content

Allow transform to have supply layout defaults handler #1122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,9 @@ plots.sendDataToCloud = function(gd) {
// gd._fullLayout._basePlotModules
// is a list of all the plot modules required to draw the plot.
//
// gd._fullLayout._transformModules
// is a list of all the transform modules invoked.
//
plots.supplyDefaults = function(gd) {
var oldFullLayout = gd._fullLayout || {},
newFullLayout = gd._fullLayout = {},
Expand All @@ -375,6 +378,9 @@ plots.supplyDefaults = function(gd) {

var i;

// Create all the storage space for frames, but only if doesn't already exist
if(!gd._transitionData) plots.createTransitionData(gd);

// first fill in what we can of layout without looking at data
// because fullData needs a few things from layout

Expand Down Expand Up @@ -435,7 +441,7 @@ plots.supplyDefaults = function(gd) {
}

// finally, fill in the pieces of layout that may need to look at data
plots.supplyLayoutModuleDefaults(newLayout, newFullLayout, newFullData);
plots.supplyLayoutModuleDefaults(newLayout, newFullLayout, newFullData, gd._transitionData);

// TODO remove in v2.0.0
// add has-plot-type refs to fullLayout for backward compatibility
Expand Down Expand Up @@ -474,12 +480,6 @@ plots.supplyDefaults = function(gd) {
(gd.calcdata[i][0] || {}).trace = trace;
}
}

// Create all the storage space for frames, but only if doesn't already
// exist:
if(!gd._transitionData) {
plots.createTransitionData(gd);
}
};

// Create storage for all of the data related to frames and transitions:
Expand Down Expand Up @@ -646,6 +646,8 @@ plots.supplyDataDefaults = function(dataIn, dataOut, layout, fullLayout) {
basePlotModules = fullLayout._basePlotModules = [],
cnt = 0;

fullLayout._transformModules = [];

function pushModule(fullTrace) {
dataOut.push(fullTrace);

Expand Down Expand Up @@ -863,6 +865,8 @@ function supplyTransformDefaults(traceIn, traceOut, layout) {
transformOut = _module.supplyDefaults(transformIn, traceOut, layout, traceIn);
transformOut.type = type;
transformOut._module = _module;

Lib.pushUnique(layout._transformModules, _module);
}
else {
transformOut = Lib.extendFlat({}, transformIn);
Expand Down Expand Up @@ -1032,7 +1036,7 @@ function calculateReservedMargins(margins) {
return resultingMargin;
}

plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData) {
plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData, transitionData) {
var i, _module;

// can't be be part of basePlotModules loop
Expand Down Expand Up @@ -1063,6 +1067,16 @@ plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData) {
}
}

// transform module layout defaults
var transformModules = layoutOut._transformModules;
for(i = 0; i < transformModules.length; i++) {
_module = transformModules[i];

if(_module.supplyLayoutDefaults) {
_module.supplyLayoutDefaults(layoutIn, layoutOut, fullData, transitionData);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rreusser I chose to only pass transitionData to transform module supplyLayoutDefaults, because that's the only place where we need them at the moment.

}
}

// should FX be a component?
Plotly.Fx.supplyLayoutDefaults(layoutIn, layoutOut, fullData);

Expand Down
8 changes: 5 additions & 3 deletions test/jasmine/tests/transform_filter_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ var assertStyle = require('../assets/assert_style');

describe('filter transforms defaults:', function() {

var fullLayout = { _transformModules: [] };

var traceIn, traceOut;

it('supplyTraceDefaults should coerce all attributes', function() {
Expand All @@ -22,7 +24,7 @@ describe('filter transforms defaults:', function() {
}]
};

traceOut = Plots.supplyTraceDefaults(traceIn, 0, {});
traceOut = Plots.supplyTraceDefaults(traceIn, 0, fullLayout);

expect(traceOut.transforms).toEqual([{
type: 'filter',
Expand All @@ -44,7 +46,7 @@ describe('filter transforms defaults:', function() {
}]
};

traceOut = Plots.supplyTraceDefaults(traceIn, 0, {});
traceOut = Plots.supplyTraceDefaults(traceIn, 0, fullLayout);

expect(traceOut.transforms).toEqual([{
type: 'filter',
Expand All @@ -70,7 +72,7 @@ describe('filter transforms defaults:', function() {
}]
};

traceOut = Plots.supplyTraceDefaults(traceIn, 0, {});
traceOut = Plots.supplyTraceDefaults(traceIn, 0, fullLayout);

expect(traceOut.transforms[0].target).toEqual('x');
expect(traceOut.transforms[1].target).toEqual('x');
Expand Down
28 changes: 22 additions & 6 deletions test/jasmine/tests/transform_multi_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ var assertStyle = require('../assets/assert_style');
describe('general transforms:', function() {
'use strict';

var fullLayout = { _transformModules: [] };

var traceIn, traceOut;

it('supplyTraceDefaults should supply the transform defaults', function() {
Expand All @@ -21,7 +23,7 @@ describe('general transforms:', function() {
transforms: [{ type: 'filter' }]
};

traceOut = Plots.supplyTraceDefaults(traceIn, 0, {});
traceOut = Plots.supplyTraceDefaults(traceIn, 0, fullLayout);

expect(traceOut.transforms).toEqual([{
type: 'filter',
Expand All @@ -39,7 +41,7 @@ describe('general transforms:', function() {
transforms: [{ type: 'invalid' }]
};

traceOut = Plots.supplyTraceDefaults(traceIn, 0, {});
traceOut = Plots.supplyTraceDefaults(traceIn, 0, fullLayout);

expect(traceOut.y).toBe(traceIn.y);
});
Expand All @@ -56,6 +58,7 @@ describe('general transforms:', function() {
};

var layout = {
_transformModules: [],
_globalTransforms: [{
type: 'filter'
}]
Expand All @@ -80,6 +83,8 @@ describe('general transforms:', function() {
target: 'x',
_module: Filter
}, '- trace second');

expect(layout._transformModules).toEqual([Filter]);
});

it('supplyDataDefaults should apply the transform while', function() {
Expand Down Expand Up @@ -164,8 +169,10 @@ describe('user-defined transforms:', function() {
transforms: [transformIn]
}];

var layout = {},
fullLayout = {};
var fullData = [],
layout = {},
fullLayout = { _has: function() {} },
transitionData = {};

function assertSupplyDefaultsArgs(_transformIn, traceOut, _layout) {
expect(_transformIn).toBe(transformIn);
Expand All @@ -184,16 +191,25 @@ describe('user-defined transforms:', function() {
return dataOut;
}

function assertSupplyLayoutDefaultsArgs(_layout, _fullLayout, _fullData, _transitionData) {
expect(_layout).toBe(layout);
expect(_fullLayout).toBe(fullLayout);
expect(_fullData).toBe(fullData);
expect(_transitionData).toBe(transitionData);
}

var fakeTransformModule = {
moduleType: 'transform',
name: 'fake',
attributes: {},
supplyDefaults: assertSupplyDefaultsArgs,
transform: assertTransformArgs
transform: assertTransformArgs,
supplyLayoutDefaults: assertSupplyLayoutDefaultsArgs
};

Plotly.register(fakeTransformModule);
Plots.supplyDataDefaults(dataIn, [], layout, fullLayout);
Plots.supplyDataDefaults(dataIn, fullData, layout, fullLayout);
Plots.supplyLayoutModuleDefaults(layout, fullLayout, fullData, transitionData);
delete Plots.transformsRegistry.fake;
});

Expand Down