Skip to content

fix(tab-group): focus change event not firing for keyboard navigation #12192

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
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
43 changes: 42 additions & 1 deletion src/lib/tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {dispatchFakeEvent} from '@angular/cdk/testing';
import {LEFT_ARROW} from '@angular/cdk/keycodes';
import {dispatchFakeEvent, dispatchKeyboardEvent} from '@angular/cdk/testing';
import {Component, OnInit, QueryList, ViewChild, ViewChildren} from '@angular/core';
import {async, ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
Expand Down Expand Up @@ -232,6 +233,46 @@ describe('MatTabGroup', () => {
expect(labels.every(label => label.getAttribute('aria-setsize') === '3')).toBe(true);
});

it('should emit focusChange event on click', () => {
spyOn(fixture.componentInstance, 'handleFocus');
fixture.detectChanges();

const tabLabels = fixture.debugElement.queryAll(By.css('.mat-tab-label'));

expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(0);

tabLabels[1].nativeElement.click();
fixture.detectChanges();

expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(1);
expect(fixture.componentInstance.handleFocus)
.toHaveBeenCalledWith(jasmine.objectContaining({index: 1}));
});

it('should emit focusChange on arrow key navigation', () => {
spyOn(fixture.componentInstance, 'handleFocus');
fixture.detectChanges();

const tabLabels = fixture.debugElement.queryAll(By.css('.mat-tab-label'));
const tabLabelContainer = fixture.debugElement
.query(By.css('.mat-tab-label-container')).nativeElement as HTMLElement;

expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(0);

// In order to verify that the `focusChange` event also fires with the correct
// index, we focus the second tab before testing the keyboard navigation.
tabLabels[1].nativeElement.click();
fixture.detectChanges();

expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(1);

dispatchKeyboardEvent(tabLabelContainer, 'keydown', LEFT_ARROW);

expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(2);
expect(fixture.componentInstance.handleFocus)
.toHaveBeenCalledWith(jasmine.objectContaining({index: 0}));
});

});

describe('aria labelling', () => {
Expand Down
30 changes: 21 additions & 9 deletions src/lib/tabs/tab-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import {
ViewEncapsulation,
} from '@angular/core';
import {CanDisableRipple, mixinDisableRipple} from '@angular/material/core';
import {merge, of as observableOf, Subscription} from 'rxjs';
import {merge, of as observableOf, Subject} from 'rxjs';
import {takeUntil} from 'rxjs/operators';
import {MatInkBar} from './ink-bar';
import {MatTabLabelWrapper} from './tab-label-wrapper';
import {FocusKeyManager} from '@angular/cdk/a11y';
Expand Down Expand Up @@ -87,8 +88,8 @@ export class MatTabHeader extends _MatTabHeaderMixinBase
/** Whether the header should scroll to the selected index after the view has been checked. */
private _selectedIndexChanged = false;

/** Combines listeners that will re-align the ink bar whenever they're invoked. */
private _realignInkBar = Subscription.EMPTY;
/** Emits when the component is destroyed. */
private readonly _destroyed = new Subject<void>();

/** Whether the controls for pagination should be displayed */
_showPaginationControls = false;
Expand Down Expand Up @@ -200,20 +201,31 @@ export class MatTabHeader extends _MatTabHeaderMixinBase
this._keyManager = new FocusKeyManager(this._labelWrappers)
.withHorizontalOrientation(this._getLayoutDirection());

this._keyManager.updateActiveItemIndex(0);
this._keyManager.updateActiveItem(0);

// Defer the first call in order to allow for slower browsers to lay out the elements.
// This helps in cases where the user lands directly on a page with paginated tabs.
typeof requestAnimationFrame !== 'undefined' ? requestAnimationFrame(realign) : realign();

this._realignInkBar = merge(dirChange, resize).subscribe(() => {
// On dir change or window resize, realign the ink bar and update the orientation of
// the key manager if the direction has changed.
merge(dirChange, resize).pipe(takeUntil(this._destroyed)).subscribe(() => {
realign();
this._keyManager.withHorizontalOrientation(this._getLayoutDirection());
});

// If there is a change in the focus key manager we need to emit the `indexFocused`
// event in order to provide a public event that notifies about focus changes. Also we realign
// the tabs container by scrolling the new focused tab into the visible section.
this._keyManager.change.pipe(takeUntil(this._destroyed)).subscribe(newFocusIndex => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will also fire for programmatic changes, but based on the old code, it seems like that's what we're expecting.

this.indexFocused.emit(newFocusIndex);
this._setTabFocus(newFocusIndex);
});
}

ngOnDestroy() {
this._realignInkBar.unsubscribe();
this._destroyed.next();
this._destroyed.complete();
}

/**
Expand Down Expand Up @@ -241,11 +253,11 @@ export class MatTabHeader extends _MatTabHeaderMixinBase

/** When the focus index is set, we must manually send focus to the correct label */
set focusIndex(value: number) {
if (!this._isValidIndex(value) || this.focusIndex == value || !this._keyManager) { return; }
if (!this._isValidIndex(value) || this.focusIndex === value || !this._keyManager) {
return;
}

this._keyManager.setActiveItem(value);
this.indexFocused.emit(value);
this._setTabFocus(value);
}

/**
Expand Down