Skip to content

Make frame with nulls clear items & array containers #1118

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 11 commits into from
Nov 11, 2016
Merged
7 changes: 4 additions & 3 deletions src/components/annotations/annotation_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ var Axes = require('../../plots/cartesian/axes');
var attributes = require('./attributes');


module.exports = function handleAnnotationDefaults(annIn, fullLayout) {
var annOut = {};
module.exports = function handleAnnotationDefaults(annIn, annOut, fullLayout, opts, itemOpts) {
opts = opts || {};
itemOpts = itemOpts || {};

function coerce(attr, dflt) {
return Lib.coerce(annIn, annOut, attributes, attr, dflt);
}

var visible = coerce('visible');
var visible = coerce('visible', !itemOpts.itemIsNotPlainObject);

if(!visible) return annOut;

Expand Down
14 changes: 6 additions & 8 deletions src/components/annotations/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,15 @@

'use strict';

var handleArrayContainerDefaults = require('../../plots/array_container_defaults');
var handleAnnotationDefaults = require('./annotation_defaults');


module.exports = function supplyLayoutDefaults(layoutIn, layoutOut) {
var containerIn = layoutIn.annotations || [],
containerOut = layoutOut.annotations = [];
var opts = {
name: 'annotations',
handleItemDefaults: handleAnnotationDefaults
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🌴

};

for(var i = 0; i < containerIn.length; i++) {
var annIn = containerIn[i] || {},
annOut = handleAnnotationDefaults(annIn, layoutOut);

containerOut.push(annOut);
}
handleArrayContainerDefaults(layoutIn, layoutOut, opts);
};
3 changes: 2 additions & 1 deletion src/components/annotations/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ function drawOne(gd, index, opt, value) {
optionsIn[axLetter] = position;
}

var options = handleAnnotationDefaults(optionsIn, fullLayout);
var options = {};
handleAnnotationDefaults(optionsIn, options, fullLayout);
fullLayout.annotations[index] = options;

var xa = Axes.getFromId(gd, options.xref),
Expand Down
20 changes: 8 additions & 12 deletions src/components/images/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,20 @@

'use strict';

var Axes = require('../../plots/cartesian/axes');
var Lib = require('../../lib');
var attributes = require('./attributes');
var Axes = require('../../plots/cartesian/axes');
var handleArrayContainerDefaults = require('../../plots/array_container_defaults');

var attributes = require('./attributes');
var name = 'images';

module.exports = function supplyLayoutDefaults(layoutIn, layoutOut) {
var contIn = Array.isArray(layoutIn[name]) ? layoutIn[name] : [],
contOut = layoutOut[name] = [];
var opts = {
name: name,
handleItemDefaults: imageDefaults
};

for(var i = 0; i < contIn.length; i++) {
var itemIn = contIn[i] || {},
itemOut = {};

imageDefaults(itemIn, itemOut, layoutOut);

contOut.push(itemOut);
}
handleArrayContainerDefaults(layoutIn, layoutOut, opts);
};


Expand Down
14 changes: 6 additions & 8 deletions src/components/shapes/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,15 @@

'use strict';

var handleArrayContainerDefaults = require('../../plots/array_container_defaults');
var handleShapeDefaults = require('./shape_defaults');


module.exports = function supplyLayoutDefaults(layoutIn, layoutOut) {
var containerIn = layoutIn.shapes || [],
containerOut = layoutOut.shapes = [];
var opts = {
name: 'shapes',
handleItemDefaults: handleShapeDefaults
};

for(var i = 0; i < containerIn.length; i++) {
var shapeIn = containerIn[i] || {},
shapeOut = handleShapeDefaults(shapeIn, layoutOut);

containerOut.push(shapeOut);
}
handleArrayContainerDefaults(layoutIn, layoutOut, opts);
};
3 changes: 2 additions & 1 deletion src/components/shapes/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@ function updateShape(gd, index, opt, value) {
optionsIn[posAttr] = position;
}

var options = handleShapeDefaults(optionsIn, gd._fullLayout);
var options = {};
handleShapeDefaults(optionsIn, options, gd._fullLayout);
gd._fullLayout.shapes[index] = options;

var clipAxes;
Expand Down
8 changes: 5 additions & 3 deletions src/components/shapes/shape_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@ var Axes = require('../../plots/cartesian/axes');
var attributes = require('./attributes');
var helpers = require('./helpers');

module.exports = function handleShapeDefaults(shapeIn, fullLayout) {
var shapeOut = {};

module.exports = function handleShapeDefaults(shapeIn, shapeOut, fullLayout, opts, itemOpts) {
opts = opts || {};
itemOpts = itemOpts || {};

function coerce(attr, dflt) {
return Lib.coerce(shapeIn, shapeOut, attributes, attr, dflt);
}

var visible = coerce('visible');
var visible = coerce('visible', !itemOpts.itemIsNotPlainObject);

if(!visible) return shapeOut;

Expand Down
22 changes: 6 additions & 16 deletions src/components/sliders/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
'use strict';

var Lib = require('../../lib');
var handleArrayContainerDefaults = require('../../plots/array_container_defaults');

var attributes = require('./attributes');
var constants = require('./constants');
Expand All @@ -18,23 +19,12 @@ var stepAttrs = attributes.steps;


module.exports = function slidersDefaults(layoutIn, layoutOut) {
var contIn = Array.isArray(layoutIn[name]) ? layoutIn[name] : [],
contOut = layoutOut[name] = [];
var opts = {
name: name,
handleItemDefaults: sliderDefaults
};

for(var i = 0; i < contIn.length; i++) {
var sliderIn = contIn[i] || {},
sliderOut = {};

sliderDefaults(sliderIn, sliderOut, layoutOut);

// used on button click to update the 'active' field
sliderOut._input = sliderIn;

// used to determine object constancy
sliderOut._index = i;

contOut.push(sliderOut);
}
handleArrayContainerDefaults(layoutIn, layoutOut, opts);
};

function sliderDefaults(sliderIn, sliderOut, layoutOut) {
Expand Down
22 changes: 6 additions & 16 deletions src/components/updatemenus/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
'use strict';

var Lib = require('../../lib');
var handleArrayContainerDefaults = require('../../plots/array_container_defaults');

var attributes = require('./attributes');
var constants = require('./constants');
Expand All @@ -18,23 +19,12 @@ var buttonAttrs = attributes.buttons;


module.exports = function updateMenusDefaults(layoutIn, layoutOut) {
var contIn = Array.isArray(layoutIn[name]) ? layoutIn[name] : [],
contOut = layoutOut[name] = [];
var opts = {
name: name,
handleItemDefaults: menuDefaults
};

for(var i = 0; i < contIn.length; i++) {
var menuIn = contIn[i] || {},
menuOut = {};

menuDefaults(menuIn, menuOut, layoutOut);

// used on button click to update the 'active' field
menuOut._input = menuIn;

// used to determine object constancy
menuOut._index = i;

contOut.push(menuOut);
}
handleArrayContainerDefaults(layoutIn, layoutOut, opts);
};

function menuDefaults(menuIn, menuOut, layoutOut) {
Expand Down
19 changes: 9 additions & 10 deletions src/plot_api/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,12 @@ exports.cleanLayout = function(layout) {
}
}

if(layout.annotations !== undefined && !Array.isArray(layout.annotations)) {
Lib.warn('Annotations must be an array.');
delete layout.annotations;
}
var annotationsLen = (layout.annotations || []).length;
var annotationsLen = Array.isArray(layout.annotations) ? layout.annotations.length : 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this commit was enough to fix bug discovered in http://codepen.io/etpinard/pen/xRGgwW

for(i = 0; i < annotationsLen; i++) {
var ann = layout.annotations[i];

if(!Lib.isPlainObject(ann)) continue;

if(ann.ref) {
if(ann.ref === 'paper') {
ann.xref = 'paper';
Expand All @@ -120,17 +119,17 @@ exports.cleanLayout = function(layout) {
}
delete ann.ref;
}

cleanAxRef(ann, 'xref');
cleanAxRef(ann, 'yref');
}

if(layout.shapes !== undefined && !Array.isArray(layout.shapes)) {
Lib.warn('Shapes must be an array.');
delete layout.shapes;
}
var shapesLen = (layout.shapes || []).length;
var shapesLen = Array.isArray(layout.shapes) ? layout.shapes.length : 0;
for(i = 0; i < shapesLen; i++) {
var shape = layout.shapes[i];

if(!Lib.isPlainObject(shape)) continue;

cleanAxRef(shape, 'xref');
cleanAxRef(shape, 'yref');
}
Expand Down
67 changes: 67 additions & 0 deletions src/plots/array_container_defaults.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* Copyright 2012-2016, Plotly, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

var Lib = require('../lib');


/** Convenience wrapper for making array container logic DRY and consistent
*
* @param {object} parentObjIn
* user input object where the container in question is linked
* (i.e. either a user trace object or the user layout object)
*
* @param {object} parentObjOut
* full object where the coerced container will be linked
* (i.e. either a full trace object or the full layout object)
*
* @param {object} opts
* options object:
* - name {string}
* name of the key linking the container in question
* - handleItemDefaults {function}
* defaults method to be called on each item in the array container in question
*
* Its arguments are:
* - itemIn {object} item in user layout
* - itemOut {object} item in full layout
* - parentObj {object} (as in closure)
* - opts {object} (as in closure)
* - itemOpts {object}
* - itemIsNotPlainObject {boolean}
* N.B.
*
* - opts is passed to handleItemDefaults so it can also store
* links to supplementary data (e.g. fullData for layout components)
*
*/
module.exports = function handleArrayContainerDefaults(parentObjIn, parentObjOut, opts) {
Copy link
Contributor Author

@etpinard etpinard Nov 10, 2016

Choose a reason for hiding this comment

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

similar to plots/subplot_defaults.

The logic below is pretty trivial, but might as well 🔒 down the behavior for all array containers.

var name = opts.name;

var contIn = Array.isArray(parentObjIn[name]) ? parentObjIn[name] : [],
contOut = parentObjOut[name] = [];

for(var i = 0; i < contIn.length; i++) {
var itemIn = contIn[i],
itemOut = {},
itemOpts = {};

if(!Lib.isPlainObject(itemIn)) {
itemOpts.itemIsNotPlainObject = true;
itemIn = {};
}

opts.handleItemDefaults(itemIn, itemOut, parentObjOut, opts, itemOpts);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, that's fine. I still don't quite see where the handler will use the original opts but I guess it doesn't hurt to leave it in.


itemOut._input = itemIn;
itemOut._index = i;

contOut.push(itemOut);
}
};
19 changes: 15 additions & 4 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -1569,8 +1569,14 @@ plots.extendObjectWithContainers = function(dest, src, containerPaths) {
for(i = 0; i < containerPaths.length; i++) {
containerProp = Lib.nestedProperty(expandedObj, containerPaths[i]);
containerVal = containerProp.get();
containerProp.set(null);
Lib.nestedProperty(containerObj, containerPaths[i]).set(containerVal);

if(containerVal === undefined) {
Lib.nestedProperty(containerObj, containerPaths[i]).set(null);
}
else {
containerProp.set(null);
Lib.nestedProperty(containerObj, containerPaths[i]).set(containerVal);
}
}
}

Expand All @@ -1584,15 +1590,20 @@ plots.extendObjectWithContainers = function(dest, src, containerPaths) {
if(!srcContainer) continue;

destProp = Lib.nestedProperty(dest, containerPaths[i]);

destContainer = destProp.get();

if(!Array.isArray(destContainer)) {
destContainer = [];
destProp.set(destContainer);
}

for(j = 0; j < srcContainer.length; j++) {
destContainer[j] = plots.extendObjectWithContainers(destContainer[j], srcContainer[j]);
var srcObj = srcContainer[j];

if(srcObj === null) destContainer[j] = null;
else {
destContainer[j] = plots.extendObjectWithContainers(destContainer[j], srcObj);
}
}

destProp.set(destContainer);
Expand Down
Loading