From 1b7b8ab35d1800ecf5da1d21d7270e1d0323706f Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 20 Jul 2018 16:34:09 +0200 Subject: [PATCH] fix(tab-group): focus change event not firing for keyboard navigation (#12192) --- src/lib/tabs/tab-group.spec.ts | 43 +++++++++++++++++++++++++++++++++- src/lib/tabs/tab-header.ts | 30 +++++++++++++++++------- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/lib/tabs/tab-group.spec.ts b/src/lib/tabs/tab-group.spec.ts index 65ebcbcef4fd..4510d2c55323 100644 --- a/src/lib/tabs/tab-group.spec.ts +++ b/src/lib/tabs/tab-group.spec.ts @@ -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'; @@ -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', () => { diff --git a/src/lib/tabs/tab-header.ts b/src/lib/tabs/tab-header.ts index af19791430e1..6efc0c9fee36 100644 --- a/src/lib/tabs/tab-header.ts +++ b/src/lib/tabs/tab-header.ts @@ -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'; @@ -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(); /** Whether the controls for pagination should be displayed */ _showPaginationControls = false; @@ -201,20 +202,31 @@ export class MatTabHeader extends _MatTabHeaderMixinBase .withHorizontalOrientation(this._getLayoutDirection()) .withWrap(); - 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 => { + this.indexFocused.emit(newFocusIndex); + this._setTabFocus(newFocusIndex); + }); } ngOnDestroy() { - this._realignInkBar.unsubscribe(); + this._destroyed.next(); + this._destroyed.complete(); } /** @@ -242,11 +254,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); } /**