From 60a352728aa8b058d03f39df7697220582cf2d71 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 16 Sep 2020 04:28:26 +0200 Subject: [PATCH] fix(tabs): always defaulting focusIndex to 0 on initialization (#20384) We were always defaulting the `focusIndex` to 0 on init, rather than taking the index of the selected tab. We actually had tests for this behavior, but they were all testing against 0 so we never caught the issue. Fixes #20374. (cherry picked from commit a0b44d4b6d2a249f621fac1763b04bf0933c789c) --- src/material-experimental/mdc-tabs/tab-group.spec.ts | 10 +++++----- src/material-experimental/mdc-tabs/tab-header.spec.ts | 9 ++++++++- src/material/tabs/paginated-tab-header.ts | 2 +- src/material/tabs/tab-group.spec.ts | 10 +++++----- src/material/tabs/tab-header.spec.ts | 9 ++++++++- 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/material-experimental/mdc-tabs/tab-group.spec.ts b/src/material-experimental/mdc-tabs/tab-group.spec.ts index 19c96cb61536..c16fe89f60f4 100644 --- a/src/material-experimental/mdc-tabs/tab-group.spec.ts +++ b/src/material-experimental/mdc-tabs/tab-group.spec.ts @@ -248,12 +248,12 @@ describe('MDC-based MatTabGroup', () => { expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(0); - tabLabels[1].nativeElement.click(); + tabLabels[2].nativeElement.click(); fixture.detectChanges(); expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(1); expect(fixture.componentInstance.handleFocus) - .toHaveBeenCalledWith(jasmine.objectContaining({index: 1})); + .toHaveBeenCalledWith(jasmine.objectContaining({index: 2})); }); it('should emit focusChange on arrow key navigation', () => { @@ -267,8 +267,8 @@ describe('MDC-based MatTabGroup', () => { 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(); + // index, we focus the third tab before testing the keyboard navigation. + tabLabels[2].nativeElement.click(); fixture.detectChanges(); expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(1); @@ -277,7 +277,7 @@ describe('MDC-based MatTabGroup', () => { expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(2); expect(fixture.componentInstance.handleFocus) - .toHaveBeenCalledWith(jasmine.objectContaining({index: 0})); + .toHaveBeenCalledWith(jasmine.objectContaining({index: 1})); }); it('should clean up the tabs QueryList on destroy', () => { diff --git a/src/material-experimental/mdc-tabs/tab-header.spec.ts b/src/material-experimental/mdc-tabs/tab-header.spec.ts index be5778268e8e..186ed8699343 100644 --- a/src/material-experimental/mdc-tabs/tab-header.spec.ts +++ b/src/material-experimental/mdc-tabs/tab-header.spec.ts @@ -63,8 +63,15 @@ describe('MDC-based MatTabHeader', () => { }); it('should initialize to the selected index', () => { + // Recreate the fixture so we can test that it works with a non-default selected index + fixture.destroy(); + fixture = TestBed.createComponent(SimpleTabHeaderApp); + fixture.componentInstance.selectedIndex = 1; fixture.detectChanges(); - expect(appComponent.tabHeader.focusIndex).toBe(appComponent.selectedIndex); + appComponent = fixture.componentInstance; + tabListContainer = appComponent.tabHeader._tabListContainer.nativeElement; + + expect(appComponent.tabHeader.focusIndex).toBe(1); }); it('should send focus change event', () => { diff --git a/src/material/tabs/paginated-tab-header.ts b/src/material/tabs/paginated-tab-header.ts index 6756352d215a..a33e1b3d7d41 100644 --- a/src/material/tabs/paginated-tab-header.ts +++ b/src/material/tabs/paginated-tab-header.ts @@ -196,7 +196,7 @@ export abstract class MatPaginatedTabHeader implements AfterContentChecked, Afte .withHomeAndEnd() .withWrap(); - this._keyManager.updateActiveItem(0); + this._keyManager.updateActiveItem(this._selectedIndex); // 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. diff --git a/src/material/tabs/tab-group.spec.ts b/src/material/tabs/tab-group.spec.ts index db724031e912..9bc77399cce9 100644 --- a/src/material/tabs/tab-group.spec.ts +++ b/src/material/tabs/tab-group.spec.ts @@ -247,12 +247,12 @@ describe('MatTabGroup', () => { expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(0); - tabLabels[1].nativeElement.click(); + tabLabels[2].nativeElement.click(); fixture.detectChanges(); expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(1); expect(fixture.componentInstance.handleFocus) - .toHaveBeenCalledWith(jasmine.objectContaining({index: 1})); + .toHaveBeenCalledWith(jasmine.objectContaining({index: 2})); }); it('should emit focusChange on arrow key navigation', () => { @@ -266,8 +266,8 @@ describe('MatTabGroup', () => { 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(); + // index, we focus the third tab before testing the keyboard navigation. + tabLabels[2].nativeElement.click(); fixture.detectChanges(); expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(1); @@ -276,7 +276,7 @@ describe('MatTabGroup', () => { expect(fixture.componentInstance.handleFocus).toHaveBeenCalledTimes(2); expect(fixture.componentInstance.handleFocus) - .toHaveBeenCalledWith(jasmine.objectContaining({index: 0})); + .toHaveBeenCalledWith(jasmine.objectContaining({index: 1})); }); it('should clean up the tabs QueryList on destroy', () => { diff --git a/src/material/tabs/tab-header.spec.ts b/src/material/tabs/tab-header.spec.ts index 5295856809ca..f44459712e5f 100644 --- a/src/material/tabs/tab-header.spec.ts +++ b/src/material/tabs/tab-header.spec.ts @@ -65,8 +65,15 @@ describe('MatTabHeader', () => { }); it('should initialize to the selected index', () => { + // Recreate the fixture so we can test that it works with a non-default selected index + fixture.destroy(); + fixture = TestBed.createComponent(SimpleTabHeaderApp); + fixture.componentInstance.selectedIndex = 1; fixture.detectChanges(); - expect(appComponent.tabHeader.focusIndex).toBe(appComponent.selectedIndex); + appComponent = fixture.componentInstance; + tabListContainer = appComponent.tabHeader._tabListContainer.nativeElement; + + expect(appComponent.tabHeader.focusIndex).toBe(1); }); it('should send focus change event', () => {