From 6ff331abcf5fd528ef7a2cc3df0a35f4b57e95a4 Mon Sep 17 00:00:00 2001
From: Steven Sojka <steelsojka@gmail.com>
Date: Thu, 14 Sep 2017 18:58:11 -0500
Subject: [PATCH] feat(accordion): make isOpen two way bindable

---
 src/accordion/accordion-group.component.ts | 17 ++++++---
 src/accordion/accordion.component.ts       |  2 +-
 src/spec/accordion.component.spec.ts       | 41 ++++++++++------------
 3 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/src/accordion/accordion-group.component.ts b/src/accordion/accordion-group.component.ts
index 09307dd44a..40794054b0 100644
--- a/src/accordion/accordion-group.component.ts
+++ b/src/accordion/accordion-group.component.ts
@@ -30,7 +30,7 @@ export class AccordionPanelComponent implements OnInit, OnDestroy {
   /** if <code>true</code> — disables accordion group */
   @Input() isDisabled: boolean;
   /** Emits when the opened state changes */
-  @Output() public isOpenChanges: EventEmitter<boolean> = new EventEmitter();
+  @Output() isOpenChange: EventEmitter<boolean> = new EventEmitter();
 
   // Questionable, maybe .panel-open should be on child div.panel element?
   /** Is accordion group open or closed */
@@ -42,8 +42,6 @@ export class AccordionPanelComponent implements OnInit, OnDestroy {
 
   set isOpen(value: boolean) {
     this._isOpen = value;
-    this.isOpenChanges.emit(this.isOpen);
-
     if (value) {
       this.accordion.closeOtherPanels(this);
     }
@@ -71,7 +69,18 @@ export class AccordionPanelComponent implements OnInit, OnDestroy {
 
   toggleOpen(event: Event): any {
     if (!this.isDisabled) {
-      this.isOpen = !this.isOpen;
+      this.setOpenState(!this.isOpen);
+    }
+  }
+
+  /**
+   * Sets the isOpen state internally
+   * @param {boolean} isOpen
+   */
+  setOpenState(isOpen: boolean): void {
+    if (isOpen !== this._isOpen) {
+      this._isOpen = isOpen;
+      this.isOpenChange.emit(this._isOpen);
     }
   }
 }
diff --git a/src/accordion/accordion.component.ts b/src/accordion/accordion.component.ts
index 140f3a2b6b..347559f3d7 100644
--- a/src/accordion/accordion.component.ts
+++ b/src/accordion/accordion.component.ts
@@ -30,7 +30,7 @@ export class AccordionComponent {
 
     this.groups.forEach((group: AccordionPanelComponent) => {
       if (group !== openGroup) {
-        group.isOpen = false;
+        group.setOpenState(false);
       }
     });
   }
diff --git a/src/spec/accordion.component.spec.ts b/src/spec/accordion.component.spec.ts
index 3478c93466..36c7770369 100644
--- a/src/spec/accordion.component.spec.ts
+++ b/src/spec/accordion.component.spec.ts
@@ -11,9 +11,9 @@ import { AccordionModule } from '../accordion/accordion.module';
 class TestAccordionComponent {
   oneAtATime = true;
   panels: any[] = [
-    {isOpen: false, isDisabled: false, isOpenChangesValue: null},
-    {isOpen: false, isDisabled: false, isOpenChangesValue: null},
-    {isOpen: false, isDisabled: false, isOpenChangesValue: null}
+    {isOpen: false, isDisabled: false},
+    {isOpen: false, isDisabled: false},
+    {isOpen: false, isDisabled: false}
   ];
 
   constructor(config: AccordionConfig) {
@@ -25,23 +25,20 @@ const html = `
   <accordion [closeOthers]="oneAtATime">
 
     <accordion-group heading="Panel 1"
-                     [isOpen]="panels[0].isOpen"
-                     [isDisabled]="panels[0].isDisabled"
-                     (isOpenChanges)="panels[0].isOpenChangesValue = $event">
+                     [(isOpen)]="panels[0].isOpen"
+                     [isDisabled]="panels[0].isDisabled">
       Content of panel 1
     </accordion-group>
 
     <accordion-group heading="Panel 2"
-                     [isOpen]="panels[1].isOpen"
-                     [isDisabled]="panels[1].isDisabled"
-                     (isOpenChanges)="panels[1].isOpenChangesValue = $event">
+                     [(isOpen)]="panels[1].isOpen"
+                     [isDisabled]="panels[1].isDisabled">
       Content of panel 2
     </accordion-group>
 
     <accordion-group heading="Panel 3"
-                     [isOpen]="panels[2].isOpen"
-                     [isDisabled]="panels[2].isDisabled"
-                     (isOpenChanges)="panels[2].isOpenChangesValue = $event">
+                     [(isOpen)]="panels[2].isOpen"
+                     [isDisabled]="panels[2].isDisabled">
       Content of panel 3
     </accordion-group>
 
@@ -170,28 +167,28 @@ describe('Component: Accordion', () => {
     expectOpenPanels(element, [false, false, false]);
   });
 
-  it('should output the open state when it is changed internally', () => {
+  it('should modify the parent isOpen state when changed internally (2 way binding)', () => {
     const headingLinks = element.querySelectorAll('.accordion-toggle');
 
     // Clicking (internal state modified)
     headingLinks[0].click();
     fixture.detectChanges();
-    expect(context.panels[0].isOpenChangesValue).toBe(true);
-    expect(context.panels[1].isOpenChangesValue).toBe(false);
-    expect(context.panels[2].isOpenChangesValue).toBe(false);
+    expect(context.panels[0].isOpen).toBe(true);
+    expect(context.panels[1].isOpen).toBe(false);
+    expect(context.panels[2].isOpen).toBe(false);
 
     // State modified by parent component
     headingLinks[2].click();
     fixture.detectChanges();
-    expect(context.panels[0].isOpenChangesValue).toBe(false);
-    expect(context.panels[1].isOpenChangesValue).toBe(false);
-    expect(context.panels[2].isOpenChangesValue).toBe(true);
+    expect(context.panels[0].isOpen).toBe(false);
+    expect(context.panels[1].isOpen).toBe(false);
+    expect(context.panels[2].isOpen).toBe(true);
 
     // Modified by binding
     context.panels[1].isOpen = true;
     fixture.detectChanges();
-    expect(context.panels[0].isOpenChangesValue).toBe(false);
-    expect(context.panels[1].isOpenChangesValue).toBe(true);
-    expect(context.panels[2].isOpenChangesValue).toBe(false);
+    expect(context.panels[0].isOpen).toBe(false);
+    expect(context.panels[1].isOpen).toBe(true);
+    expect(context.panels[2].isOpen).toBe(false);
   });
 });