fix(tabs): selectedIndex being overwritten if tabs are being added / removed

Due to a recent change that ensures that the selected tab will be kept selected if a new tab has been added or removed (#9132), updating the `selectedIndex` at the same time will not have any effect because it will be overwritten by the `_tabs.change` (from #9132).

In order to guarantee that developers can add new tabs and immediately select them once the change detection runs, we only re-index the `selectedIndex` (purpose of #9132) whenever the `indexToSelect` has not explicitly changed (through developer bindings for example)

Fixes #12038

Author: devversion

src/lib/tabs/tab-group.spec.ts

@@ -382,25 +382,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(() => {

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 boundsof 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));
+    // Clamp the `indexToSelect` not 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._clampIndexToSelect();
 
     // 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();
-
-      // 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;
+      const indexToSelect = this._clampIndexToSelect();
+
+      // 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 indexToSelect to the bounds of 0 and the tabs length. */
+  private _clampIndexToSelect(): 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(this._indexToSelect || 0, 0));
+  }
+
   /** Returns a unique id for each tab label element */
   _getTabLabelId(i: number): string {
     return `mat-tab-label-${this._groupId}-${i}`;