-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Changes from 8 commits
69bffe1
4c49755
d21d782
138ac35
d0953a0
174b640
36859f1
66f8bb7
8ed1957
9de2f77
5de6ff0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch here. Thanks! The only (I think) case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done in 9de2f77 |
||
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'; | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done in 9de2f77 |
||
for(i = 0; i < shapesLen; i++) { | ||
var shape = layout.shapes[i]; | ||
|
||
if(!Lib.isPlainObject(shape)) continue; | ||
|
||
cleanAxRef(shape, 'xref'); | ||
cleanAxRef(shape, 'yref'); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
/** | ||
* 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, | ||
* | ||
* N.B. | ||
* | ||
* - opts is passed to handleItemDefaults so it can also store | ||
* links to supplementary data (e.g. fullData for layout components) | ||
* | ||
* - opts.itemIsNotPlainObject is mutated on every pass in case so logic | ||
* in handleItemDefaults relies on that fact. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this feels a little odd to me. Do you ever see a case where other parts of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, annotations and shapes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. I'll make this the last arg. I've ran into some problem with our multi-argument internal function lately, but yeah you're right, mutating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done in 5de6ff0 |
||
* | ||
*/ | ||
module.exports = function handleArrayContainerDefaults(parentObjIn, parentObjOut, opts) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar to 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 = {}; | ||
|
||
if(!Lib.isPlainObject(itemIn)) { | ||
opts.itemIsNotPlainObject = true; | ||
itemIn = {}; | ||
} | ||
else { | ||
opts.itemIsNotPlainObject = false; | ||
} | ||
|
||
opts.handleItemDefaults(itemIn, itemOut, parentObjOut, opts); | ||
|
||
itemOut._input = itemIn; | ||
itemOut._index = i; | ||
|
||
contOut.push(itemOut); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌴