Skip to content

Commit b533e61

Browse files
authored
fix(material/slide-toggle): remove tabindex from host node (#23891)
The `tabindex="-1"` on the host node was causing VoicerOver to read out the slide toggle as a group and to read out the label twice. These changes remove the `tabindex` like we've done for `mat-radio` and `mat-checkbox` in the past.
1 parent f69cf6c commit b533e61

File tree

4 files changed

+9
-40
lines changed

4 files changed

+9
-40
lines changed

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

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -287,16 +287,6 @@ describe('MDC-based MatSlideToggle without forms', () => {
287287
expect(document.activeElement).toBe(buttonElement);
288288
}));
289289

290-
it('should focus on underlying element when the host is focused', fakeAsync(() => {
291-
expect(document.activeElement).not.toBe(buttonElement);
292-
293-
slideToggleElement.focus();
294-
fixture.detectChanges();
295-
tick();
296-
297-
expect(document.activeElement).toBe(buttonElement);
298-
}));
299-
300290
it('should not manually move focus to underlying when focus comes from mouse or touch', fakeAsync(
301291
inject([FocusMonitor], (focusMonitor: FocusMonitor) => {
302292
expect(document.activeElement).not.toBe(buttonElement);
@@ -397,13 +387,13 @@ describe('MDC-based MatSlideToggle without forms', () => {
397387
expect(switchEl.classList).toContain('mdc-switch--checked');
398388
}));
399389

400-
it('should set the tabindex of the host element to -1', fakeAsync(() => {
390+
it('should remove the tabindex from the host node', fakeAsync(() => {
401391
const fixture = TestBed.createComponent(SlideToggleWithTabindexAttr);
402392

403393
fixture.detectChanges();
404394

405395
const slideToggle = fixture.debugElement.query(By.directive(MatSlideToggle))!.nativeElement;
406-
expect(slideToggle.getAttribute('tabindex')).toBe('-1');
396+
expect(slideToggle.hasAttribute('tabindex')).toBe(false);
407397
}));
408398

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

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ export class MatSlideToggleChange {
6666
host: {
6767
'class': 'mat-mdc-slide-toggle',
6868
'[id]': 'id',
69-
// Needs to be `-1` so it can still receive programmatic focus.
70-
'[attr.tabindex]': 'disabled ? null : -1',
69+
// Needs to be removed since it causes some a11y issues (see #21266).
70+
'[attr.tabindex]': 'null',
7171
'[attr.aria-label]': 'null',
7272
'[attr.aria-labelledby]': 'null',
7373
'[class.mat-primary]': 'color === "primary"',
@@ -221,12 +221,7 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
221221
foundation.setChecked(this.checked);
222222

223223
this._focusMonitor.monitor(this._elementRef, true).subscribe(focusOrigin => {
224-
// Only forward focus manually when it was received programmatically or through the
225-
// keyboard. We should not do this for mouse/touch focus for two reasons:
226-
// 1. It can prevent clicks from landing in Chrome (see #18269).
227-
// 2. They're already handled by the wrapping `label` element.
228224
if (focusOrigin === 'keyboard' || focusOrigin === 'program') {
229-
this._switchElement.nativeElement.focus();
230225
this._focused = true;
231226
} else if (!focusOrigin) {
232227
// When a focused element becomes disabled, the browser *immediately* fires a blur event.

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

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -318,16 +318,6 @@ describe('MatSlideToggle without forms', () => {
318318
expect(document.activeElement).toBe(inputElement);
319319
}));
320320

321-
it('should focus on underlying element when the host is focused', fakeAsync(() => {
322-
expect(document.activeElement).not.toBe(inputElement);
323-
324-
slideToggleElement.focus();
325-
fixture.detectChanges();
326-
flush();
327-
328-
expect(document.activeElement).toBe(inputElement);
329-
}));
330-
331321
it('should not manually move focus to underlying when focus comes from mouse or touch', inject(
332322
[FocusMonitor],
333323
(focusMonitor: FocusMonitor) => {
@@ -410,13 +400,13 @@ describe('MatSlideToggle without forms', () => {
410400
.toBe(5);
411401
}));
412402

413-
it('should set the tabindex of the host element to -1', fakeAsync(() => {
403+
it('should remove the tabindex from the host node', fakeAsync(() => {
414404
const fixture = TestBed.createComponent(SlideToggleWithTabindexAttr);
415405

416406
fixture.detectChanges();
417407

418408
const slideToggle = fixture.debugElement.query(By.directive(MatSlideToggle))!.nativeElement;
419-
expect(slideToggle.getAttribute('tabindex')).toBe('-1');
409+
expect(slideToggle.hasAttribute('tabindex')).toBe(false);
420410
}));
421411

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

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ const _MatSlideToggleBase = mixinTabIndex(
8383
host: {
8484
'class': 'mat-slide-toggle',
8585
'[id]': 'id',
86-
// Needs to be `-1` so it can still receive programmatic focus.
87-
'[attr.tabindex]': 'disabled ? null : -1',
86+
// Needs to be removed since it causes some a11y issues (see #21266).
87+
'[attr.tabindex]': 'null',
8888
'[attr.aria-label]': 'null',
8989
'[attr.aria-labelledby]': 'null',
9090
'[class.mat-checked]': 'checked',
@@ -198,13 +198,7 @@ export class MatSlideToggle
198198

199199
ngAfterContentInit() {
200200
this._focusMonitor.monitor(this._elementRef, true).subscribe(focusOrigin => {
201-
// Only forward focus manually when it was received programmatically or through the
202-
// keyboard. We should not do this for mouse/touch focus for two reasons:
203-
// 1. It can prevent clicks from landing in Chrome (see #18269).
204-
// 2. They're already handled by the wrapping `label` element.
205-
if (focusOrigin === 'keyboard' || focusOrigin === 'program') {
206-
this._inputElement.nativeElement.focus();
207-
} else if (!focusOrigin) {
201+
if (!focusOrigin) {
208202
// When a focused element becomes disabled, the browser *immediately* fires a blur event.
209203
// Angular does not expect events to be raised during change detection, so any state
210204
// change (such as a form control's 'ng-touched') will cause a changed-after-checked

0 commit comments

Comments
 (0)