fix(button-toggle): remove emit change event when value changes (#6034)

Author: tinayuangao

src/lib/button-toggle/button-toggle.spec.ts

@@ -87,10 +87,11 @@ describe('MdButtonToggle with forms', () => {
     let buttonToggleInstances: MdButtonToggle[];
     let testComponent: ButtonToggleGroupWithNgModel;
     let groupNgModel: NgModel;
+    let buttonToggleLabels: HTMLElement[];
 
     beforeEach(async(() => {
       fixture = TestBed.createComponent(ButtonToggleGroupWithNgModel);
-
+      fixture.detectChanges();
       testComponent = fixture.debugElement.componentInstance;
 
       groupDebugElement = fixture.debugElement.query(By.directive(MdButtonToggleGroup));
@@ -102,6 +103,8 @@ describe('MdButtonToggle with forms', () => {
       buttonToggleNativeElements =
         buttonToggleDebugElements.map(debugEl => debugEl.nativeElement);
       buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance);
+      buttonToggleLabels = buttonToggleDebugElements.map(
+        debugEl => debugEl.query(By.css('label')).nativeElement);
 
       fixture.detectChanges();
     }));
@@ -110,42 +113,13 @@ describe('MdButtonToggle with forms', () => {
       expect(testComponent.modelValue).toBeUndefined();
       expect(testComponent.lastEvent).toBeUndefined();
 
-      groupInstance.value = 'red';
+      buttonToggleLabels[0].click();
       fixture.detectChanges();
 
       tick();
       expect(testComponent.modelValue).toBe('red');
       expect(testComponent.lastEvent.value).toBe('red');
     }));
-  });
-
-  describe('button toggle group with ngModel', () => {
-    let fixture: ComponentFixture<ButtonToggleGroupWithNgModel>;
-    let groupDebugElement: DebugElement;
-    let groupNativeElement: HTMLElement;
-    let buttonToggleDebugElements: DebugElement[];
-    let buttonToggleNativeElements: HTMLElement[];
-    let groupInstance: MdButtonToggleGroup;
-    let buttonToggleInstances: MdButtonToggle[];
-    let testComponent: ButtonToggleGroupWithNgModel;
-    let groupNgModel: NgModel;
-
-    beforeEach(async(() => {
-      fixture = TestBed.createComponent(ButtonToggleGroupWithNgModel);
-      fixture.detectChanges();
-
-      testComponent = fixture.debugElement.componentInstance;
-
-      groupDebugElement = fixture.debugElement.query(By.directive(MdButtonToggleGroup));
-      groupNativeElement = groupDebugElement.nativeElement;
-      groupInstance = groupDebugElement.injector.get<MdButtonToggleGroup>(MdButtonToggleGroup);
-      groupNgModel = groupDebugElement.injector.get<NgModel>(NgModel);
-
-      buttonToggleDebugElements = fixture.debugElement.queryAll(By.directive(MdButtonToggle));
-      buttonToggleNativeElements =
-        buttonToggleDebugElements.map(debugEl => debugEl.nativeElement);
-      buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance);
-    }));
 
     it('should set individual radio names based on the group name', () => {
       expect(groupInstance.name).toBeTruthy();
@@ -183,11 +157,10 @@ describe('MdButtonToggle with forms', () => {
         tick();
 
         expect(groupNgModel.valid).toBe(true);
-        expect(groupNgModel.pristine).toBe(false);
+        expect(groupNgModel.pristine).toBe(true);
         expect(groupNgModel.touched).toBe(false);
 
-        let nativeRadioLabel = buttonToggleDebugElements[2].query(By.css('label')).nativeElement;
-        nativeRadioLabel.click();
+        buttonToggleLabels[2].click();
         fixture.detectChanges();
         tick();
 
@@ -197,7 +170,7 @@ describe('MdButtonToggle with forms', () => {
       }));
 
     it('should update the ngModel value when selecting a button toggle', fakeAsync(() => {
-      buttonToggleInstances[1].checked = true;
+      buttonToggleLabels[1].click();
       fixture.detectChanges();
 
       tick();
@@ -276,9 +249,7 @@ describe('MdButtonToggle without forms', () => {
 
     it('should update the group value when one of the toggles changes', () => {
       expect(groupInstance.value).toBeFalsy();
-      let nativeCheckboxLabel = buttonToggleDebugElements[0].query(By.css('label')).nativeElement;
-
-      nativeCheckboxLabel.click();
+      buttonToggleLabelElements[0].click();
       fixture.detectChanges();
 
       expect(groupInstance.value).toBe('test1');
@@ -287,19 +258,15 @@ describe('MdButtonToggle without forms', () => {
 
     it('should update the group and toggles when one of the button toggles is clicked', () => {
       expect(groupInstance.value).toBeFalsy();
-      let nativeCheckboxLabel = buttonToggleDebugElements[0].query(By.css('label')).nativeElement;
-
-      nativeCheckboxLabel.click();
+      buttonToggleLabelElements[0].click();
       fixture.detectChanges();
 
       expect(groupInstance.value).toBe('test1');
       expect(groupInstance.selected).toBe(buttonToggleInstances[0]);
       expect(buttonToggleInstances[0].checked).toBe(true);
       expect(buttonToggleInstances[1].checked).toBe(false);
 
-      let nativeCheckboxLabel2 = buttonToggleDebugElements[1].query(By.css('label')).nativeElement;
-
-      nativeCheckboxLabel2.click();
+      buttonToggleLabelElements[1].click();
       fixture.detectChanges();
 
       expect(groupInstance.value).toBe('test2');
@@ -309,9 +276,7 @@ describe('MdButtonToggle without forms', () => {
     });
 
     it('should check a button toggle upon interaction with underlying native radio button', () => {
-      let nativeRadioInput = buttonToggleDebugElements[0].query(By.css('input')).nativeElement;
-
-      nativeRadioInput.click();
+      buttonToggleLabelElements[0].click();
       fixture.detectChanges();
 
       expect(buttonToggleInstances[0].checked).toBe(true);
@@ -354,12 +319,12 @@ describe('MdButtonToggle without forms', () => {
       let changeSpy = jasmine.createSpy('button-toggle-group change listener');
       groupInstance.change.subscribe(changeSpy);
 
-      groupInstance.value = 'test1';
+      buttonToggleLabelElements[0].click();
       fixture.detectChanges();
       tick();
       expect(changeSpy).toHaveBeenCalled();
 
-      groupInstance.value = 'test2';
+      buttonToggleLabelElements[1].click();
       fixture.detectChanges();
       tick();
       expect(changeSpy).toHaveBeenCalledTimes(2);
@@ -414,7 +379,7 @@ describe('MdButtonToggle without forms', () => {
       fixture.detectChanges();
 
       expect(groupInstance.value).toBe('green');
-      expect(testComponent.lastEvent.value).toBe('green');
+      expect(testComponent.lastEvent).toBeFalsy();
     });
 
   });

src/lib/button-toggle/button-toggle.ts

@@ -23,7 +23,6 @@ import {
   ViewChild,
   ViewEncapsulation,
   forwardRef,
-  AfterViewInit,
   ChangeDetectionStrategy,
   ChangeDetectorRef,
 } from '@angular/core';
@@ -72,8 +71,8 @@ export class MdButtonToggleChange {
   },
   exportAs: 'mdButtonToggleGroup',
 })
-export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implements AfterViewInit,
-    ControlValueAccessor, CanDisable {
+export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase
+    implements ControlValueAccessor, CanDisable {
 
   /** The value for the button toggle group. Should match currently selected button toggle. */
   private _value: any = null;
@@ -87,25 +86,18 @@ export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implement
   /** The currently selected button toggle, should match the value. */
   private _selected: MdButtonToggle | null = null;
 
-  /** Whether the button toggle group is initialized or not. */
-  private _isInitialized: boolean = false;
-
   /**
    * The method to be called in order to update ngModel.
    * Now `ngModel` binding is not supported in multiple selection mode.
    */
-  private _controlValueAccessorChangeFn: (value: any) => void = () => {};
+  _controlValueAccessorChangeFn: (value: any) => void = () => {};
 
   /** onTouch function registered via registerOnTouch (ControlValueAccessor). */
   onTouched: () => any = () => {};
 
   /** Child button toggle buttons. */
   @ContentChildren(forwardRef(() => MdButtonToggle)) _buttonToggles: QueryList<MdButtonToggle>;
 
-  ngAfterViewInit() {
-    this._isInitialized = true;
-  }
-
   /** `name` attribute for the underlying `input` element. */
   @Input()
   get name(): string {
@@ -132,18 +124,11 @@ export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implement
   get value(): any {
     return this._value;
   }
-
   set value(newValue: any) {
     if (this._value != newValue) {
       this._value = newValue;
 
       this._updateSelectedButtonToggleFromValue();
-
-      // Only emit a change event if the view is completely initialized.
-      // We don't want to emit a change event for the initial values.
-      if (this._isInitialized) {
-        this._emitChangeEvent();
-      }
     }
   }
 
@@ -165,6 +150,10 @@ export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implement
   /** Event emitted when the group's value changes. */
   @Output() change: EventEmitter<MdButtonToggleChange> = new EventEmitter<MdButtonToggleChange>();
 
+  constructor(private _changeDetector: ChangeDetectorRef) {
+    super();
+  }
+
   private _updateButtonToggleNames(): void {
     if (this._buttonToggles) {
       this._buttonToggles.forEach((toggle) => {
@@ -193,7 +182,7 @@ export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implement
   }
 
   /** Dispatch change event with current selection and group value. */
-  private _emitChangeEvent(): void {
+  _emitChangeEvent(): void {
     let event = new MdButtonToggleChange();
     event.source = this._selected;
     event.value = this._value;
@@ -207,6 +196,7 @@ export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implement
    */
   writeValue(value: any) {
     this.value = value;
+    this._changeDetector.markForCheck();
   }
 
   /**
@@ -438,9 +428,13 @@ export class MdButtonToggle implements OnInit, OnDestroy {
     if (this._isSingleSelector) {
       // Propagate the change one-way via the group, which will in turn mark this
       // button toggle as checked.
+      let groupValueChanged = this.buttonToggleGroup.selected != this;
       this.checked = true;
       this.buttonToggleGroup.selected = this;
       this.buttonToggleGroup.onTouched();
+      if (groupValueChanged) {
+        this.buttonToggleGroup._emitChangeEvent();
+      }
     } else {
       this._toggle();
     }