Skip to content

Commit 93f67d4

Browse files
committed
fix(): slide-toggle should only emit events if native input emitted one.
1 parent 63c7d9c commit 93f67d4

File tree

2 files changed

+33
-21
lines changed

2 files changed

+33
-21
lines changed

src/components/slide-toggle/slide-toggle.spec.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,11 @@ describe('MdSlideToggle', () => {
129129
expect(testComponent.onSlideClick).toHaveBeenCalledTimes(1);
130130
});
131131

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

136-
testComponent.slideChecked = true;
136+
labelElement.click();
137137
fixture.detectChanges();
138138

139139
expect(inputElement.checked).toBe(true);
@@ -142,11 +142,33 @@ describe('MdSlideToggle', () => {
142142
// Wait for the fixture to become stable, because the EventEmitter for the change event,
143143
// will only fire after the zone async change detection has finished.
144144
fixture.whenStable().then(() => {
145+
// The change event shouldn't fire, because the value change was not caused
146+
// by any interaction.
145147
expect(testComponent.onSlideChange).toHaveBeenCalledTimes(1);
146148
});
147149

148150
}));
149151

152+
it('should not trigger the change event by changing the native value', async(() => {
153+
expect(inputElement.checked).toBe(false);
154+
expect(slideToggleElement.classList).not.toContain('md-checked');
155+
156+
testComponent.slideChecked = true;
157+
fixture.detectChanges();
158+
159+
expect(inputElement.checked).toBe(true);
160+
expect(slideToggleElement.classList).toContain('md-checked');
161+
162+
// Wait for the fixture to become stable, because the EventEmitter for the change event,
163+
// will only fire after the zone async change detection has finished.
164+
fixture.whenStable().then(() => {
165+
// The change event shouldn't fire, because the value change was not caused
166+
// by any interaction.
167+
expect(testComponent.onSlideChange).not.toHaveBeenCalled();
168+
});
169+
170+
}));
171+
150172
it('should not trigger the change event on initialization', async(() => {
151173
expect(inputElement.checked).toBe(false);
152174
expect(slideToggleElement.classList).not.toContain('md-checked');
@@ -160,7 +182,8 @@ describe('MdSlideToggle', () => {
160182
// Wait for the fixture to become stable, because the EventEmitter for the change event,
161183
// will only fire after the zone async change detection has finished.
162184
fixture.whenStable().then(() => {
163-
expect(testComponent.onSlideChange).toHaveBeenCalledTimes(1);
185+
// The change event shouldn't fire, because the native input element is not focused.
186+
expect(testComponent.onSlideChange).not.toHaveBeenCalled();
164187
});
165188

166189
}));

src/components/slide-toggle/slide-toggle.ts

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ import {
66
ChangeDetectionStrategy,
77
Input,
88
Output,
9-
EventEmitter,
10-
AfterContentInit
9+
EventEmitter
1110
} from '@angular/core';
1211
import {
1312
ControlValueAccessor,
@@ -46,7 +45,7 @@ let nextId = 0;
4645
providers: [MD_SLIDE_TOGGLE_VALUE_ACCESSOR],
4746
changeDetection: ChangeDetectionStrategy.OnPush
4847
})
49-
export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
48+
export class MdSlideToggle implements ControlValueAccessor {
5049

5150
private onChange = (_: any) => {};
5251
private onTouched = () => {};
@@ -57,7 +56,6 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
5756
private _color: string;
5857
private _hasFocus: boolean = false;
5958
private _isMousedown: boolean = false;
60-
private _isInitialized: boolean = false;
6159

6260
@Input() @BooleanFieldValue() disabled: boolean = false;
6361
@Input() name: string = null;
@@ -76,14 +74,6 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
7674
private _renderer: Renderer) {
7775
}
7876

79-
/** TODO: internal */
80-
ngAfterContentInit() {
81-
// Mark this component as initialized in AfterContentInit because the initial checked value can
82-
// possibly be set by NgModel or the checked attribute. This would cause the change event to
83-
// be emitted, before the component is actually initialized.
84-
this._isInitialized = true;
85-
}
86-
8777
/**
8878
* The onChangeEvent method will be also called on click.
8979
* This is because everything for the slide-toggle is wrapped inside of a label,
@@ -98,6 +88,11 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
9888

9989
if (!this.disabled) {
10090
this.toggle();
91+
92+
// Emit our custom change event if the native input emitted one.
93+
// It is important to only emit it, if the native input triggered one, because
94+
// we don't want to trigger a change event, when the `checked` variable changes for example.
95+
this._emitChangeEvent();
10196
}
10297
}
10398

@@ -173,12 +168,6 @@ export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
173168
if (this.checked !== !!value) {
174169
this._checked = value;
175170
this.onChange(this._checked);
176-
177-
// Only fire a change event if the `slide-toggle` is completely initialized and
178-
// all attributes / inputs are properly loaded.
179-
if (this._isInitialized) {
180-
this._emitChangeEvent();
181-
}
182171
}
183172
}
184173

0 commit comments

Comments
 (0)