From 283e0ba38b810cca0f12ccd466955acdb8336cf4 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 20 Jul 2017 19:41:15 -0700 Subject: [PATCH] remove emit change event when value changes --- src/lib/button-toggle/button-toggle.spec.ts | 65 +++++---------------- src/lib/button-toggle/button-toggle.ts | 31 +++++----- 2 files changed, 28 insertions(+), 68 deletions(-) diff --git a/src/lib/button-toggle/button-toggle.spec.ts b/src/lib/button-toggle/button-toggle.spec.ts index b0c1971a4b18..48fd73fe1b28 100644 --- a/src/lib/button-toggle/button-toggle.spec.ts +++ b/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; - 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); - groupNgModel = groupDebugElement.injector.get(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,9 +258,7 @@ 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'); @@ -297,9 +266,7 @@ describe('MdButtonToggle without forms', () => { 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(); }); }); diff --git a/src/lib/button-toggle/button-toggle.ts b/src/lib/button-toggle/button-toggle.ts index f2692909efe9..afe1a60c14d3 100644 --- a/src/lib/button-toggle/button-toggle.ts +++ b/src/lib/button-toggle/button-toggle.ts @@ -72,8 +72,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,14 +87,11 @@ 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 = () => {}; @@ -102,10 +99,6 @@ export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implement /** Child button toggle buttons. */ @ContentChildren(forwardRef(() => MdButtonToggle)) _buttonToggles: QueryList; - ngAfterViewInit() { - this._isInitialized = true; - } - /** `name` attribute for the underlying `input` element. */ @Input() get name(): string { @@ -132,18 +125,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 +151,10 @@ export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implement /** Event emitted when the group's value changes. */ @Output() change: EventEmitter = new EventEmitter(); + constructor(private _changeDetector: ChangeDetectorRef) { + super(); + } + private _updateButtonToggleNames(): void { if (this._buttonToggles) { this._buttonToggles.forEach((toggle) => { @@ -193,7 +183,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 +197,7 @@ export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implement */ writeValue(value: any) { this.value = value; + this._changeDetector.markForCheck(); } /** @@ -438,9 +429,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(); }