code review

Author: Aarondorn2

addon/components/x-tree-children.js

@@ -1,6 +1,6 @@
 import Component from '@ember/component';
 import layout from '../templates/components/x-tree-children';
-import { setProperties } from '@ember/object';
+import { get, setProperties } from '@ember/object';
 
 export default Component.extend({
   layout,
@@ -10,8 +10,7 @@ export default Component.extend({
   actions: {
     updateCheckbox() {
       if (this.recursiveCheck) {
-        let model = this.model;
-        let children = model.children;
+        let children = get(this, 'model.children');
 
         if (children.length) {
           let isChecked = false;
@@ -23,7 +22,7 @@ export default Component.extend({
             isIndeterminate = true;
           }
 
-          setProperties(model, { isChecked, isIndeterminate });
+          setProperties(this.model, { isChecked, isIndeterminate });
         }
 
         if (this.updateCheckbox) {

addon/components/x-tree-node.js

@@ -1,5 +1,5 @@
 import Component from '@ember/component';
-import { computed, set, setProperties }  from '@ember/object';
+import { computed, get, set, setProperties }  from '@ember/object';
 import layout from '../templates/components/x-tree-node';
 
 export default Component.extend({
@@ -13,20 +13,18 @@ export default Component.extend({
   recursiveCheck: false,
 
   isChosen: computed('model.id', 'chosenId', function() {
-    return this.model.id === this.chosenId;
+    return get(this, 'model.id') === this.chosenId;
   }),
 
   click() {
-    let select = this.onSelect;
-    if (select) {
-      let model = this.model;
-      let wasChecked = model.isChecked;
+    if (this.onSelect) {
+      let wasChecked = get(this, 'model.isChecked');
 
-      select(model);
+      this.onSelect(this.model);
 
-      let isChecked = model.isChecked;
+      let isChecked = get(this, 'model.isChecked');
       if (isChecked !== wasChecked && this.recursiveCheck) {
-        this.setChildCheckboxesRecursively(model, isChecked);
+        this.setChildCheckboxesRecursively(this.model, isChecked);
         this.updateCheckbox();
       }
     }
@@ -35,23 +33,21 @@ export default Component.extend({
   mouseEnter() {
     set(this, 'model.isSelected', true);
 
-    let hover = this.onHover;
-    if (hover) {
-      hover(this.model);
+    if (this.onHover) {
+      this.onHover(this.model);
     }
   },
 
   mouseLeave() {
     set(this, 'model.isSelected', false);
 
-    let hoverOut = this.onHoverOut;
-    if (hoverOut) {
-      hoverOut(this.model);
+    if (this.onHoverOut) {
+      this.onHoverOut(this.model);
     }
   },
 
   setChildCheckboxesRecursively(node, isChecked) {
-    let children = node.children;
+    let children = get(node, 'children');
     if (children.length) {
       children.forEach(child => {
         setProperties(child, {
@@ -69,16 +65,14 @@ export default Component.extend({
       event.stopPropagation();
 
       let isChecked = this.toggleProperty('model.isChecked');
-      let model = this.model;
 
       if (this.recursiveCheck) {
-        this.setChildCheckboxesRecursively(model, isChecked);
+        this.setChildCheckboxesRecursively(this.model, isChecked);
         this.updateCheckbox();
       }
 
-      let check = this.onCheck;
-      if (check) {
-        check(model);
+      if (this.onCheck) {
+        this.onCheck(this.model);
       }
     },
 

addon/components/x-tree.js

@@ -1,7 +1,7 @@
 import Component from '@ember/component';
 import layout from '../templates/components/x-tree';
 import { getDescendents, getAncestors } from '../utils/tree';
-import { set }  from '@ember/object';
+import { get, set }  from '@ember/object';
 
 export default Component.extend({
   layout,
@@ -14,25 +14,22 @@ export default Component.extend({
 
   init() {
     this._super(...arguments);
-    let tree = this.model;
 
     // Make sure chosen item is highlighted and expanded-to in the tree
-    let chosenId = this.chosenId;
-    if (chosenId) {
-      let chosen = getDescendents(tree).findBy('id', chosenId);
+    if (this.chosenId) {
+      let chosen = getDescendents(this.model).findBy('id', this.chosenId);
       if (chosen) {
-        getAncestors(tree, chosen).forEach(x => {
-          if (x.id !== chosenId) {
+        getAncestors(this.model, chosen).forEach(x => {
+          if (get(x, 'id') !== this.chosenId) {
             set(x, 'isExpanded', true);
           }
         });
       }
     }
 
     // Expand to given depth
-    let expandDepth = this.expandDepth;
-    if (expandDepth) {
-      getDescendents(tree, expandDepth).setEach('isExpanded', true);
+    if (this.expandDepth) {
+      getDescendents(this.model, this.expandDepth).setEach('isExpanded', true);
     }
   }
 });

addon/utils/tree.js

@@ -31,16 +31,16 @@ export function buildTree(model, options = {}) {
 
     // Defaults
     set(node, 'children', A());
-    set(node, 'isVisible', node.isVisible || true);
-    set(node, 'isExpanded', node.isExpanded || false);
+    set(node, 'isVisible', get(node, 'isVisible') || true);
+    set(node, 'isExpanded', get(node, 'isExpanded') || false);
 
-    tree[node.id] = node;
+    tree[get(node, 'id')] = node;
   });
 
   // Connect all children to their parent
   model.forEach(node => {
     let child = tree[get(node, options.valueKey || 'id')];
-    let parent = node.parentId;
+    let parent = get(node, 'parentId');
 
     if (isEmpty(parent)) {
       roots.pushObject(child);
@@ -59,12 +59,12 @@ export function getDescendents(tree, depth = -1) {
   if (depth < 0) { // Unlimited depth
     tree.forEach(node => {
       descendents.pushObject(node);
-      descendents.pushObjects(getDescendents(node.children));
+      descendents.pushObjects(getDescendents(get(node, 'children')));
     });
   } else if (depth > 0) {
     tree.forEach(node => {
       descendents.pushObject(node);
-      descendents.pushObjects(getDescendents(node.children, depth - 1));
+      descendents.pushObjects(getDescendents(get(node, 'children'), depth - 1));
     });
   }
 
@@ -74,14 +74,14 @@ export function getDescendents(tree, depth = -1) {
 // Returns a flat list of ancestors, including the child
 export function getAncestors(tree, childNode) {
   let ancestors = A();
-  let childId = childNode.id;
+  let childId = get(childNode, 'id');
 
   tree.forEach(node => {
     if (!ancestors.isAny('id', childId)) {
-      if (node.id === childId) {
+      if (get(node, 'id') === childId) {
         ancestors.pushObject(node);
-      } else if (node.children.length > 0) {
-        ancestors.pushObjects(getAncestors(node.children, childNode));
+      } else if (get(node, 'children.length') > 0) {
+        ancestors.pushObjects(getAncestors(get(node, 'children'), childNode));
         if (ancestors.length > 0) {
           ancestors.pushObject(node);
         }
@@ -94,5 +94,5 @@ export function getAncestors(tree, childNode) {
 
 // Returns the parent of a child
 export function getParent(list, childNode) {
-  return list.find(x => x.children.find(y => y.id === childNode.id));
+  return list.find(x => x.children.find(y => y.id === get(childNode, 'id')));
 }