Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update(): prevent change event to be fired at initialization #713

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 99 additions & 4 deletions src/components/slide-toggle/slide-toggle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,19 @@ describe('MdSlideToggle', () => {
beforeEach(async(() => {
builder.createAsync(SlideToggleTestApp).then(f => {
fixture = f;

testComponent = fixture.debugElement.componentInstance;

// Enable jasmine spies on event functions, which may trigger at initialization
// of the slide-toggle component.
spyOn(fixture.debugElement.componentInstance, 'onSlideChange').and.callThrough();
spyOn(fixture.debugElement.componentInstance, 'onSlideClick').and.callThrough();

// Initialize the slide-toggle component, by triggering the first change detection cycle.
fixture.detectChanges();

let slideToggleDebug = fixture.debugElement.query(By.css('md-slide-toggle'));

testComponent = fixture.debugElement.componentInstance;
slideToggle = slideToggleDebug.componentInstance;
slideToggleElement = slideToggleDebug.nativeElement;
slideToggleControl = slideToggleDebug.injector.get(NgControl);
Expand Down Expand Up @@ -103,8 +111,6 @@ describe('MdSlideToggle', () => {
// Since we're using a label element and a visual hidden input, this behavior can led
// to an issue, where the click events on the slide-toggle are getting executed twice.

spyOn(testComponent, 'onSlideClick');

expect(slideToggle.checked).toBe(false);
expect(slideToggleElement.classList).not.toContain('md-checked');

Expand All @@ -117,6 +123,42 @@ describe('MdSlideToggle', () => {
expect(testComponent.onSlideClick).toHaveBeenCalledTimes(1);
});

it('should not trigger the change event multiple times', async(() => {
expect(inputElement.checked).toBe(false);
expect(slideToggleElement.classList).not.toContain('md-checked');

testComponent.slideChecked = true;
fixture.detectChanges();

expect(inputElement.checked).toBe(true);
expect(slideToggleElement.classList).toContain('md-checked');

// Wait for the fixture to become stable, because the EventEmitter for the change event,
// will only fire after the zone async change detection has finished.
fixture.whenStable().then(() => {
expect(testComponent.onSlideChange).toHaveBeenCalledTimes(1);
});

}));

it('should not trigger the change event on initialization', async(() => {
expect(inputElement.checked).toBe(false);
expect(slideToggleElement.classList).not.toContain('md-checked');

testComponent.slideChecked = true;
fixture.detectChanges();

expect(inputElement.checked).toBe(true);
expect(slideToggleElement.classList).toContain('md-checked');

// Wait for the fixture to become stable, because the EventEmitter for the change event,
// will only fire after the zone async change detection has finished.
fixture.whenStable().then(() => {
expect(testComponent.onSlideChange).toHaveBeenCalledTimes(1);
});

}));

it('should add a suffix to the inputs id', () => {
testComponent.slideId = 'myId';
fixture.detectChanges();
Expand Down Expand Up @@ -269,6 +311,56 @@ describe('MdSlideToggle', () => {

});

describe('custom template', () => {

let testComponent: SlideToggleTestApp;
let slideToggle: MdSlideToggle;
let slideToggleElement: HTMLElement;
let labelElement: HTMLLabelElement;
let inputElement: HTMLInputElement;

it('should not trigger the change event on initialization', async(() => {
builder
.overrideTemplate(SlideToggleTestApp, `
<md-slide-toggle checked="true" (change)="onSlideChange($event)"></md-slide-toggle>
`)
.createAsync(SlideToggleTestApp)
.then(fixture => {
// Initialize the variables for our test.
initializeTest(fixture);

// Enable jasmine spies on event functions, which may trigger at initialization
// of the slide-toggle component.
spyOn(fixture.debugElement.componentInstance, 'onSlideChange').and.callThrough();

fixture.detectChanges();

fixture.whenStable().then(() => {
expect(testComponent.onSlideChange).not.toHaveBeenCalled();
});
});
}));

/**
* Initializes the suites variables, to allow developers to easily access the several variables
* without loading / querying them always again.
* @param fixture Custom fixture, which contains the slide-toggle component.
*/
function initializeTest(fixture: ComponentFixture<any>) {
testComponent = fixture.debugElement.componentInstance;

// Initialize the slide-toggle component, by triggering the first change detection cycle.
fixture.detectChanges();

let slideToggleDebug = fixture.debugElement.query(By.css('md-slide-toggle'));

slideToggle = slideToggleDebug.componentInstance;
slideToggleElement = slideToggleDebug.nativeElement;
inputElement = fixture.debugElement.query(By.css('input')).nativeElement;
labelElement = fixture.debugElement.query(By.css('label')).nativeElement;
}
});

});

/**
Expand All @@ -288,7 +380,7 @@ function dispatchFocusChangeEvent(eventName: string, element: HTMLElement): void
<md-slide-toggle [(ngModel)]="slideModel" [disabled]="isDisabled" [color]="slideColor"
[id]="slideId" [checked]="slideChecked" [name]="slideName"
[aria-label]="slideLabel" [ariaLabel]="slideLabel"
[ariaLabelledby]="slideLabelledBy" (change)="lastEvent = $event"
[ariaLabelledby]="slideLabelledBy" (change)="onSlideChange($event)"
(click)="onSlideClick($event)">
<span>Test Slide Toggle</span>
</md-slide-toggle>
Expand All @@ -307,4 +399,7 @@ class SlideToggleTestApp {
lastEvent: MdSlideToggleChange;

onSlideClick(event: Event) {}
onSlideChange(event: MdSlideToggleChange) {
this.lastEvent = event;
}
}
21 changes: 18 additions & 3 deletions src/components/slide-toggle/slide-toggle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {
ChangeDetectionStrategy,
Input,
Output,
EventEmitter
EventEmitter,
AfterContentInit
} from '@angular/core';
import {
ControlValueAccessor,
Expand Down Expand Up @@ -45,7 +46,7 @@ let nextId = 0;
providers: [MD_SLIDE_TOGGLE_VALUE_ACCESSOR],
changeDetection: ChangeDetectionStrategy.OnPush
})
export class MdSlideToggle implements ControlValueAccessor {
export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {

private onChange = (_: any) => {};
private onTouched = () => {};
Expand All @@ -56,6 +57,7 @@ export class MdSlideToggle implements ControlValueAccessor {
private _color: string;
private _hasFocus: boolean = false;
private _isMousedown: boolean = false;
private _isInitialized: boolean = false;

@Input() @BooleanFieldValue() disabled: boolean = false;
@Input() name: string = null;
Expand All @@ -74,6 +76,14 @@ export class MdSlideToggle implements ControlValueAccessor {
private _renderer: Renderer) {
}

/** TODO: internal */
ngAfterContentInit() {
// Mark this component as initialized in AfterContentInit because the initial checked value can
// possibly be set by NgModel or the checked attribute. This would cause the change event to
// be emitted, before the component is actually initialized.
this._isInitialized = true;
}

/**
* The onChangeEvent method will be also called on click.
* This is because everything for the slide-toggle is wrapped inside of a label,
Expand Down Expand Up @@ -163,7 +173,12 @@ export class MdSlideToggle implements ControlValueAccessor {
if (this.checked !== !!value) {
this._checked = value;
this.onChange(this._checked);
this._emitChangeEvent();

// Only fire a change event if the `slide-toggle` is completely initialized and
// all attributes / inputs are properly loaded.
if (this._isInitialized) {
this._emitChangeEvent();
}
}
}

Expand Down