Skip to content

Commit

Permalink
fix(button-toggle): remove emit change event when value changes (#6034)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tinayuangao @andrewseguin
tinayuangao authored and andrewseguin committed Jul 28, 2017
1 parent 05ca4a7 commit f8c5be8
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 69 deletions.
65 changes: 15 additions & 50 deletions src/lib/button-toggle/button-toggle.spec.ts
View file
Expand Up @@ -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));
Expand All @@ -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();
}));
Expand All @@ -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();
Expand Down Expand Up @@ -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();

Expand All @@ -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();
Expand Down Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
});

});
Expand Down
32 changes: 13 additions & 19 deletions src/lib/button-toggle/button-toggle.ts
View file
Expand Up @@ -23,7 +23,6 @@ import {
ViewChild,
ViewEncapsulation,
forwardRef,
AfterViewInit,
ChangeDetectionStrategy,
ChangeDetectorRef,
} from '@angular/core';
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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();
}
}
}

Expand All @@ -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) => {
Expand Down Expand Up @@ -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;
Expand All @@ -207,6 +196,7 @@ export class MdButtonToggleGroup extends _MdButtonToggleGroupMixinBase implement
*/
writeValue(value: any) {
this.value = value;
this._changeDetector.markForCheck();
}

/**
Expand Down Expand Up @@ -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();
}
Expand Down

0 comments on commit f8c5be8

Please sign in to comment.