Skip to content

fix(material/tabs): allow focus on disabled tabs #26397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/material/legacy-tabs/_tabs-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
@each $name, $color in $theme-colors {
// Set the foreground color of the tabs
&.mat-#{$name} {
@include _label-focus-color($color);
@include _label-focus-color($foreground, $color);
@include _ink-bar-color($color);

// Override ink bar when background color is the same
Expand All @@ -75,7 +75,7 @@
@each $name, $color in $theme-colors {
// Set background color of the tabs and override focus color
&.mat-background-#{$name} {
@include _label-focus-color($color);
@include _label-focus-color($foreground, $color);
@include _tabs-background($color);
}
}
Expand All @@ -88,13 +88,15 @@
}
}

@mixin _label-focus-color($tab-focus-color) {
@mixin _label-focus-color($foreground, $tab-focus-color) {
.mat-tab-label,
.mat-tab-link {
&.cdk-keyboard-focused,
&.cdk-program-focused {
&:not(.mat-tab-disabled) {
background-color: theming.get-color-from-palette($tab-focus-color, lighter, 0.3);
background-color: theming.get-color-from-palette($tab-focus-color, lighter, 0.3);

&.mat-tab-disabled {
background-color: theming.get-color-from-palette($foreground, disabled, 0.1);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/material/legacy-tabs/tab-group.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
cdkMonitorElementFocus
*ngFor="let tab of _tabs; let i = index"
[id]="_getTabLabelId(i)"
[attr.tabIndex]="_getTabIndex(tab, i)"
[attr.tabIndex]="_getTabIndex(i)"
[attr.aria-posinset]="i + 1"
[attr.aria-setsize]="_tabs.length"
[attr.aria-controls]="_getTabContentId(i)"
Expand Down
52 changes: 32 additions & 20 deletions src/material/legacy-tabs/tab-header.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,35 +74,32 @@ describe('MatTabHeader', () => {
expect(appComponent.tabHeader.focusIndex).toBe(2);
});

it('should not set focus a disabled tab', () => {
it('should be able to focus a disabled tab', () => {
appComponent.tabHeader.focusIndex = 0;
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(0);

// Set focus on the disabled tab, but focus should remain 0
appComponent.tabHeader.focusIndex = appComponent.disabledTabIndex;
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(0);
expect(appComponent.tabHeader.focusIndex).toBe(appComponent.disabledTabIndex);
});

it('should move focus right and skip disabled tabs', () => {
it('should move focus right including over disabled tabs', () => {
appComponent.tabHeader.focusIndex = 0;
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(0);

// Move focus right, verify that the disabled tab is 1 and should be skipped
expect(appComponent.disabledTabIndex).toBe(1);
dispatchKeyboardEvent(tabListContainer, 'keydown', RIGHT_ARROW);
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(2);
expect(appComponent.tabHeader.focusIndex).toBe(1);

// Move focus right to index 3
dispatchKeyboardEvent(tabListContainer, 'keydown', RIGHT_ARROW);
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(3);
expect(appComponent.tabHeader.focusIndex).toBe(2);
});

it('should move focus left and skip disabled tabs', () => {
it('should move focus left including over disabled tabs', () => {
appComponent.tabHeader.focusIndex = 3;
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(3);
Expand All @@ -112,31 +109,47 @@ describe('MatTabHeader', () => {
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(2);

// Move focus left, verify that the disabled tab is 1 and should be skipped
expect(appComponent.disabledTabIndex).toBe(1);
dispatchKeyboardEvent(tabListContainer, 'keydown', LEFT_ARROW);
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(0);
expect(appComponent.tabHeader.focusIndex).toBe(1);
});

it('should support key down events to move and select focus', () => {
appComponent.tabHeader.focusIndex = 0;
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(0);

// Move focus right to 1
dispatchKeyboardEvent(tabListContainer, 'keydown', RIGHT_ARROW);
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(1);

// Try to select 1. Should not work since it's disabled.
expect(appComponent.selectedIndex).toBe(0);
const firstEnterEvent = dispatchKeyboardEvent(tabListContainer, 'keydown', ENTER);
fixture.detectChanges();
expect(appComponent.selectedIndex).toBe(0);
expect(firstEnterEvent.defaultPrevented).toBe(false);

// Move focus right to 2
dispatchKeyboardEvent(tabListContainer, 'keydown', RIGHT_ARROW);
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(2);

// Select the focused index 2
// Select 2 which is enabled.
expect(appComponent.selectedIndex).toBe(0);
const enterEvent = dispatchKeyboardEvent(tabListContainer, 'keydown', ENTER);
const secondEnterEvent = dispatchKeyboardEvent(tabListContainer, 'keydown', ENTER);
fixture.detectChanges();
expect(appComponent.selectedIndex).toBe(2);
expect(enterEvent.defaultPrevented).toBe(true);
expect(secondEnterEvent.defaultPrevented).toBe(true);

// Move focus left to 1
dispatchKeyboardEvent(tabListContainer, 'keydown', LEFT_ARROW);
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(1);

// Move focus right to 0
// Move again to 0
dispatchKeyboardEvent(tabListContainer, 'keydown', LEFT_ARROW);
fixture.detectChanges();
expect(appComponent.tabHeader.focusIndex).toBe(0);
Expand Down Expand Up @@ -174,7 +187,7 @@ describe('MatTabHeader', () => {
expect(event.defaultPrevented).toBe(true);
});

it('should skip disabled items when moving focus using HOME', () => {
it('should focus disabled items when moving focus using HOME', () => {
appComponent.tabHeader.focusIndex = 3;
appComponent.tabs[0].disabled = true;
fixture.detectChanges();
Expand All @@ -183,8 +196,7 @@ describe('MatTabHeader', () => {
dispatchKeyboardEvent(tabListContainer, 'keydown', HOME);
fixture.detectChanges();

// Note that the second tab is disabled by default already.
expect(appComponent.tabHeader.focusIndex).toBe(2);
expect(appComponent.tabHeader.focusIndex).toBe(0);
});

it('should move focus to the last tab when pressing END', () => {
Expand All @@ -199,7 +211,7 @@ describe('MatTabHeader', () => {
expect(event.defaultPrevented).toBe(true);
});

it('should skip disabled items when moving focus using END', () => {
it('should focus disabled items when moving focus using END', () => {
appComponent.tabHeader.focusIndex = 0;
appComponent.tabs[3].disabled = true;
fixture.detectChanges();
Expand All @@ -208,7 +220,7 @@ describe('MatTabHeader', () => {
dispatchKeyboardEvent(tabListContainer, 'keydown', END);
fixture.detectChanges();

expect(appComponent.tabHeader.focusIndex).toBe(2);
expect(appComponent.tabHeader.focusIndex).toBe(3);
});

it('should not do anything if a modifier key is pressed', () => {
Expand Down
8 changes: 8 additions & 0 deletions src/material/tabs/_tabs-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
$primary: theming.get-color-from-palette(map.get($config, primary));
$accent: theming.get-color-from-palette(map.get($config, accent));
$warn: theming.get-color-from-palette(map.get($config, warn));
$foreground: map.get($config, foreground);

@include mdc-helpers.using-mdc-theme($config) {
.mat-mdc-tab, .mat-mdc-tab-link {
Expand All @@ -27,6 +28,13 @@
// Disable for now for backwards compatibility. These styles are inside the theme in order
// to avoid CSS specificity issues.
background-color: transparent;

&.mat-mdc-tab-disabled {
.mdc-tab__ripple::before,
.mat-ripple-element {
background-color: theming.get-color-from-palette($foreground, disabled);
}
}
}

@include _palette-styles($primary);
Expand Down
19 changes: 10 additions & 9 deletions src/material/tabs/paginated-tab-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,9 @@ export abstract class MatPaginatedTabHeader
this._keyManager = new FocusKeyManager<MatPaginatedTabHeaderItem>(this._items)
.withHorizontalOrientation(this._getLayoutDirection())
.withHomeAndEnd()
.withWrap();
.withWrap()
// Allow focus to land on disabled tabs, as per https://w3c.github.io/aria-practices/#kbd_disabled_controls
.skipPredicate(() => false);

this._keyManager.updateActiveItem(this._selectedIndex);

Expand Down Expand Up @@ -329,8 +331,12 @@ export abstract class MatPaginatedTabHeader
case ENTER:
case SPACE:
if (this.focusIndex !== this.selectedIndex) {
this.selectFocusedIndex.emit(this.focusIndex);
this._itemSelected(event);
const item = this._items.get(this.focusIndex);

if (item && !item.disabled) {
this.selectFocusedIndex.emit(this.focusIndex);
this._itemSelected(event);
}
}
break;
default:
Expand Down Expand Up @@ -392,12 +398,7 @@ export abstract class MatPaginatedTabHeader
* providing a valid index and return true.
*/
_isValidIndex(index: number): boolean {
if (!this._items) {
return true;
}

const tab = this._items ? this._items.toArray()[index] : null;
return !!tab && !tab.disabled;
return this._items ? !!this._items.toArray()[index] : true;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/material/tabs/tab-group.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
cdkMonitorElementFocus
*ngFor="let tab of _tabs; let i = index"
[id]="_getTabLabelId(i)"
[attr.tabIndex]="_getTabIndex(tab, i)"
[attr.tabIndex]="_getTabIndex(i)"
[attr.aria-posinset]="i + 1"
[attr.aria-setsize]="_tabs.length"
[attr.aria-controls]="_getTabContentId(i)"
Expand Down
1 change: 0 additions & 1 deletion src/material/tabs/tab-group.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
// MDC doesn't support disabled tabs so we need to improvise.
.mat-mdc-tab-disabled {
opacity: 0.4;
pointer-events: none;
}

.mat-mdc-tab-group {
Expand Down
9 changes: 4 additions & 5 deletions src/material/tabs/tab-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,16 +477,15 @@ export abstract class _MatTabGroupBase

/** Handle click events, setting new selected index if appropriate. */
_handleClick(tab: MatTab, tabHeader: MatTabGroupBaseHeader, index: number) {
tabHeader.focusIndex = index;

if (!tab.disabled) {
this.selectedIndex = tabHeader.focusIndex = index;
this.selectedIndex = index;
}
}

/** Retrieves the tabindex for the tab. */
_getTabIndex(tab: MatTab, index: number): number | null {
if (tab.disabled) {
return null;
}
_getTabIndex(index: number): number {
const targetIndex = this._lastFocusedTabIndex ?? this.selectedIndex;
return index === targetIndex ? 0 : -1;
}
Expand Down
Loading