-
Notifications
You must be signed in to change notification settings - Fork 4
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
Issue 4 of cosmoz-treenode-navigator #1
Changes from 15 commits
92c303e
9dca38a
b19a36f
b9fb2ff
f7d2fdb
dea0b9a
760b426
1c88582
cd64e21
5a2064e
f8b75df
5af285e
d8e9ffd
e878ec1
00bb3de
44ca085
13f837a
4174ae1
47dfbf0
16dcf27
90122a3
fa33008
e465ec6
9e5078d
3c5318f
23ab530
5308aae
20a3aad
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 |
---|---|---|
@@ -1,3 +1,4 @@ | ||
bower_components/ | ||
node_modules/ | ||
debug.log | ||
.DS_Store |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,22 @@ | ||
<link rel="import" href="cosmoz-tree.html"> | ||
|
||
<!-- | ||
Navigator through object with treelike datastructure and default settings. | ||
|
||
@demo demo/index.html | ||
--> | ||
|
||
<script> | ||
'use strict'; | ||
|
||
Polymer({ is: 'cosmoz-default-tree' }); | ||
|
||
window.Cosmoz = window.Cosmoz || {}; | ||
|
||
function _objectValues(obj) { | ||
if (!obj) { | ||
return; | ||
} | ||
return Object.keys(obj).map(function (key) { | ||
return obj[key]; | ||
}); | ||
|
@@ -23,134 +35,146 @@ | |
|
||
Cosmoz.DefaultTree.prototype = Object.create(Cosmoz.Tree.prototype); | ||
|
||
Cosmoz.DefaultTree.prototype.getNodeByProperty = function (propertyName, propertyValue) { | ||
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. As in comment, needs to stay. |
||
/** | ||
* Returns the first found Node. | ||
*/ | ||
Cosmoz.DefaultTree.prototype.getNodeByProperty = function (propertyName, propertyValue, exact, nodes) { | ||
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 might be a use case to search for a node with a specific (exact/fuzzy) property beneath a certain node in the tree, but it can't be a required argument since we have a tree already. ( |
||
if (propertyName === undefined || propertyValue === undefined) { | ||
return; | ||
} | ||
// Defaults | ||
exact = exact !== undefined ? exact : true; | ||
|
||
if (propertyName === 'pathLocator') { | ||
return this._getNodeByPathLocator(propertyValue); | ||
return this.getNodeByPathLocator(propertyValue); | ||
} | ||
|
||
return this._searchAllRoots(propertyName, propertyValue); | ||
var results = this.searchNodes(propertyName, propertyValue, exact, nodes); | ||
return results && results.length > 0 ? results[0] : null; | ||
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. If we use But do we need to make a difference between |
||
}; | ||
|
||
Cosmoz.DefaultTree.prototype.getPath = function (node) { | ||
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. don't think we can drop getPath |
||
if (node === undefined) { | ||
return; | ||
} | ||
return this._getPathByPathLocator(node.pathLocator); | ||
/** | ||
* Loops over a node list (or roots if not defined) and returns the concatenated results of search(). | ||
*/ | ||
Cosmoz.DefaultTree.prototype.searchNodes = function (propertyName, propertyValue, exact, nodes) { | ||
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. I'm a bit concerned that the behavior has changed completely from returning the first hit, to returning first or all, to always returning all. It also has a significant performance hit since we often expect only (or care about) just the first hit. How about: _searchNodes = function (propertyValue, options, nodes) { } where the result gathering is something like: nodes.some(function (node) {
results = results.concat(this.search(node, propertyValue, options));
return options.firstHitOnly && results.length > 0;
}, this); and Then we can expose two different methods to use it: searchNodes = function (propertyValue, exact, propertyName, nodes) {
var options = {
propertyName: propertyName || this.searchProperty,
exact: exact !== undefined ? exact : true,
firstHitOnly: false
};
return this._searchNodes(propertyValue, options, nodes);
}
findNode = function (propertyValue, propertyName, nodes) {
var options = {
propertyName: propertyName || this.searchProperty,
exact: true,
firstHitOnly: true
},
nodes = this._searchNodes(propertyValue, options, nodes);
return nodes.shift() || null;
} I think this would make the calling code more readable as well, where we more explicitly describe our intent to find a unique node, or to find nodes matching a pattern. I think since What do you think? 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. Love it! Really nice! |
||
var results = []; | ||
|
||
// Defaults | ||
exact = exact !== undefined ? exact : true; | ||
nodes = nodes ? nodes : this._roots; | ||
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.
|
||
|
||
nodes.forEach(function (node) { | ||
results = results.concat(this.search(node, propertyName, propertyValue, exact)); | ||
}, this); | ||
|
||
return results; | ||
}; | ||
|
||
Cosmoz.DefaultTree.prototype.getPathByProperty = function (propertyName, propertyValue) { | ||
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. Can't drop getPathByProperty 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. As in comment, needs to stay. |
||
if (propertyName === undefined || propertyValue === undefined) { | ||
return; | ||
/** | ||
* Returns the Node of a given pathLocator. | ||
*/ | ||
Cosmoz.DefaultTree.prototype.getNodeByPathLocator = function (pathLocator) { | ||
if (!pathLocator) { | ||
return this._treeData; | ||
} | ||
var pathNodes = this.getPathNodes(pathLocator, this._treeData), | ||
lastNodeOnPath = pathNodes.pop(); | ||
return lastNodeOnPath !== undefined ? lastNodeOnPath : null; | ||
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.
But do we need to differentiate between |
||
}; | ||
|
||
if (propertyName === 'pathLocator') { | ||
return this._getPathByPathLocator(propertyValue); | ||
/** | ||
* Returns an Array of nodes on a given path. | ||
* If pathLocator is empty or not defined, the Root objects get returned. | ||
*/ | ||
Cosmoz.DefaultTree.prototype.getPathNodes = function (pathLocator, nodes, separatorSign, childProperty) { | ||
if (!pathLocator) { | ||
return this._roots; | ||
} | ||
|
||
var node = this._searchAllRoots(propertyName, propertyValue); | ||
if (node !== undefined) { | ||
return this._getPathByPathLocator(node.pathLocator); | ||
} | ||
}; | ||
// Defaults | ||
separatorSign = separatorSign ? separatorSign : '.'; | ||
childProperty = childProperty ? childProperty : 'children'; | ||
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. I think since |
||
|
||
var path = pathLocator.split(separatorSign), | ||
pathSegment = nodes ? nodes : this._treeData, | ||
pathHasUndefinedSegments, | ||
nodes; | ||
|
||
// Get the nodes on the path. | ||
nodes = path.map(function (nodeKey) { | ||
var node = pathSegment[nodeKey], | ||
children = node[childProperty]; | ||
if (node && children !== undefined && Object.keys(children).length > 0) { | ||
pathSegment = children; | ||
} | ||
return node; | ||
}, this); | ||
|
||
Cosmoz.DefaultTree.prototype.getChildren = function (node) { | ||
if (node === undefined) { | ||
return; | ||
} | ||
return _objectValues(node.children); | ||
}; | ||
// Check if all nodes on the path are valid. | ||
pathHasUndefinedSegments = nodes.some(function (n) { | ||
if (n === undefined) { | ||
return true; | ||
} | ||
}); | ||
|
||
Cosmoz.DefaultTree.prototype.getProperty = function (node, propertyName) { | ||
if (node === undefined || propertyName === undefined) { | ||
return; | ||
} | ||
return pathHasUndefinedSegments ? null : nodes; | ||
}, | ||
|
||
return node[propertyName]; | ||
}; | ||
/** | ||
* Returns a String (separated node properties) representing the path of a given path locator. | ||
*/ | ||
Cosmoz.DefaultTree.prototype.getPathString = function (pathLocator, pathProperty, pathSeparator) { | ||
var pathNodes = this.getPathNodes(pathLocator, this._treeData); | ||
|
||
Cosmoz.DefaultTree.prototype._searchAllRoots = function (propertyName, propertyValue) { | ||
var i, | ||
node; | ||
// Defaults | ||
pathSeparator = pathSeparator ? pathSeparator : '/'; | ||
|
||
for (i = 0; i < this._roots.length; i+=1) { | ||
node = this.search(this._roots[i], propertyName, propertyValue); | ||
if (node !== undefined) { | ||
return node; | ||
} | ||
} | ||
return pathNodes.map(function (node) { | ||
return node[pathProperty]; | ||
}).join(pathSeparator); | ||
}; | ||
|
||
Cosmoz.DefaultTree.prototype._getNodeByPathLocator = function (pathLocator) { | ||
var node, | ||
path, | ||
i; | ||
Cosmoz.DefaultTree.prototype.getPathStringByProperty = function (propertyName, propertyValue, pathProperty, pathSeparator) { | ||
if (propertyName === undefined || propertyValue === undefined) { | ||
return; | ||
} | ||
|
||
for (i = 0; i < this._roots.length; i+=1) { | ||
node = this._roots[i]; | ||
if (node.pathLocator === pathLocator) { | ||
return node; | ||
} | ||
// Defaults | ||
pathProperty = pathProperty ? pathProperty : 'name'; | ||
|
||
if (pathLocator.indexOf(node.pathLocator + '.') === 0) { | ||
path = pathLocator.slice(node.pathLocator.length + 1).split('.'); | ||
for (i = 0; i < path.length; i+=1) { | ||
if (node.children) { | ||
node = node.children[path[i]]; | ||
} else { | ||
node = null; | ||
} | ||
if (node === undefined || node === null) { | ||
break; | ||
} | ||
} | ||
|
||
if (node !== undefined && node !== null) { | ||
return node; | ||
} | ||
} | ||
if (propertyName === 'pathLocator') { | ||
return this.getPathString(propertyValue, pathProperty); | ||
} | ||
|
||
var node = this.getNodeByProperty(propertyName, propertyValue); | ||
|
||
if (node) { | ||
return this.getPathString(node.pathLocator || node.path, pathProperty); | ||
} | ||
}; | ||
|
||
Cosmoz.DefaultTree.prototype._getPathByPathLocator = function (pathLocator) { | ||
var node, | ||
path, | ||
subPath, | ||
i; | ||
/** | ||
* Returns an Object or an Array representing the children of a node. | ||
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. Again, multiple return types. 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. Docs needs updating |
||
*/ | ||
Cosmoz.DefaultTree.prototype.getChildren = function (node, childProperty) { | ||
if (!node) { | ||
return; | ||
} | ||
|
||
for (i = 0; i < this._roots.length; i+=1) { | ||
node = this._roots[i]; | ||
path = [node]; | ||
// Defaults | ||
childProperty = childProperty ? childProperty : 'children'; | ||
|
||
if (node.pathLocator === pathLocator) { | ||
return path; | ||
} | ||
return _objectValues(node[childProperty]); | ||
}; | ||
|
||
if (pathLocator.indexOf(node.pathLocator + '.') === 0) { | ||
subPath = pathLocator.slice(node.pathLocator.length + 1).split('.'); | ||
for (i = 0; i < subPath.length; i+=1) { | ||
if (node.children) { | ||
node = node.children[subPath[i]]; | ||
} else { | ||
node = null; | ||
} | ||
if (node === undefined || node === null) { | ||
path = null; | ||
break; | ||
} else { | ||
path.push(node); | ||
} | ||
} | ||
|
||
if (path !== null) { | ||
return path; | ||
} | ||
} | ||
/** | ||
* Returns the property of a Node based on a given property name. | ||
*/ | ||
Cosmoz.DefaultTree.prototype.getProperty = function (node, propertyName) { | ||
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. @nomego actually similar discussion as with 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. You're right, it's pretty weird. |
||
if (!node || !propertyName) { | ||
return; | ||
} | ||
return node[propertyName]; | ||
}; | ||
|
||
|
||
</script> | ||
</script> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,13 @@ | ||
|
||
<!-- | ||
Navigator through object with treelike datastructure. | ||
--> | ||
|
||
<script> | ||
'use strict'; | ||
|
||
Polymer({ is: 'cosmoz-tree' }); | ||
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. Not sure this needs to be a Polymer element either? |
||
|
||
window.Cosmoz = window.Cosmoz || {}; | ||
|
||
|
||
|
@@ -22,56 +29,68 @@ | |
throw new Error('Must be implemented in derived object'); | ||
}, | ||
|
||
getPath: function (node) { | ||
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. Keep this placeholder method in "abstract class" too |
||
getPathNodes: function (node) { | ||
throw new Error('Must be implemented in derived object'); | ||
}, | ||
|
||
getPathByProperty: function (propertyName, propertyValue) { | ||
getPathString: function (node) { | ||
throw new Error('Must be implemented in derived object'); | ||
}, | ||
|
||
getChildren: function (node) { | ||
getPathStringByProperty: function (node) { | ||
throw new Error('Must be implemented in derived object'); | ||
}, | ||
|
||
getProperty: function (node, propertyName) { | ||
getChildren: function (node, asList) { | ||
throw new Error('Must be implemented in derived object'); | ||
}, | ||
|
||
getPathString: function (node, textPropertyName, separator) { | ||
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 method (in PR called "getPath") should be able to live on in the "abstract class"? |
||
var path = this.getPath(node); | ||
return path.map(function (n) { | ||
return this.getProperty(n, textPropertyName); | ||
}, this).join(separator); | ||
getProperty: function (node, propertyName) { | ||
throw new Error('Must be implemented in derived object'); | ||
}, | ||
|
||
getPathStringByProperty: function (propertyName, propertyValue, textPropertyName, separator) { | ||
var node = this.getNodeByProperty(propertyName, propertyValue); | ||
if (node !== undefined) { | ||
return this.getPathString(node, textPropertyName, separator); | ||
/** | ||
* Returns True if a node fullfills the search criteria. | ||
*/ | ||
nodeConformsSearch: function (node, propertyName, propertyValue, exact) { | ||
var property = this.getProperty(node, propertyName); | ||
|
||
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. You could also do
to be sure that the property exists for the remainder of the function. |
||
if (exact) { | ||
return property && property === propertyValue; | ||
} | ||
|
||
return property && property.toLowerCase().indexOf(propertyValue.toLowerCase()) > -1; | ||
}, | ||
|
||
/** | ||
* Basic search for a node by one of its property | ||
* Basic search for a node by one of its properties. | ||
*/ | ||
search: function (node, propertyName, propertyValue) { | ||
if (this.getProperty(node, propertyName) === propertyValue) { | ||
return node; | ||
search: function (node, propertyName, propertyValue, exact, results) { | ||
var nodeConforms = this.nodeConformsSearch(node, propertyName, propertyValue, exact), | ||
children, | ||
result, | ||
i; | ||
|
||
// Defaults | ||
exact = exact !== undefined ? exact : true; | ||
results = results ? results : []; | ||
|
||
if (nodeConforms) { | ||
results.push(node); | ||
} | ||
|
||
var children = this.getChildren(node), | ||
i = 0, | ||
n; | ||
children = this.getChildren(node); | ||
|
||
if (children) { | ||
for (i = 0; i < children.length; i+=1) { | ||
n = this.search(children[i], propertyName, propertyValue); | ||
if (n !== undefined) { | ||
return n; | ||
if (Array.isArray(children)) { | ||
for (i = 0; i < children.length; i++) { | ||
result = this.search(children[i], propertyName, propertyValue, exact, results); | ||
if (!Array.isArray(result)) { | ||
return result; | ||
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. Should be |
||
} | ||
} | ||
} | ||
|
||
return results; | ||
} | ||
}; | ||
</script> | ||
</script> |
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.
I'm not sure we actually need to make this a Polymer element.
Did you see a reason?
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.
The only reason was for the
iron-component-page
to not crash. (demo & docs)Maybe there is another way but I didn't explore it yet..
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.
Oh ok well then leave it but maybe add a comment about it, too.