Skip to content

Commit 99b27e8

Browse files
authored
fix(slide-toggle): clicks not landing correctly in some cases on Chrome (#18285)
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.
1 parent 27ef842 commit 99b27e8

File tree

3 files changed

+23
-2
lines changed

3 files changed

+23
-2
lines changed

src/material/slide-toggle/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ ng_test_library(
5656
),
5757
deps = [
5858
":slide-toggle",
59+
"//src/cdk/a11y",
5960
"//src/cdk/bidi",
6061
"//src/cdk/observers",
6162
"//src/cdk/testing/private",

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ import {
99
flushMicrotasks,
1010
TestBed,
1111
tick,
12+
inject,
1213
} from '@angular/core/testing';
1314
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
1415
import {By} from '@angular/platform-browser';
16+
import {FocusMonitor} from '@angular/cdk/a11y';
1517
import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
1618
import {MAT_SLIDE_TOGGLE_DEFAULT_OPTIONS} from './slide-toggle-config';
1719

@@ -310,6 +312,19 @@ describe('MatSlideToggle without forms', () => {
310312
expect(document.activeElement).toBe(inputElement);
311313
});
312314

315+
it('should not manually move focus to underlying input when focus comes from mouse or touch',
316+
inject([FocusMonitor], (focusMonitor: FocusMonitor) => {
317+
expect(document.activeElement).not.toBe(inputElement);
318+
319+
focusMonitor.focusVia(slideToggleElement, 'mouse');
320+
fixture.detectChanges();
321+
expect(document.activeElement).not.toBe(inputElement);
322+
323+
focusMonitor.focusVia(slideToggleElement, 'touch');
324+
fixture.detectChanges();
325+
expect(document.activeElement).not.toBe(inputElement);
326+
}));
327+
313328
it('should set a element class if labelPosition is set to before', () => {
314329
expect(slideToggleElement.classList).not.toContain('mat-slide-toggle-label-before');
315330

src/material/slide-toggle/slide-toggle.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ const _MatSlideToggleMixinBase:
9191
'[class.mat-disabled]': 'disabled',
9292
'[class.mat-slide-toggle-label-before]': 'labelPosition == "before"',
9393
'[class._mat-animation-noopable]': '_animationMode === "NoopAnimations"',
94-
'(focus)': '_inputElement.nativeElement.focus()',
9594
},
9695
templateUrl: 'slide-toggle.html',
9796
styleUrls: ['slide-toggle.css'],
@@ -193,7 +192,13 @@ export class MatSlideToggle extends _MatSlideToggleMixinBase implements OnDestro
193192
this._focusMonitor
194193
.monitor(this._elementRef, true)
195194
.subscribe(focusOrigin => {
196-
if (!focusOrigin) {
195+
// Only forward focus manually when it was received programmatically or through the
196+
// keyboard. We should not do this for mouse/touch focus for two reasons:
197+
// 1. It can prevent clicks from landing in Chrome (see #18269).
198+
// 2. They're already handled by the wrapping `label` element.
199+
if (focusOrigin === 'keyboard' || focusOrigin === 'program') {
200+
this._inputElement.nativeElement.focus();
201+
} else if (!focusOrigin) {
197202
// When a focused element becomes disabled, the browser *immediately* fires a blur event.
198203
// Angular does not expect events to be raised during change detection, so any state
199204
// change (such as a form control's 'ng-touched') will cause a changed-after-checked

0 commit comments

Comments
 (0)