refactor: code review

ng-select · Jun 20, 2018 · b961806 · b961806
1 parent 6f6be0f
commit b961806
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 43 deletions.
diff --git a/src/ng-select/items-list.ts b/src/ng-select/items-list.ts
@@ -72,7 +72,7 @@ export class ItemsList {
 
         this._selectionModel.select(item, multiple);
         if (this._ngSelect.hideSelected && multiple) {
-            this._hideSelectedItems(item);
+            this._hideSelected(item);
         }
     }
 
@@ -82,7 +82,7 @@ export class ItemsList {
         }
         this._selectionModel.unselect(item, this._ngSelect.multiple);
         if (this._ngSelect.hideSelected && isDefined(item.index) && this._ngSelect.multiple) {
-            this._showSelectedItems(item);
+            this._showSelected(item);
         }
     }
 
@@ -250,46 +250,35 @@ export class ItemsList {
         }
     }
 
-    private _showSelectedItems(current: NgOption) {
-        this._filteredItems.splice(current.index, 0, current);
-        if (current.parent) {
-            const parent = current.parent;
-            const alreadyAdded = this._filteredItems.find(x => x === parent);
-            if (!alreadyAdded) {
-                this._filteredItems.splice(parent.index, 0, parent);
+    private _showSelected(item: NgOption) {
+        this._filteredItems.push(item);
+        if (item.parent) {
+            const parent = item.parent;
+            const parentExists = this._filteredItems.find(x => x === parent);
+            if (!parentExists) {
+                this._filteredItems.push(parent);
             }
-        } else if (current.children) {
-            for (const child of current.children) {
+        } else if (item.children) {
+            for (const child of item.children) {
                 child.selected = false;
-                this._filteredItems.splice(child.index, 0, child);
+                this._filteredItems.push(child);
             }
         }
         this._filteredItems = [...this._filteredItems.sort((a, b) => (a.index - b.index))];
     }
 
-    private _hideSelectedItems(current: NgOption) {
-        this._filteredItems = this._filteredItems.filter(x => x !== current);
-        if (current.parent) {
-            const children = current.parent.children;
-            const selectedChildrenCount = this._countItems(this.selectedItems, x => children.indexOf(x) > -1);
-            if (children.length === selectedChildrenCount) {
-                this._filteredItems = this._filteredItems.filter(x => x !== current.parent);
+    private _hideSelected(item: NgOption) {
+        this._filteredItems = this._filteredItems.filter(x => x !== item);
+        if (item.parent) {
+            const children = item.parent.children;
+            if (children.every(x => x.selected)) {
+                this._filteredItems = this._filteredItems.filter(x => x !== item.parent);
             }
-        } else if (current.children) {
-            this._filteredItems = this.filteredItems.filter(x => x.parent !== current);
+        } else if (item.children) {
+            this._filteredItems = this.filteredItems.filter(x => x.parent !== item);
         }
     }
 
-    private _countItems(items: NgOption[], predicate: (item: NgOption) => boolean) {
-        let count = 0;
-        for (const item of items) {
-            if (predicate(item)) {
-                count++;
-            }
-        }   
-        return count;
-    }
-
     private _defaultSearchFn(search: string, opt: NgOption) {
         const label = searchHelper.stripSpecialChars(opt.label).toLocaleLowerCase();
         return label.indexOf(search) > -1

diff --git a/src/ng-select/selection-model.ts b/src/ng-select/selection-model.ts
@@ -13,13 +13,12 @@ export class SelectionModel {
         if (multiple) {
             if (item.parent) {
                 this._removeParent(item.parent);
-                const childrenCount = item.parent.children ? item.parent.children.length : 0;
-                const selectedCount = this._selected.filter(x => x.parent === item.parent).length;
+                const childrenCount = item.parent.children.length;
+                const selectedCount = item.parent.children.filter(x => x.selected).length;
                 item.parent.selected = childrenCount === selectedCount;
             } else if (item.children) {
-                const children = item.children;
-                this._setChildrenSelectedState(children, true);
-                this._removeSelectedChildren(children);
+                this._setChildrenSelectedState(item.children, true);
+                this._removeParentChildren(item);
             }
         }
     }
@@ -31,13 +30,12 @@ export class SelectionModel {
             if (item.parent && item.parent.selected) {
                 const children = item.parent.children;
                 this._removeParent(item.parent);
-                this._removeSelectedChildren(children);
+                this._removeParentChildren(item.parent);
                 this._selected.push(...children.filter(x => x !== item));
                 item.parent.selected = false;
             } else if (item.children) {
-                const children = item.children
-                this._setChildrenSelectedState(children, false);
-                this._removeSelectedChildren(children);
+                this._setChildrenSelectedState(item.children, false);
+                this._removeParentChildren(item);
             }
         }
     }
@@ -47,11 +45,11 @@ export class SelectionModel {
     }
 
     private _setChildrenSelectedState(children: NgOption[], selected: boolean) {
-        children.forEach(c => c.selected = selected);
+        children.forEach(x => x.selected = selected);
     }
 
-    private _removeSelectedChildren(children: NgOption[]) {
-        this._selected = this._selected.filter(x => children.indexOf(x) === -1);
+    private _removeParentChildren(parent: NgOption) {
+        this._selected = this._selected.filter(x => x.parent !== parent);
     }
 
     private _removeParent(parent: NgOption) {