diff --git a/src/lib/tabs/tab-group.spec.ts b/src/lib/tabs/tab-group.spec.ts index 4510d2c5532..cb456caa681 100644 --- a/src/lib/tabs/tab-group.spec.ts +++ b/src/lib/tabs/tab-group.spec.ts @@ -427,25 +427,35 @@ describe('MatTabGroup', () => { it('should maintain the selected tab if a tab is removed', () => { - // Add a couple of tabs so we have more to work with. - fixture.componentInstance.tabs.push( - {label: 'New tab', content: 'with new content'}, - {label: 'Another new tab', content: 'with newer content'} - ); - - // Select the second-to-last tab. - fixture.componentInstance.selectedIndex = 3; + // Select the second tab. + fixture.componentInstance.selectedIndex = 1; fixture.detectChanges(); const component: MatTabGroup = fixture.debugElement.query(By.css('mat-tab-group')).componentInstance; - // Remove a tab right before the selected one. - fixture.componentInstance.tabs.splice(2, 1); + // Remove the first tab that is right before the selected one. + fixture.componentInstance.tabs.splice(0, 1); fixture.detectChanges(); - expect(component.selectedIndex).toBe(1); - expect(component._tabs.toArray()[1].isActive).toBe(true); + // Since the first tab has been removed and the second one was selected before, the selected + // tab moved one position to the right. Meaning that the tab is now the first tab. + expect(component.selectedIndex).toBe(0); + expect(component._tabs.toArray()[0].isActive).toBe(true); + }); + + it('should be able to select a new tab after creation', () => { + fixture.detectChanges(); + const component: MatTabGroup = + fixture.debugElement.query(By.css('mat-tab-group')).componentInstance; + + fixture.componentInstance.tabs.push({label: 'Last tab', content: 'at the end'}); + fixture.componentInstance.selectedIndex = 3; + + fixture.detectChanges(); + + expect(component.selectedIndex).toBe(3); + expect(component._tabs.toArray()[3].isActive).toBe(true); }); it('should not fire `selectedTabChange` when the amount of tabs changes', fakeAsync(() => { diff --git a/src/lib/tabs/tab-group.ts b/src/lib/tabs/tab-group.ts index 198c43330c7..5ef2df2273d 100644 --- a/src/lib/tabs/tab-group.ts +++ b/src/lib/tabs/tab-group.ts @@ -159,12 +159,9 @@ export class MatTabGroup extends _MatTabGroupMixinBase implements AfterContentIn * a new selected tab should transition in (from the left or right). */ ngAfterContentChecked() { - // Clamp the next selected index to the bounds of 0 and the tabs length. - // Note the `|| 0`, which ensures that values like NaN can't get through - // and which would otherwise throw the component into an infinite loop - // (since Math.max(NaN, 0) === NaN). - let indexToSelect = this._indexToSelect = - Math.min(this._tabs.length - 1, Math.max(this._indexToSelect || 0, 0)); + // Don't clamp the `indexToSelect` immediately in the setter because it can happen that + // the amount of tabs changes before the actual change detection runs. + const indexToSelect = this._indexToSelect = this._clampTabIndex(this._indexToSelect); // If there is a change in selected index, emit a change event. Should not trigger if // the selected index has not yet been initialized. @@ -200,16 +197,21 @@ export class MatTabGroup extends _MatTabGroupMixinBase implements AfterContentIn // Subscribe to changes in the amount of tabs, in order to be // able to re-render the content as new tabs are added or removed. this._tabsSubscription = this._tabs.changes.subscribe(() => { - const tabs = this._tabs.toArray(); + const indexToSelect = this._clampTabIndex(this._indexToSelect); - // Maintain the previously-selected tab if a new tab is added or removed. - for (let i = 0; i < tabs.length; i++) { - if (tabs[i].isActive) { - // Assign both to the `_indexToSelect` and `_selectedIndex` so we don't fire a changed - // event, otherwise the consumer may end up in an infinite loop in some edge cases like - // adding a tab within the `selectedIndexChange` event. - this._indexToSelect = this._selectedIndex = i; - break; + // Maintain the previously-selected tab if a new tab is added or removed and there is no + // explicit change that selects a different tab. + if (indexToSelect === this._selectedIndex) { + const tabs = this._tabs.toArray(); + + for (let i = 0; i < tabs.length; i++) { + if (tabs[i].isActive) { + // Assign both to the `_indexToSelect` and `_selectedIndex` so we don't fire a changed + // event, otherwise the consumer may end up in an infinite loop in some edge cases like + // adding a tab within the `selectedIndexChange` event. + this._indexToSelect = this._selectedIndex = i; + break; + } } } @@ -261,6 +263,14 @@ export class MatTabGroup extends _MatTabGroupMixinBase implements AfterContentIn }); } + /** Clamps the given index to the bounds of 0 and the tabs length. */ + private _clampTabIndex(index: number | null): number { + // Note the `|| 0`, which ensures that values like NaN can't get through + // and which would otherwise throw the component into an infinite loop + // (since Math.max(NaN, 0) === NaN). + return Math.min(this._tabs.length - 1, Math.max(index || 0, 0)); + } + /** Returns a unique id for each tab label element */ _getTabLabelId(i: number): string { return `mat-tab-label-${this._groupId}-${i}`;