Skip to content

Commit c40893b

Browse files
committed
fix(): visual hidden inputs should not bubble change event
* Currently the change event of the visual hidden inputs will bubble up and emit its event object to the component's `change` output. This is currently an issue of Angular 2 - See angular/angular#4059 To prevent the events from bubbling up, we have to stop propagation on change. Fixes #544.
1 parent 3b527e8 commit c40893b

File tree

7 files changed

+57
-18
lines changed

7 files changed

+57
-18
lines changed

src/components/checkbox/checkbox.spec.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,27 @@ describe('MdCheckbox', () => {
286286
fixture.detectChanges();
287287

288288
tick();
289-
expect(testComponent.handleChange).toHaveBeenCalled();
289+
290+
expect(testComponent.handleChange).toHaveBeenCalledTimes(1);
291+
expect(testComponent.handleChange).toHaveBeenCalledWith(true);
292+
}));
293+
294+
it('should not emit a DOM event to the change output', fakeAsync(() => {
295+
expect(testComponent.handleChange).not.toHaveBeenCalled();
296+
297+
// Trigger the click on the inputElement, because the input will probably
298+
// emit a DOM event to the change output.
299+
inputElement.click();
300+
fixture.detectChanges();
301+
302+
fixture.whenStable().then(() => {
303+
// We're checking the arguments type / emitted value to be a boolean, because sometimes the
304+
// emitted value can be a DOM Event, which is not valid.
305+
// See angular/angular#4059
306+
expect(testComponent.handleChange).toHaveBeenCalledTimes(1);
307+
expect(testComponent.handleChange).toHaveBeenCalledWith(true);
308+
});
309+
290310
}));
291311
});
292312

@@ -521,8 +541,8 @@ class CheckboxWithNameAttribute {}
521541
/** Simple test component with change event */
522542
@Component({
523543
directives: [MdCheckbox],
524-
template: `<md-checkbox (change)="handleChange()"></md-checkbox>`
544+
template: `<md-checkbox (change)="handleChange($event)"></md-checkbox>`
525545
})
526546
class CheckboxWithChangeEvent {
527-
handleChange(): void {}
547+
handleChange(event: any): void {}
528548
}

src/components/checkbox/checkbox.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,14 +242,18 @@ export class MdCheckbox implements ControlValueAccessor {
242242
}
243243

244244
/**
245-
* Event handler for checkbox input element. Toggles checked state if element is not disabled.
245+
* Event handler for checkbox input element.
246+
* Toggles checked state if element is not disabled.
246247
* @param event
247248
* @internal
248249
*/
249250
onInteractionEvent(event: Event) {
250-
if (this.disabled) {
251-
event.stopPropagation();
252-
} else {
251+
// We always have to stop propagation on the change event.
252+
// Otherwise the change event, from the input element, will bubble up and
253+
// emit its event object to the `change` output.
254+
event.stopPropagation();
255+
256+
if (!this.disabled) {
253257
this.toggle();
254258
}
255259
}

src/components/radio/radio.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
[checked]="checked"
1414
[disabled]="disabled"
1515
[name]="name"
16-
(change)="onInputChange()"
16+
(change)="onInputChange($event)"
1717
(focus)="onInputFocus()"
1818
(blur)="onInputBlur()" />
1919

src/components/radio/radio.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,12 @@ export class MdRadioButton implements OnInit {
391391
* Checks the radio due to an interaction with the underlying native <input type="radio">
392392
* @internal
393393
*/
394-
onInputChange() {
394+
onInputChange(event: Event) {
395+
// We always have to stop propagation on the change event.
396+
// Otherwise the change event, from the input element, will bubble up and
397+
// emit its event object to the `change` output.
398+
event.stopPropagation();
399+
395400
this.checked = true;
396401
if (this.radioGroup) {
397402
this.radioGroup.touch();

src/components/slide-toggle/slide-toggle.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
[attr.aria-labelledby]="ariaLabelledby"
1818
(blur)="onInputBlur()"
1919
(focus)="onInputFocus()"
20-
(change)="onChangeEvent()">
20+
(change)="onChangeEvent($event)">
2121
</div>
2222
<span class="md-slide-toggle-content">
2323
<ng-content></ng-content>

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import {
44
expect,
55
beforeEach,
66
inject,
7-
async,
7+
async,
8+
fakeAsync,
89
} from '@angular/core/testing';
910
import {TestComponentBuilder, ComponentFixture} from '@angular/compiler/testing';
1011
import {By} from '@angular/platform-browser';
@@ -161,14 +162,17 @@ describe('MdSlideToggle', () => {
161162
expect(slideToggleElement.classList).not.toContain('ng-dirty');
162163
});
163164

164-
it('should emit the new values', () => {
165-
expect(testComponent.changeCount).toBe(0);
165+
it('should emit the new values properly', fakeAsync(() => {
166+
spyOn(testComponent, 'onValueChange');
166167

167168
labelElement.click();
168169
fixture.detectChanges();
169170

170-
expect(testComponent.changeCount).toBe(1);
171-
});
171+
fixture.whenStable().then(() => {
172+
expect(testComponent.onValueChange).toHaveBeenCalledTimes(1);
173+
expect(testComponent.onValueChange).toHaveBeenCalledWith(true);
174+
});
175+
}));
172176

173177
it('should support subscription on the change observable', () => {
174178
slideToggle.change.subscribe(value => {
@@ -265,7 +269,7 @@ function dispatchFocusChangeEvent(eventName: string, element: HTMLElement): void
265269
<md-slide-toggle [(ngModel)]="slideModel" [disabled]="isDisabled" [color]="slideColor"
266270
[id]="slideId" [checked]="slideChecked" [name]="slideName"
267271
[aria-label]="slideLabel" [ariaLabel]="slideLabel"
268-
[ariaLabelledby]="slideLabelledBy" (change)="changeCount = changeCount + 1">
272+
[ariaLabelledby]="slideLabelledBy" (change)="onValueChange($event)">
269273
<span>Test Slide Toggle</span>
270274
</md-slide-toggle>
271275
`,
@@ -280,5 +284,6 @@ class SlideToggleTestApp {
280284
slideName: string;
281285
slideLabel: string;
282286
slideLabelledBy: string;
283-
changeCount: number = 0;
287+
288+
onValueChange(event: any): void {};
284289
}

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,12 @@ export class MdSlideToggle implements ControlValueAccessor {
7575
* which triggers a onChange event on click.
7676
* @internal
7777
*/
78-
onChangeEvent() {
78+
onChangeEvent(event: Event) {
79+
// We always have to stop propagation on the change event.
80+
// Otherwise the change event, from the input element, will bubble up and
81+
// emit its event object to the component's `change` output.
82+
event.stopPropagation();
83+
7984
if (!this.disabled) {
8085
this.toggle();
8186
}

0 commit comments

Comments
 (0)