From 1932e65d616a10bf6c14b6d5283db50dad99161d Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 31 Aug 2016 20:38:18 +0200 Subject: [PATCH 1/4] fix(button-toggle): button-toggle-group should not emit an initial change event. References #1125 --- src/lib/button-toggle/button-toggle.spec.ts | 40 +++++++++++++++++++-- src/lib/button-toggle/button-toggle.ts | 20 +++++++++-- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/lib/button-toggle/button-toggle.spec.ts b/src/lib/button-toggle/button-toggle.spec.ts index 569745937955..2cb11fdcb9f1 100644 --- a/src/lib/button-toggle/button-toggle.spec.ts +++ b/src/lib/button-toggle/button-toggle.spec.ts @@ -25,6 +25,7 @@ describe('MdButtonToggle', () => { ButtonTogglesInsideButtonToggleGroup, ButtonToggleGroupWithNgModel, ButtonTogglesInsideButtonToggleGroupMultiple, + ButtonToggleGroupWithInitialValue, StandaloneButtonToggle, ], }); @@ -305,11 +306,11 @@ describe('MdButtonToggle', () => { buttonToggleNativeElements = buttonToggleDebugElements.map(debugEl => debugEl.nativeElement); buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance); - - fixture.detectChanges(); })); it('should update the model before firing change event', fakeAsync(() => { + fixture.detectChanges(); + expect(testComponent.modelValue).toBeUndefined(); expect(testComponent.lastEvent).toBeUndefined(); @@ -320,6 +321,29 @@ describe('MdButtonToggle', () => { expect(testComponent.modelValue).toBe('red'); expect(testComponent.lastEvent.value).toBe('red'); })); + + }); + + describe('with initial value and change event', () => { + + it('should not fire an initial change event', async(() => { + let fixture = TestBed.createComponent(ButtonToggleGroupWithInitialValue); + let testComponent = fixture.debugElement.componentInstance; + let groupDebugElement = fixture.debugElement.query(By.directive(MdButtonToggleGroup)); + let groupInstance: MdButtonToggleGroup = groupDebugElement.injector.get(MdButtonToggleGroup); + + fixture.detectChanges(); + + expect(groupInstance.value).toBe('red'); + expect(testComponent.lastEvent).toBeFalsy(); + + groupInstance.value = 'green'; + fixture.detectChanges(); + + expect(groupInstance.value).toBe('green'); + expect(testComponent.lastEvent.value).toBe('green'); + })); + }); describe('inside of a multiple selection group', () => { @@ -532,3 +556,15 @@ class ButtonTogglesInsideButtonToggleGroupMultiple { ` }) class StandaloneButtonToggle { } + +@Component({ + template: ` + + Value Red + Value Green + + ` +}) +class ButtonToggleGroupWithInitialValue { + lastEvent: MdButtonToggleChange; +} diff --git a/src/lib/button-toggle/button-toggle.ts b/src/lib/button-toggle/button-toggle.ts index ca6b2058ff13..a2ce4ae6306f 100644 --- a/src/lib/button-toggle/button-toggle.ts +++ b/src/lib/button-toggle/button-toggle.ts @@ -12,7 +12,8 @@ import { Output, QueryList, ViewEncapsulation, - forwardRef + forwardRef, + AfterViewInit } from '@angular/core'; import { NG_VALUE_ACCESSOR, @@ -55,7 +56,7 @@ export class MdButtonToggleChange { 'role': 'radiogroup', }, }) -export class MdButtonToggleGroup implements ControlValueAccessor { +export class MdButtonToggleGroup implements AfterViewInit, ControlValueAccessor { /** The value for the button toggle group. Should match currently selected button toggle. */ private _value: any = null; @@ -68,6 +69,9 @@ export class MdButtonToggleGroup implements ControlValueAccessor { /** The currently selected button toggle, should match the value. */ private _selected: MdButtonToggle = null; + /** Whether the button toggle group is initialized or not. */ + private _isInitialized: boolean = false; + /** The method to be called in order to update ngModel. */ private _controlValueAccessorChangeFn: (value: any) => void = (value) => {}; @@ -84,6 +88,11 @@ export class MdButtonToggleGroup implements ControlValueAccessor { @ContentChildren(forwardRef(() => MdButtonToggle)) _buttonToggles: QueryList = null; + /** TODO: internal */ + ngAfterViewInit() { + this._isInitialized = true; + } + @Input() get name(): string { return this._name; @@ -114,7 +123,12 @@ export class MdButtonToggleGroup implements ControlValueAccessor { this._value = newValue; this._updateSelectedButtonToggleFromValue(); - this._emitChangeEvent(); + + // 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(); + } } } From 949fd253fe1bb6a1d87c35b3b9f0d20705687df1 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 31 Aug 2016 23:24:52 +0200 Subject: [PATCH 2/4] Use NgOnInit lifecycle hook instead --- src/lib/button-toggle/button-toggle.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lib/button-toggle/button-toggle.ts b/src/lib/button-toggle/button-toggle.ts index a2ce4ae6306f..adb056ab8cca 100644 --- a/src/lib/button-toggle/button-toggle.ts +++ b/src/lib/button-toggle/button-toggle.ts @@ -12,8 +12,7 @@ import { Output, QueryList, ViewEncapsulation, - forwardRef, - AfterViewInit + forwardRef } from '@angular/core'; import { NG_VALUE_ACCESSOR, @@ -56,7 +55,7 @@ export class MdButtonToggleChange { 'role': 'radiogroup', }, }) -export class MdButtonToggleGroup implements AfterViewInit, ControlValueAccessor { +export class MdButtonToggleGroup implements OnInit, ControlValueAccessor { /** The value for the button toggle group. Should match currently selected button toggle. */ private _value: any = null; @@ -89,7 +88,7 @@ export class MdButtonToggleGroup implements AfterViewInit, ControlValueAccessor _buttonToggles: QueryList = null; /** TODO: internal */ - ngAfterViewInit() { + ngOnInit() { this._isInitialized = true; } From 4a58aeb51fb4e0d90c15d90b9760d6a07b4a5db3 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 2 Sep 2016 03:00:51 +0200 Subject: [PATCH 3/4] Use ngAfterViewInit due to possibly unregistered directives on the component. --- src/lib/button-toggle/button-toggle.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lib/button-toggle/button-toggle.ts b/src/lib/button-toggle/button-toggle.ts index adb056ab8cca..a2ce4ae6306f 100644 --- a/src/lib/button-toggle/button-toggle.ts +++ b/src/lib/button-toggle/button-toggle.ts @@ -12,7 +12,8 @@ import { Output, QueryList, ViewEncapsulation, - forwardRef + forwardRef, + AfterViewInit } from '@angular/core'; import { NG_VALUE_ACCESSOR, @@ -55,7 +56,7 @@ export class MdButtonToggleChange { 'role': 'radiogroup', }, }) -export class MdButtonToggleGroup implements OnInit, ControlValueAccessor { +export class MdButtonToggleGroup implements AfterViewInit, ControlValueAccessor { /** The value for the button toggle group. Should match currently selected button toggle. */ private _value: any = null; @@ -88,7 +89,7 @@ export class MdButtonToggleGroup implements OnInit, ControlValueAccessor { _buttonToggles: QueryList = null; /** TODO: internal */ - ngOnInit() { + ngAfterViewInit() { this._isInitialized = true; } From 31728c156a36a4785de2e0fa3726d3a3a9bc7e3d Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 2 Sep 2016 03:25:06 +0200 Subject: [PATCH 4/4] Remove unecessary change and trigger CI rerun. --- src/lib/button-toggle/button-toggle.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/button-toggle/button-toggle.spec.ts b/src/lib/button-toggle/button-toggle.spec.ts index 2cb11fdcb9f1..ab93e493c71f 100644 --- a/src/lib/button-toggle/button-toggle.spec.ts +++ b/src/lib/button-toggle/button-toggle.spec.ts @@ -306,11 +306,11 @@ describe('MdButtonToggle', () => { buttonToggleNativeElements = buttonToggleDebugElements.map(debugEl => debugEl.nativeElement); buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance); - })); - it('should update the model before firing change event', fakeAsync(() => { fixture.detectChanges(); + })); + it('should update the model before firing change event', fakeAsync(() => { expect(testComponent.modelValue).toBeUndefined(); expect(testComponent.lastEvent).toBeUndefined();