From 77358cc8b56199b53e3515a5c2b11775c6648ac7 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@users.noreply.github.com>
Date: Fri, 16 Oct 2020 20:58:11 +0200
Subject: [PATCH] fix(material-experimental/mdc-slide-toggle): align focus
 behavior with standard version (#20772)

The focus behavior of the existing `mat-slide-toggle` was changed slightly after the MDC
version was implemented. These changes align the behavior and add some missing tests.

(cherry picked from commit f854196a8cd388e5a6267f8c23c84a3a9f832e5d)
---
 .../mdc-slide-toggle/BUILD.bazel              |  1 +
 .../mdc-slide-toggle/slide-toggle.html        |  6 +-
 .../mdc-slide-toggle/slide-toggle.spec.ts     | 60 +++++++++++++++++--
 .../mdc-slide-toggle/slide-toggle.ts          | 49 +++++++++------
 .../slide-toggle/slide-toggle.spec.ts         |  5 +-
 5 files changed, 94 insertions(+), 27 deletions(-)

diff --git a/src/material-experimental/mdc-slide-toggle/BUILD.bazel b/src/material-experimental/mdc-slide-toggle/BUILD.bazel
index fd813724624d..289e669cee39 100644
--- a/src/material-experimental/mdc-slide-toggle/BUILD.bazel
+++ b/src/material-experimental/mdc-slide-toggle/BUILD.bazel
@@ -64,6 +64,7 @@ ng_test_library(
     ),
     deps = [
         ":mdc-slide-toggle",
+        "//src/cdk/a11y",
         "//src/cdk/bidi",
         "//src/cdk/testing/private",
         "//src/material/slide-toggle",
diff --git a/src/material-experimental/mdc-slide-toggle/slide-toggle.html b/src/material-experimental/mdc-slide-toggle/slide-toggle.html
index c31607d74227..437adb299e61 100644
--- a/src/material-experimental/mdc-slide-toggle/slide-toggle.html
+++ b/src/material-experimental/mdc-slide-toggle/slide-toggle.html
@@ -21,14 +21,12 @@
             [attr.aria-label]="ariaLabel"
             [attr.aria-labelledby]="ariaLabelledby"
             (change)="_onChangeEvent($event)"
-            (click)="_onInputClick($event)"
-            (blur)="_onBlur()"
-            (focus)="_focused = true">
+            (click)="_onInputClick($event)">
       </div>
     </div>
   </div>
 
-  <label [for]="inputId" (click)="$event.stopPropagation()">
+  <label [for]="inputId">
     <ng-content></ng-content>
   </label>
 </div>
diff --git a/src/material-experimental/mdc-slide-toggle/slide-toggle.spec.ts b/src/material-experimental/mdc-slide-toggle/slide-toggle.spec.ts
index 80170e9f86fb..e66b198d0d93 100644
--- a/src/material-experimental/mdc-slide-toggle/slide-toggle.spec.ts
+++ b/src/material-experimental/mdc-slide-toggle/slide-toggle.spec.ts
@@ -1,9 +1,18 @@
 import {BidiModule, Direction} from '@angular/cdk/bidi';
 import {dispatchFakeEvent} from '@angular/cdk/testing/private';
 import {Component} from '@angular/core';
-import {ComponentFixture, fakeAsync, flushMicrotasks, TestBed, tick} from '@angular/core/testing';
+import {
+  ComponentFixture,
+  fakeAsync,
+  flush,
+  flushMicrotasks,
+  inject,
+  TestBed,
+  tick,
+} from '@angular/core/testing';
 import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
 import {By} from '@angular/platform-browser';
+import {FocusMonitor} from '@angular/cdk/a11y';
 import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
 import {MAT_SLIDE_TOGGLE_DEFAULT_OPTIONS} from './slide-toggle-config';
 
@@ -99,6 +108,24 @@ describe('MDC-based MatSlideToggle without forms', () => {
       expect(inputElement.getAttribute('aria-checked')).toBe('true');
     });
 
+    it('should not trigger the click event multiple times', fakeAsync(() => {
+      // By default, when clicking on a label element, a generated click will be dispatched
+      // on the associated input element.
+      // 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.
+
+      expect(slideToggle.checked).toBe(false);
+      expect(slideToggleElement.classList).not.toContain('mat-mdc-slide-toggle-checked');
+
+      labelElement.click();
+      fixture.detectChanges();
+      tick();
+
+      expect(slideToggleElement.classList).toContain('mat-mdc-slide-toggle-checked');
+      expect(slideToggle.checked).toBe(true);
+      expect(testComponent.onSlideClick).toHaveBeenCalledTimes(1);
+    }));
+
     it('should trigger the change event properly', () => {
       expect(inputElement.checked).toBe(false);
       expect(slideToggleElement.classList).not.toContain('mat-mdc-slide-toggle-checked');
@@ -261,6 +288,31 @@ describe('MDC-based MatSlideToggle without forms', () => {
       expect(document.activeElement).toBe(inputElement);
     });
 
+    it('should focus on underlying input element when the host is focused', fakeAsync(() => {
+      expect(document.activeElement).not.toBe(inputElement);
+
+      slideToggleElement.focus();
+      fixture.detectChanges();
+      tick();
+
+      expect(document.activeElement).toBe(inputElement);
+    }));
+
+    it('should not manually move focus to underlying input when focus comes from mouse or touch',
+      fakeAsync(inject([FocusMonitor], (focusMonitor: FocusMonitor) => {
+        expect(document.activeElement).not.toBe(inputElement);
+
+        focusMonitor.focusVia(slideToggleElement, 'mouse');
+        fixture.detectChanges();
+        flush();
+        expect(document.activeElement).not.toBe(inputElement);
+
+        focusMonitor.focusVia(slideToggleElement, 'touch');
+        fixture.detectChanges();
+        flush();
+        expect(document.activeElement).not.toBe(inputElement);
+      })));
+
     it('should set a element class if labelPosition is set to before', () => {
       const formField = slideToggleElement.querySelector('.mdc-form-field')!;
 
@@ -272,7 +324,7 @@ describe('MDC-based MatSlideToggle without forms', () => {
       expect(formField.classList).toContain('mdc-form-field--align-end');
     });
 
-    it('should show ripples on switch element', () => {
+    it('should show ripples', () => {
       const rippleSelector = '.mat-ripple-element';
       const switchElement = slideToggleElement.querySelector('.mdc-switch')!;
 
@@ -342,13 +394,13 @@ describe('MDC-based MatSlideToggle without forms', () => {
       expect(switchEl.classList).toContain('mdc-switch--checked');
     });
 
-    it('should remove the tabindex from the host element', fakeAsync(() => {
+    it('should set the tabindex of the host element to -1', fakeAsync(() => {
       const fixture = TestBed.createComponent(SlideToggleWithTabindexAttr);
 
       fixture.detectChanges();
 
       const slideToggle = fixture.debugElement.query(By.directive(MatSlideToggle))!.nativeElement;
-      expect(slideToggle.hasAttribute('tabindex')).toBe(false);
+      expect(slideToggle.getAttribute('tabindex')).toBe('-1');
     }));
 
     it('should remove the tabindex from the host element when disabled', fakeAsync(() => {
diff --git a/src/material-experimental/mdc-slide-toggle/slide-toggle.ts b/src/material-experimental/mdc-slide-toggle/slide-toggle.ts
index d38573f11009..2dd13632d75e 100644
--- a/src/material-experimental/mdc-slide-toggle/slide-toggle.ts
+++ b/src/material-experimental/mdc-slide-toggle/slide-toggle.ts
@@ -34,6 +34,7 @@ import {
 import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
 import {ThemePalette, RippleAnimationConfig} from '@angular/material/core';
 import {numbers} from '@material/ripple';
+import {FocusMonitor} from '@angular/cdk/a11y';
 import {
   MAT_SLIDE_TOGGLE_DEFAULT_OPTIONS,
   MatSlideToggleDefaultOptions,
@@ -71,7 +72,8 @@ export class MatSlideToggleChange {
   host: {
     'class': 'mat-mdc-slide-toggle',
     '[id]': 'id',
-    '[attr.tabindex]': 'null',
+    // Needs to be `-1` so it can still receive programmatic focus.
+    '[attr.tabindex]': 'disabled ? null : -1',
     '[attr.aria-label]': 'null',
     '[attr.aria-labelledby]': 'null',
     '[class.mat-primary]': 'color === "primary"',
@@ -80,7 +82,6 @@ export class MatSlideToggleChange {
     '[class.mat-mdc-slide-toggle-focused]': '_focused',
     '[class.mat-mdc-slide-toggle-checked]': 'checked',
     '[class._mat-animation-noopable]': '_animationMode === "NoopAnimations"',
-    '(focus)': '_inputElement.nativeElement.focus()',
   },
   exportAs: 'matSlideToggle',
   encapsulation: ViewEncapsulation.None,
@@ -194,7 +195,9 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
   /** Reference to the MDC switch element. */
   @ViewChild('switch') _switchElement: ElementRef<HTMLElement>;
 
-  constructor(private _changeDetectorRef: ChangeDetectorRef,
+  constructor(private _elementRef: ElementRef,
+              private _focusMonitor: FocusMonitor,
+              private _changeDetectorRef: ChangeDetectorRef,
               @Attribute('tabindex') tabIndex: string,
               @Inject(MAT_SLIDE_TOGGLE_DEFAULT_OPTIONS)
                   public defaults: MatSlideToggleDefaultOptions,
@@ -206,9 +209,35 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
     const foundation = this._foundation = new MDCSwitchFoundation(this._adapter);
     foundation.setDisabled(this.disabled);
     foundation.setChecked(this.checked);
+
+    this._focusMonitor
+      .monitor(this._elementRef, true)
+      .subscribe(focusOrigin => {
+        // Only forward focus manually when it was received programmatically or through the
+        // keyboard. We should not do this for mouse/touch focus for two reasons:
+        // 1. It can prevent clicks from landing in Chrome (see #18269).
+        // 2. They're already handled by the wrapping `label` element.
+        if (focusOrigin === 'keyboard' || focusOrigin === 'program') {
+          this._inputElement.nativeElement.focus();
+          this._focused = true;
+        } else if (!focusOrigin) {
+          // When a focused element becomes disabled, the browser *immediately* fires a blur event.
+          // Angular does not expect events to be raised during change detection, so any state
+          // change (such as a form control's ng-touched) will cause a changed-after-checked error.
+          // See https://github.com/angular/angular/issues/17793. To work around this, we defer
+          // telling the form control it has been touched until the next tick.
+          Promise.resolve().then(() => {
+            this._focused = false;
+            this._onTouched();
+            this._changeDetectorRef.markForCheck();
+          });
+        }
+    });
   }
 
   ngOnDestroy() {
+    this._focusMonitor.stopMonitoring(this._elementRef);
+
     if (this._foundation) {
       this._foundation.destroy();
     }
@@ -285,20 +314,6 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
     this._onChange(this.checked);
   }
 
-  /** Handles blur events on the native input. */
-  _onBlur() {
-    // When a focused element becomes disabled, the browser *immediately* fires a blur event.
-    // Angular does not expect events to be raised during change detection, so any state change
-    // (such as a form control's 'ng-touched') will cause a changed-after-checked error.
-    // See https://github.com/angular/angular/issues/17793. To work around this, we defer
-    // telling the form control it has been touched until the next tick.
-    Promise.resolve().then(() => {
-      this._focused = false;
-      this._onTouched();
-      this._changeDetectorRef.markForCheck();
-    });
-  }
-
   static ngAcceptInputType_tabIndex: NumberInput;
   static ngAcceptInputType_required: BooleanInput;
   static ngAcceptInputType_checked: BooleanInput;
diff --git a/src/material/slide-toggle/slide-toggle.spec.ts b/src/material/slide-toggle/slide-toggle.spec.ts
index e40d4bbf4210..8b55aa0cfb21 100644
--- a/src/material/slide-toggle/slide-toggle.spec.ts
+++ b/src/material/slide-toggle/slide-toggle.spec.ts
@@ -333,7 +333,7 @@ describe('MatSlideToggle without forms', () => {
       expect(slideToggleElement.classList).toContain('mat-slide-toggle-label-before');
     });
 
-    it('should show ripples on label mousedown', () => {
+    it('should show ripples', () => {
       const rippleSelector = '.mat-ripple-element:not(.mat-slide-toggle-persistent-ripple)';
 
       expect(slideToggleElement.querySelectorAll(rippleSelector).length).toBe(0);
@@ -408,7 +408,8 @@ describe('MatSlideToggle without forms', () => {
   });
 
   describe('custom action configuration', () => {
-    it('should not change value on click when click action is noop', () => {
+    it('should not change value on click when click action is noop when using custom a ' +
+      'action configuration', () => {
       TestBed
         .resetTestingModule()
         .configureTestingModule({