fix(slide-toggle): invalid model change event (#4220)

* fix(slide-toggle): invalid model change event (#4140)" (#4218)

While initially looking into #4124, there were a few more issues inside of the slide-toggle.

* ngModelChange event is dispatched at initialization.

* Checked state isn't synchronized when state changes through drag. New state after dragging got overwritten by click event on label.

* Removes unnecessary logic inside of `change` listener. Change event doesn't fire if underlying checkbox is disabled.

Fixes #4124.

* Cast NgModel value to a boolean

* Properly mark component for check

Author: devversion

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

@@ -547,18 +547,21 @@ describe('MdSlideToggle with forms', () => {
       expect(slideToggleModel.pristine).toBe(true);
       expect(slideToggleModel.touched).toBe(false);
 
-      // After changing the value programmatically, the control should
-      // become dirty (not pristine), but remain untouched.
+      // After changing the value from the view, the control should
+      // become dirty (not pristine), but remain untouched if focus is still there.
       slideToggle.checked = true;
-      fixture.detectChanges();
+
+      // Dispatch a change event on the input element to fake a user interaction that triggered
+      // the state change.
+      dispatchFakeEvent(inputElement, 'change');
 
       expect(slideToggleModel.valid).toBe(true);
       expect(slideToggleModel.pristine).toBe(false);
       expect(slideToggleModel.touched).toBe(false);
 
-      // After a user interaction occurs (such as a click), the control should remain dirty and
-      // now also be touched.
-      labelElement.click();
+      // Once the input element loses focus, the control should remain dirty but should
+      // also turn touched.
+      dispatchFakeEvent(inputElement, 'blur');
       fixture.detectChanges();
 
       expect(slideToggleModel.valid).toBe(true);
@@ -576,13 +579,13 @@ describe('MdSlideToggle with forms', () => {
       expect(slideToggleModel.touched).toBe(false);
       expect(slideToggleElement.classList).toContain('mat-checked');
 
-      // After a user interaction occurs (such as a click), the control should remain dirty and
-      // now also be touched.
-      inputElement.click();
+      // Once the input element loses focus, the control should remain dirty but should
+      // also turn touched.
+      dispatchFakeEvent(inputElement, 'blur');
       fixture.detectChanges();
 
       expect(slideToggleModel.touched).toBe(true);
-      expect(slideToggleElement.classList).not.toContain('mat-checked');
+      expect(slideToggleElement.classList).toContain('mat-checked');
     });
 
     it('should not set the control to touched when changing the model', fakeAsync(() => {
@@ -603,6 +606,31 @@ describe('MdSlideToggle with forms', () => {
       expect(slideToggle.checked).toBe(true);
       expect(slideToggleElement.classList).toContain('mat-checked');
     }));
+
+    it('should update checked state on click if control is checked initially', fakeAsync(() => {
+      fixture = TestBed.createComponent(SlideToggleWithModel);
+      slideToggle = fixture.debugElement.query(By.directive(MdSlideToggle)).componentInstance;
+      labelElement = fixture.debugElement.query(By.css('label')).nativeElement;
+
+      fixture.componentInstance.modelValue = true;
+      fixture.detectChanges();
+
+      // Flush the microtasks because the forms module updates the model state asynchronously.
+      flushMicrotasks();
+
+      // Now the new checked variable has been updated in the slide-toggle and the slide-toggle
+      // is marked for check because it still needs to update the underlying input.
+      fixture.detectChanges();
+
+      expect(slideToggle.checked)
+        .toBe(true, 'Expected slide-toggle to be checked initially');
+
+      labelElement.click();
+      fixture.detectChanges();
+
+      expect(slideToggle.checked)
+        .toBe(false, 'Expected slide-toggle to be no longer checked after label click.');
+    }));
   });
 
   describe('with a FormControl', () => {

src/lib/slide-toggle/slide-toggle.ts

@@ -84,9 +84,9 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
   private onTouched = () => {};
 
   private _uniqueId: string = `md-slide-toggle-${++nextUniqueId}`;
-  private _checked: boolean = false;
   private _slideRenderer: SlideToggleRenderer;
   private _required: boolean = false;
+  private _checked: boolean = false;
 
   /** Reference to the focus state ripple. */
   private _focusRipple: RippleRef | null;
@@ -103,6 +103,8 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
   /** Whether the label should appear after or before the slide-toggle. Defaults to 'after' */
   @Input() labelPosition: 'before' | 'after' = 'after';
 
+  /** Whether the slide-toggle element is checked or not */
+
   /** Used to set the aria-label attribute on the underlying input element. */
   @Input('aria-label') ariaLabel: string | null = null;
 
@@ -114,6 +116,13 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
   get required(): boolean { return this._required; }
   set required(value) { this._required = coerceBooleanProperty(value); }
 
+  /** Whether the slide-toggle element is checked or not */
+  @Input()
+  get checked(): boolean { return this._checked; }
+  set checked(value) {
+    this._checked = !!value;
+    this._changeDetectorRef.markForCheck();
+  }
   /** An event will be dispatched each time the slide-toggle changes its value. */
   @Output() change: EventEmitter<MdSlideToggleChange> = new EventEmitter<MdSlideToggleChange>();
 
@@ -147,29 +156,30 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
   }
 
   /**
-   * The onChangeEvent method will be also called on click.
-   * This is because everything for the slide-toggle is wrapped inside of a label,
-   * which triggers a onChange event on click.
+   * This function will called if the underlying input changed its value through user interaction.
    */
   _onChangeEvent(event: Event) {
     // We always have to stop propagation on the change event.
     // Otherwise the change event, from the input element, will bubble up and
     // emit its event object to the component's `change` output.
     event.stopPropagation();
 
-    // Once a drag is currently in progress, we do not want to toggle the slide-toggle on a click.
-    if (!this.disabled && !this._slideRenderer.dragging) {
-      this.toggle();
+    // Sync the value from the underlying input element with the slide-toggle component.
+    this.checked = this._inputElement.nativeElement.checked;
 
-      // Emit our custom change event if the native input emitted one.
-      // It is important to only emit it, if the native input triggered one, because
-      // we don't want to trigger a change event, when the `checked` variable changes for example.
-      this._emitChangeEvent();
-    }
+    // Emit our custom change event if the native input emitted one.
+    // It is important to only emit it, if the native input triggered one, because we don't want
+    // to trigger a change event, when the `checked` variable changes programmatically.
+    this._emitChangeEvent();
   }
 
   _onInputClick(event: Event) {
-    this.onTouched();
+    // In some situations the user will release the mouse on the label element. The label element
+    // redirects the click to the underlying input element and will result in a value change.
+    // Prevent the default behavior if dragging, because the value will be set after drag.
+    if (this._slideRenderer.dragging) {
+      event.preventDefault();
+    }
 
     // We have to stop propagation for click events on the visual hidden input element.
     // By default, when a user clicks on a label element, a generated click event will be
@@ -183,7 +193,7 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
 
   /** Implemented as part of ControlValueAccessor. */
   writeValue(value: any): void {
-    this.checked = value;
+    this.checked = !!value;
   }
 
   /** Implemented as part of ControlValueAccessor. */
@@ -207,16 +217,6 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
     this._focusOriginMonitor.focusVia(this._inputElement.nativeElement, 'keyboard');
   }
 
-  /** Whether the slide-toggle is checked. */
-  @Input()
-  get checked() { return !!this._checked; }
-  set checked(value) {
-    if (this.checked !== !!value) {
-      this._checked = value;
-      this.onChange(this._checked);
-    }
-  }
-
   /** Toggles the checked state of the slide-toggle. */
   toggle() {
     this.checked = !this.checked;
@@ -238,15 +238,17 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
     }
   }
 
-  /** Emits the change event to the `change` output EventEmitter */
+  /**
+   * Emits a change event on the `change` output. Also notifies the FormControl about the change.
+   */
   private _emitChangeEvent() {
     let event = new MdSlideToggleChange();
     event.source = this;
     event.checked = this.checked;
     this.change.emit(event);
+    this.onChange(this.checked);
   }
 
-
   _onDragStart() {
     if (!this.disabled) {
       this._slideRenderer.startThumbDrag(this.checked);