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.

Author: devversion

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

@@ -295,18 +295,23 @@ describe('MdSlideToggle', () => {
       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;
+
+      // Dispatch a change event on the input element to fake a user interaction that triggered
+      // the state change.
+      dispatchFakeEvent(inputElement, 'change');
+
       fixture.detectChanges();
 
       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 looses focus, the control should remain dirty but should
+      // also turn touched.
+      dispatchFakeEvent(inputElement, 'blur');
       fixture.detectChanges();
 
       expect(slideToggleModel.valid).toBe(true);
@@ -324,13 +329,13 @@ describe('MdSlideToggle', () => {
       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 looses 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');
     });
 
     // TODO(kara): update when core/testing adds fix
@@ -434,15 +439,13 @@ describe('MdSlideToggle', () => {
     }));
 
     it('should prevent the form from submit when being required', () => {
-
       if ('reportValidity' in inputElement === false) {
         // If the browser does not report the validity then the tests will break.
         // e.g Safari 8 on Mobile.
         return;
       }
 
       testComponent.isRequired = true;
-
       fixture.detectChanges();
 
       buttonElement.click();

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

@@ -70,7 +70,6 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase
 
   // A unique id for the slide-toggle. By default the id is auto-generated.
   private _uniqueId = `md-slide-toggle-${++nextId}`;
-  private _checked: boolean = false;
   private _color: string;
   private _slideRenderer: SlideToggleRenderer = null;
   private _required: boolean = false;
@@ -91,6 +90,9 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase
   /** 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 */
+  @Input() checked: boolean = false;
+
   /** Used to set the aria-label attribute on the underlying input element. */
   @Input('aria-label') ariaLabel: string = null;
 
@@ -138,29 +140,30 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase
   }
 
   /**
-   * 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
@@ -197,16 +200,6 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase
     this._focusOriginMonitor.focusVia(this._inputElement.nativeElement, this._renderer, '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);
-    }
-  }
-
   /** The color of the slide-toggle. Can be primary, accent, or warn. */
   @Input()
   get color(): string { return this._color; }
@@ -251,12 +244,15 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase
     }
   }
 
-  /** 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);
   }