From 6693ccb3ba636fd83730cd90983514ffa0550e04 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 27 Mar 2022 19:13:50 +0200 Subject: [PATCH] fix(material/tabs): focus wrapping back to selected label when using shift + tab (#14194) Currently the `tabindex` of each of the tab labels is determined by the selected index. This means that if the user tabbed into the header, pressed the right arrow and then pressed shift + tab, their focus would end up on the selected tab. These changes switch to basing the `tabindex` on the focused index which is tied both to the selected index and the user's keyboard navigation. --- .../mdc-tabs/tab-group.spec.ts | 19 ++++++++++++++++++- src/material/tabs/tab-group.spec.ts | 19 ++++++++++++++++++- src/material/tabs/tab-group.ts | 11 +++++++++-- tools/public_api_guard/material/tabs.md | 2 +- 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/material-experimental/mdc-tabs/tab-group.spec.ts b/src/material-experimental/mdc-tabs/tab-group.spec.ts index 91392e711386..b4cbfe26c874 100644 --- a/src/material-experimental/mdc-tabs/tab-group.spec.ts +++ b/src/material-experimental/mdc-tabs/tab-group.spec.ts @@ -1,4 +1,4 @@ -import {LEFT_ARROW} from '@angular/cdk/keycodes'; +import {LEFT_ARROW, RIGHT_ARROW} from '@angular/cdk/keycodes'; import {dispatchFakeEvent, dispatchKeyboardEvent} from '../../cdk/testing/private'; import {Component, DebugElement, OnInit, QueryList, ViewChild, ViewChildren} from '@angular/core'; import { @@ -378,6 +378,23 @@ describe('MDC-based MatTabGroup', () => { expect(contentElements.map(e => e.getAttribute('tabindex'))).toEqual(['1', null, null]); }); + + it('should update the tabindex of the labels when navigating via keyboard', () => { + fixture.detectChanges(); + + const tabLabels = fixture.debugElement + .queryAll(By.css('.mat-mdc-tab')) + .map(label => label.nativeElement); + const tabLabelContainer = fixture.debugElement.query(By.css('.mat-mdc-tab-label-container')) + .nativeElement as HTMLElement; + + expect(tabLabels.map(label => label.getAttribute('tabindex'))).toEqual(['-1', '0', '-1']); + + dispatchKeyboardEvent(tabLabelContainer, 'keydown', RIGHT_ARROW); + fixture.detectChanges(); + + expect(tabLabels.map(label => label.getAttribute('tabindex'))).toEqual(['-1', '-1', '0']); + }); }); describe('aria labelling', () => { diff --git a/src/material/tabs/tab-group.spec.ts b/src/material/tabs/tab-group.spec.ts index be18b08293d3..5a28bb8da8e4 100644 --- a/src/material/tabs/tab-group.spec.ts +++ b/src/material/tabs/tab-group.spec.ts @@ -1,4 +1,4 @@ -import {LEFT_ARROW} from '@angular/cdk/keycodes'; +import {LEFT_ARROW, RIGHT_ARROW} from '@angular/cdk/keycodes'; import {dispatchFakeEvent, dispatchKeyboardEvent} from '../../cdk/testing/private'; import {Component, DebugElement, OnInit, QueryList, ViewChild, ViewChildren} from '@angular/core'; import { @@ -376,6 +376,23 @@ describe('MatTabGroup', () => { expect(contentElements.map(e => e.getAttribute('tabindex'))).toEqual(['1', null, null]); }); + + it('should update the tabindex of the labels when navigating via keyboard', () => { + fixture.detectChanges(); + + const tabLabels = fixture.debugElement + .queryAll(By.css('.mat-tab-label')) + .map(label => label.nativeElement); + const tabLabelContainer = fixture.debugElement.query(By.css('.mat-tab-label-container')) + .nativeElement as HTMLElement; + + expect(tabLabels.map(label => label.getAttribute('tabindex'))).toEqual(['-1', '0', '-1']); + + dispatchKeyboardEvent(tabLabelContainer, 'keydown', RIGHT_ARROW); + fixture.detectChanges(); + + expect(tabLabels.map(label => label.getAttribute('tabindex'))).toEqual(['-1', '-1', '0']); + }); }); describe('aria labelling', () => { diff --git a/src/material/tabs/tab-group.ts b/src/material/tabs/tab-group.ts index 1a47833b9844..71c48fd81570 100644 --- a/src/material/tabs/tab-group.ts +++ b/src/material/tabs/tab-group.ts @@ -99,6 +99,9 @@ export abstract class _MatTabGroupBase /** The tab index that should be selected after the content has been checked. */ private _indexToSelect: number | null = 0; + /** Index of the tab that was focused last. */ + private _lastFocusedTabIndex: number | null = null; + /** Snapshot of the height of the tab body wrapper before another tab is activated. */ private _tabBodyWrapperHeight: number = 0; @@ -288,6 +291,7 @@ export abstract class _MatTabGroupBase if (this._selectedIndex !== indexToSelect) { this._selectedIndex = indexToSelect; + this._lastFocusedTabIndex = null; this._changeDetectorRef.markForCheck(); } } @@ -313,6 +317,7 @@ export abstract class _MatTabGroupBase // event, otherwise the consumer may end up in an infinite loop in some edge cases like // adding a tab within the `selectedIndexChange` event. this._indexToSelect = this._selectedIndex = i; + this._lastFocusedTabIndex = null; selectedTab = tabs[i]; break; } @@ -387,6 +392,7 @@ export abstract class _MatTabGroupBase } _focusChanged(index: number) { + this._lastFocusedTabIndex = index; this.focusChange.emit(this._createChangeEvent(index)); } @@ -469,11 +475,12 @@ export abstract class _MatTabGroupBase } /** Retrieves the tabindex for the tab. */ - _getTabIndex(tab: MatTab, idx: number): number | null { + _getTabIndex(tab: MatTab, index: number): number | null { if (tab.disabled) { return null; } - return this.selectedIndex === idx ? 0 : -1; + const targetIndex = this._lastFocusedTabIndex ?? this.selectedIndex; + return index === targetIndex ? 0 : -1; } /** Callback for when the focused state of a tab has changed. */ diff --git a/tools/public_api_guard/material/tabs.md b/tools/public_api_guard/material/tabs.md index 4b17c414793b..c24a57a1076d 100644 --- a/tools/public_api_guard/material/tabs.md +++ b/tools/public_api_guard/material/tabs.md @@ -242,7 +242,7 @@ export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements _focusChanged(index: number): void; focusTab(index: number): void; _getTabContentId(i: number): string; - _getTabIndex(tab: MatTab, idx: number): number | null; + _getTabIndex(tab: MatTab, index: number): number | null; _getTabLabelId(i: number): string; _handleClick(tab: MatTab, tabHeader: MatTabGroupBaseHeader, index: number): void; headerPosition: MatTabHeaderPosition;