Skip to content

Commit f854196

Browse files
authored
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.
1 parent 5e9e41b commit f854196

File tree

5 files changed

+94
-27
lines changed

5 files changed

+94
-27
lines changed

src/material-experimental/mdc-slide-toggle/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ ng_test_library(
6464
),
6565
deps = [
6666
":mdc-slide-toggle",
67+
"//src/cdk/a11y",
6768
"//src/cdk/bidi",
6869
"//src/cdk/testing/private",
6970
"//src/material/slide-toggle",

src/material-experimental/mdc-slide-toggle/slide-toggle.html

+2-4
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,12 @@
2121
[attr.aria-label]="ariaLabel"
2222
[attr.aria-labelledby]="ariaLabelledby"
2323
(change)="_onChangeEvent($event)"
24-
(click)="_onInputClick($event)"
25-
(blur)="_onBlur()"
26-
(focus)="_focused = true">
24+
(click)="_onInputClick($event)">
2725
</div>
2826
</div>
2927
</div>
3028

31-
<label [for]="inputId" (click)="$event.stopPropagation()">
29+
<label [for]="inputId">
3230
<ng-content></ng-content>
3331
</label>
3432
</div>

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

+56-4
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11
import {BidiModule, Direction} from '@angular/cdk/bidi';
22
import {dispatchFakeEvent} from '@angular/cdk/testing/private';
33
import {Component} from '@angular/core';
4-
import {ComponentFixture, fakeAsync, flushMicrotasks, TestBed, tick} from '@angular/core/testing';
4+
import {
5+
ComponentFixture,
6+
fakeAsync,
7+
flush,
8+
flushMicrotasks,
9+
inject,
10+
TestBed,
11+
tick,
12+
} from '@angular/core/testing';
513
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
614
import {By} from '@angular/platform-browser';
15+
import {FocusMonitor} from '@angular/cdk/a11y';
716
import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
817
import {MAT_SLIDE_TOGGLE_DEFAULT_OPTIONS} from './slide-toggle-config';
918

@@ -99,6 +108,24 @@ describe('MDC-based MatSlideToggle without forms', () => {
99108
expect(inputElement.getAttribute('aria-checked')).toBe('true');
100109
});
101110

111+
it('should not trigger the click event multiple times', fakeAsync(() => {
112+
// By default, when clicking on a label element, a generated click will be dispatched
113+
// on the associated input element.
114+
// Since we're using a label element and a visual hidden input, this behavior can led
115+
// to an issue, where the click events on the slide-toggle are getting executed twice.
116+
117+
expect(slideToggle.checked).toBe(false);
118+
expect(slideToggleElement.classList).not.toContain('mat-mdc-slide-toggle-checked');
119+
120+
labelElement.click();
121+
fixture.detectChanges();
122+
tick();
123+
124+
expect(slideToggleElement.classList).toContain('mat-mdc-slide-toggle-checked');
125+
expect(slideToggle.checked).toBe(true);
126+
expect(testComponent.onSlideClick).toHaveBeenCalledTimes(1);
127+
}));
128+
102129
it('should trigger the change event properly', () => {
103130
expect(inputElement.checked).toBe(false);
104131
expect(slideToggleElement.classList).not.toContain('mat-mdc-slide-toggle-checked');
@@ -261,6 +288,31 @@ describe('MDC-based MatSlideToggle without forms', () => {
261288
expect(document.activeElement).toBe(inputElement);
262289
});
263290

291+
it('should focus on underlying input element when the host is focused', fakeAsync(() => {
292+
expect(document.activeElement).not.toBe(inputElement);
293+
294+
slideToggleElement.focus();
295+
fixture.detectChanges();
296+
tick();
297+
298+
expect(document.activeElement).toBe(inputElement);
299+
}));
300+
301+
it('should not manually move focus to underlying input when focus comes from mouse or touch',
302+
fakeAsync(inject([FocusMonitor], (focusMonitor: FocusMonitor) => {
303+
expect(document.activeElement).not.toBe(inputElement);
304+
305+
focusMonitor.focusVia(slideToggleElement, 'mouse');
306+
fixture.detectChanges();
307+
flush();
308+
expect(document.activeElement).not.toBe(inputElement);
309+
310+
focusMonitor.focusVia(slideToggleElement, 'touch');
311+
fixture.detectChanges();
312+
flush();
313+
expect(document.activeElement).not.toBe(inputElement);
314+
})));
315+
264316
it('should set a element class if labelPosition is set to before', () => {
265317
const formField = slideToggleElement.querySelector('.mdc-form-field')!;
266318

@@ -272,7 +324,7 @@ describe('MDC-based MatSlideToggle without forms', () => {
272324
expect(formField.classList).toContain('mdc-form-field--align-end');
273325
});
274326

275-
it('should show ripples on switch element', () => {
327+
it('should show ripples', () => {
276328
const rippleSelector = '.mat-ripple-element';
277329
const switchElement = slideToggleElement.querySelector('.mdc-switch')!;
278330

@@ -342,13 +394,13 @@ describe('MDC-based MatSlideToggle without forms', () => {
342394
expect(switchEl.classList).toContain('mdc-switch--checked');
343395
});
344396

345-
it('should remove the tabindex from the host element', fakeAsync(() => {
397+
it('should set the tabindex of the host element to -1', fakeAsync(() => {
346398
const fixture = TestBed.createComponent(SlideToggleWithTabindexAttr);
347399

348400
fixture.detectChanges();
349401

350402
const slideToggle = fixture.debugElement.query(By.directive(MatSlideToggle))!.nativeElement;
351-
expect(slideToggle.hasAttribute('tabindex')).toBe(false);
403+
expect(slideToggle.getAttribute('tabindex')).toBe('-1');
352404
}));
353405

354406
it('should remove the tabindex from the host element when disabled', fakeAsync(() => {

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

+32-17
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
3535
import {ThemePalette, RippleAnimationConfig} from '@angular/material-experimental/mdc-core';
3636
import {numbers} from '@material/ripple';
37+
import {FocusMonitor} from '@angular/cdk/a11y';
3738
import {
3839
MAT_SLIDE_TOGGLE_DEFAULT_OPTIONS,
3940
MatSlideToggleDefaultOptions,
@@ -71,7 +72,8 @@ export class MatSlideToggleChange {
7172
host: {
7273
'class': 'mat-mdc-slide-toggle',
7374
'[id]': 'id',
74-
'[attr.tabindex]': 'null',
75+
// Needs to be `-1` so it can still receive programmatic focus.
76+
'[attr.tabindex]': 'disabled ? null : -1',
7577
'[attr.aria-label]': 'null',
7678
'[attr.aria-labelledby]': 'null',
7779
'[class.mat-primary]': 'color === "primary"',
@@ -80,7 +82,6 @@ export class MatSlideToggleChange {
8082
'[class.mat-mdc-slide-toggle-focused]': '_focused',
8183
'[class.mat-mdc-slide-toggle-checked]': 'checked',
8284
'[class._mat-animation-noopable]': '_animationMode === "NoopAnimations"',
83-
'(focus)': '_inputElement.nativeElement.focus()',
8485
},
8586
exportAs: 'matSlideToggle',
8687
encapsulation: ViewEncapsulation.None,
@@ -194,7 +195,9 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
194195
/** Reference to the MDC switch element. */
195196
@ViewChild('switch') _switchElement: ElementRef<HTMLElement>;
196197

197-
constructor(private _changeDetectorRef: ChangeDetectorRef,
198+
constructor(private _elementRef: ElementRef,
199+
private _focusMonitor: FocusMonitor,
200+
private _changeDetectorRef: ChangeDetectorRef,
198201
@Attribute('tabindex') tabIndex: string,
199202
@Inject(MAT_SLIDE_TOGGLE_DEFAULT_OPTIONS)
200203
public defaults: MatSlideToggleDefaultOptions,
@@ -206,9 +209,35 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
206209
const foundation = this._foundation = new MDCSwitchFoundation(this._adapter);
207210
foundation.setDisabled(this.disabled);
208211
foundation.setChecked(this.checked);
212+
213+
this._focusMonitor
214+
.monitor(this._elementRef, true)
215+
.subscribe(focusOrigin => {
216+
// Only forward focus manually when it was received programmatically or through the
217+
// keyboard. We should not do this for mouse/touch focus for two reasons:
218+
// 1. It can prevent clicks from landing in Chrome (see #18269).
219+
// 2. They're already handled by the wrapping `label` element.
220+
if (focusOrigin === 'keyboard' || focusOrigin === 'program') {
221+
this._inputElement.nativeElement.focus();
222+
this._focused = true;
223+
} else if (!focusOrigin) {
224+
// When a focused element becomes disabled, the browser *immediately* fires a blur event.
225+
// Angular does not expect events to be raised during change detection, so any state
226+
// change (such as a form control's ng-touched) will cause a changed-after-checked error.
227+
// See https://github.com/angular/angular/issues/17793. To work around this, we defer
228+
// telling the form control it has been touched until the next tick.
229+
Promise.resolve().then(() => {
230+
this._focused = false;
231+
this._onTouched();
232+
this._changeDetectorRef.markForCheck();
233+
});
234+
}
235+
});
209236
}
210237

211238
ngOnDestroy() {
239+
this._focusMonitor.stopMonitoring(this._elementRef);
240+
212241
if (this._foundation) {
213242
this._foundation.destroy();
214243
}
@@ -285,20 +314,6 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
285314
this._onChange(this.checked);
286315
}
287316

288-
/** Handles blur events on the native input. */
289-
_onBlur() {
290-
// When a focused element becomes disabled, the browser *immediately* fires a blur event.
291-
// Angular does not expect events to be raised during change detection, so any state change
292-
// (such as a form control's 'ng-touched') will cause a changed-after-checked error.
293-
// See https://github.com/angular/angular/issues/17793. To work around this, we defer
294-
// telling the form control it has been touched until the next tick.
295-
Promise.resolve().then(() => {
296-
this._focused = false;
297-
this._onTouched();
298-
this._changeDetectorRef.markForCheck();
299-
});
300-
}
301-
302317
static ngAcceptInputType_tabIndex: NumberInput;
303318
static ngAcceptInputType_required: BooleanInput;
304319
static ngAcceptInputType_checked: BooleanInput;

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ describe('MatSlideToggle without forms', () => {
333333
expect(slideToggleElement.classList).toContain('mat-slide-toggle-label-before');
334334
});
335335

336-
it('should show ripples on label mousedown', () => {
336+
it('should show ripples', () => {
337337
const rippleSelector = '.mat-ripple-element:not(.mat-slide-toggle-persistent-ripple)';
338338

339339
expect(slideToggleElement.querySelectorAll(rippleSelector).length).toBe(0);
@@ -408,7 +408,8 @@ describe('MatSlideToggle without forms', () => {
408408
});
409409

410410
describe('custom action configuration', () => {
411-
it('should not change value on click when click action is noop', () => {
411+
it('should not change value on click when click action is noop when using custom a ' +
412+
'action configuration', () => {
412413
TestBed
413414
.resetTestingModule()
414415
.configureTestingModule({

0 commit comments

Comments
 (0)