-
-
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
69bffe1
test: add cases for Lib.expandObjectPaths w/ array containers
etpinard 4c49755
make clean data work for non-plain-object annotation / shape items
etpinard d21d782
add general array container defaults handler
etpinard 138ac35
DRY layout array container defaults using handleArrayContainerDefaults
etpinard d0953a0
test: add cases for array container defaults
etpinard 174b640
make sure array containers set to null in frames are honored
etpinard 36859f1
make sure items inside array container set to null are honored in frames
etpinard 66f8bb7
test: add cases for extendObjectWithContainers
etpinard 8ed1957
Merge branch 'master' into frame-extend-with-nulls
etpinard 9de2f77
make annotation/shapes cleanLayout blocks more robust
etpinard 5de6ff0
split container opts from item opts in handleArrayContainerDefaults
etpinard File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. | ||
* | ||
*/ | ||
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); | ||
} | ||
}; |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
this feels a little odd to me. Do you ever see a case where other parts of
opts
would need to be accessible to the item handler (or is there one already that I didn't notice?), or could we just passitemIsNotPlainObject
by itself as the last arg to the handler?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.
yep, annotations and shapes.
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.
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
opts
is stupid.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.
done in 5de6ff0