Skip to content

Commit

Permalink
fix(material/tabs): focus wrapping back to selected label when using …
Browse files Browse the repository at this point in the history
…shift + tab (angular#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.
  • Loading branch information
crisbeto authored and forsti0506 committed Apr 3, 2022
1 parent 1277c5a commit 6693ccb
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 5 deletions.
19 changes: 18 additions & 1 deletion src/material-experimental/mdc-tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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', () => {
Expand Down
19 changes: 18 additions & 1 deletion src/material/tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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', () => {
Expand Down
11 changes: 9 additions & 2 deletions src/material/tabs/tab-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -288,6 +291,7 @@ export abstract class _MatTabGroupBase

if (this._selectedIndex !== indexToSelect) {
this._selectedIndex = indexToSelect;
this._lastFocusedTabIndex = null;
this._changeDetectorRef.markForCheck();
}
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -387,6 +392,7 @@ export abstract class _MatTabGroupBase
}

_focusChanged(index: number) {
this._lastFocusedTabIndex = index;
this.focusChange.emit(this._createChangeEvent(index));
}

Expand Down Expand Up @@ -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. */
Expand Down
2 changes: 1 addition & 1 deletion tools/public_api_guard/material/tabs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 6693ccb

Please sign in to comment.