From c454df5d34afd4a834ce6d0afb0f17e4872fab02 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 25 Jan 2020 11:10:41 +0100 Subject: [PATCH] fix(slide-toggle): clicks not landing correctly in some cases on Chrome Any time the slide toggle host receives focus we forward it immediately to the underlying `input` element. This bouncing around of focus seems to interrupt click events in some cases on Chrome. These changes make it so that we only forward focus for events originating from the keyboard or programmatically. Fixes #18269. --- src/material/slide-toggle/BUILD.bazel | 1 + src/material/slide-toggle/slide-toggle.spec.ts | 15 +++++++++++++++ src/material/slide-toggle/slide-toggle.ts | 9 +++++++-- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/material/slide-toggle/BUILD.bazel b/src/material/slide-toggle/BUILD.bazel index 8ba0c4b72e9c..88cf7b2463ce 100644 --- a/src/material/slide-toggle/BUILD.bazel +++ b/src/material/slide-toggle/BUILD.bazel @@ -56,6 +56,7 @@ ng_test_library( ), deps = [ ":slide-toggle", + "//src/cdk/a11y", "//src/cdk/bidi", "//src/cdk/observers", "//src/cdk/testing/private", diff --git a/src/material/slide-toggle/slide-toggle.spec.ts b/src/material/slide-toggle/slide-toggle.spec.ts index ed97ce8ab4ec..f20ef066c898 100644 --- a/src/material/slide-toggle/slide-toggle.spec.ts +++ b/src/material/slide-toggle/slide-toggle.spec.ts @@ -9,9 +9,11 @@ import { flushMicrotasks, TestBed, tick, + inject, } 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'; @@ -310,6 +312,19 @@ describe('MatSlideToggle without forms', () => { expect(document.activeElement).toBe(inputElement); }); + it('should not manually move focus to underlying input when focus comes from mouse or touch', + inject([FocusMonitor], (focusMonitor: FocusMonitor) => { + expect(document.activeElement).not.toBe(inputElement); + + focusMonitor.focusVia(slideToggleElement, 'mouse'); + fixture.detectChanges(); + expect(document.activeElement).not.toBe(inputElement); + + focusMonitor.focusVia(slideToggleElement, 'touch'); + fixture.detectChanges(); + expect(document.activeElement).not.toBe(inputElement); + })); + it('should set a element class if labelPosition is set to before', () => { expect(slideToggleElement.classList).not.toContain('mat-slide-toggle-label-before'); diff --git a/src/material/slide-toggle/slide-toggle.ts b/src/material/slide-toggle/slide-toggle.ts index bdd9a923ce70..79aafb166f14 100644 --- a/src/material/slide-toggle/slide-toggle.ts +++ b/src/material/slide-toggle/slide-toggle.ts @@ -91,7 +91,6 @@ const _MatSlideToggleMixinBase: '[class.mat-disabled]': 'disabled', '[class.mat-slide-toggle-label-before]': 'labelPosition == "before"', '[class._mat-animation-noopable]': '_animationMode === "NoopAnimations"', - '(focus)': '_inputElement.nativeElement.focus()', }, templateUrl: 'slide-toggle.html', styleUrls: ['slide-toggle.css'], @@ -193,7 +192,13 @@ export class MatSlideToggle extends _MatSlideToggleMixinBase implements OnDestro this._focusMonitor .monitor(this._elementRef, true) .subscribe(focusOrigin => { - if (!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(); + } 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