Skip to content

Commit

Permalink
fix(tabs): always defaulting focusIndex to 0 on initialization (#20384)
Browse files Browse the repository at this point in the history
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 a0b44d4)
  • Loading branch information
crisbeto authored and wagnermaciel committed Sep 16, 2020
1 parent f7e0f31 commit 60a3527
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 13 deletions.
10 changes: 5 additions & 5 deletions src/material-experimental/mdc-tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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);
Expand All @@ -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', () => {
Expand Down
9 changes: 8 additions & 1 deletion src/material-experimental/mdc-tabs/tab-header.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/material/tabs/paginated-tab-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions src/material/tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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);
Expand All @@ -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', () => {
Expand Down
9 changes: 8 additions & 1 deletion src/material/tabs/tab-header.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 60a3527

Please sign in to comment.