Added JDocs for changeable-helper classes, Renamed functions for clarity

Author: xjerwa

libs/fabric/src/lib/components/contextual-menu/directives/contextual-menu-item.directive.ts

@@ -117,11 +117,10 @@ export class ContextualMenuItemDirective extends ChangeableItemDirective<IContex
         this.subMenuProps.items = items;
       }
     });
-    this.changeableItemsHelper.afterContentInit();
   }
 
   ngOnDestroy() {
-    this.changeableItemsHelper.onDestroy();
+    this.changeableItemsHelper.destroy();
   }
 
   private _directiveToContextualMenuItem(directive: ContextualMenuItemDirective): IContextualMenuItem {

libs/fabric/src/lib/components/core/shared/changeable-helper.ts

@@ -16,7 +16,12 @@ export class ChangeableItemHelper<TItem> {
 
   constructor(private readonly key: string) {}
 
-  onChanges(changes: TypedChanges<TItem>) {
+  /**
+   * Action to be called by user of class when change is detected
+   * (Typically called in ngOnChanges)
+   * @param changes TypedChanges that are to be emitted
+   */
+  emitChanges(changes: TypedChanges<TItem>) {
     this.onItemChanged.emit({ key: this.key, changes });
   }
 }
@@ -31,22 +36,32 @@ export class ChangeableItemsHelper<TItem> {
   private readonly _subscriptionsMap: { [key: string]: Subscription } = {};
   private _changeSubscription: Subscription;
 
+  /**
+   * Initialize subscriptions to watch for changes to children ChangeableItemDirectives
+   * (Typically called in ngAfterContentInit after @ContentChildren are initialized)
+   * @param directiveItems List of children to watch for
+   * @param self Reference to component using this helper. This component gets filtered out in case
+   *  it appears in the list of children (i.e. when a component has children of its own type)
+   * @param nonSelfHandler Callback to handle filtered list of children when updated
+   */
   constructor(
     private directiveItems: QueryList<ChangeableItemDirective<TItem>>,
     private self?: IChangeableItemsContainer<TItem>,
     private nonSelfHandler?: (nonSelfDirectives: ChangeableItemDirective<TItem>[]) => void
-  ) {}
-
-  afterContentInit() {
+  ) {
     this._subscribeNewDirectives();
     this._changeSubscription = this.directiveItems.changes.subscribe(newValues => {
       this.onItemsChanged.emit(newValues);
       this._subscribeNewDirectives();
     });
   }
 
-  onDestroy() {
-    Object.keys(this._subscriptionsMap).forEach(key => this._subscriptionsMap[key].unsubscribe());
+  /**
+   * Action to be called by user of class when directive/component is destroyed
+   * (Typically called in ngOnDestroy)
+   */
+  destroy() {
+    Object.values(this._subscriptionsMap).forEach(value => value.unsubscribe());
     this._changeSubscription.unsubscribe();
   }
 

libs/fabric/src/lib/components/core/shared/changeable-item.directive.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-import { EventEmitter, Input, Output } from '@angular/core';
+import { EventEmitter, Input, OnInit, Output } from '@angular/core';
 
 import { OnChanges, TypedChanges } from '../../../declarations/angular/typed-changes';
 import { ItemChangedPayload } from '../../core/declarative/item-changed.payload';
@@ -10,7 +10,7 @@ import { ChangeableItemHelper } from './changeable-helper';
 /**
  * Parent class for wrapper directive for single item with OnChanges
  */
-export abstract class ChangeableItemDirective<TItem> implements OnChanges<ChangeableItemDirective<TItem>> {
+export abstract class ChangeableItemDirective<TItem> implements OnChanges<ChangeableItemDirective<TItem>>, OnInit {
   @Input()
   key: string;
 
@@ -27,7 +27,7 @@ export abstract class ChangeableItemDirective<TItem> implements OnChanges<Change
 
   ngOnChanges(changes: TypedChanges<TItem>) {
     if (this.changeableItemHelper) {
-      this.changeableItemHelper.onChanges(changes);
+      this.changeableItemHelper.emitChanges(changes);
     }
   }
 }

libs/fabric/src/lib/components/core/shared/changeable-items.directive.ts

@@ -30,10 +30,9 @@ export abstract class ChangeableItemsDirective<TItem>
 
   ngAfterContentInit() {
     this.changeableItemsHelper = new ChangeableItemsHelper(this.directiveItems);
-    this.changeableItemsHelper.afterContentInit();
   }
 
   ngOnDestroy() {
-    this.changeableItemsHelper.onDestroy();
+    this.changeableItemsHelper.destroy();
   }
 }

libs/fabric/src/lib/components/group/directives/group-item.directive.ts

@@ -19,7 +19,7 @@ import { ChangeableItemDirective } from '../../core/shared/changeable-item.direc
 
 @Directive({ selector: 'fab-group-item' })
 export class GroupItemDirective extends ChangeableItemDirective<IGroup>
-  implements AfterContentInit, IChangeableItemsContainer<IGroup>, OnDestroy {
+  implements AfterContentInit, IChangeableItemsContainer<IGroup>, IGroup, OnDestroy {
   @ContentChildren(GroupItemDirective)
   readonly groupItemsDirectives: QueryList<GroupItemDirective>;
 
@@ -61,10 +61,9 @@ export class GroupItemDirective extends ChangeableItemDirective<IGroup>
     this.changeableItemsHelper = new ChangeableItemsHelper(this.groupItemsDirectives, this, nonSelf => {
       this.children = nonSelf as any;
     });
-    this.changeableItemsHelper.afterContentInit();
   }
 
   ngOnDestroy() {
-    this.changeableItemsHelper.onDestroy();
+    this.changeableItemsHelper.destroy();
   }
 }