fix(tabs): not picking up indirect descendant tabs in ivy (#17346)

In ViewEngine `ContentChildren` would pick up indirect descendant items, as long as another directive wasn't matched along the way. This allowed for tabs to be wrapped inside something like an `ng-container`. With Ivy `ContentChildren` is a little more strict and it only works on direct descendants.

These changes turn on `descendants` so that we continue supporting the existing use cases from ViewEngine. Note that it needs a bit of extra logic in order to ensure that nested tab groups continue to work as expected.

Fixes #17336.

Author: crisbeto

src/material-experimental/mdc-tabs/public-api.ts

@@ -27,4 +27,5 @@ export {
   MatTabHeaderPosition,
   MatTabsConfig,
   MAT_TABS_CONFIG,
+  MAT_TAB_GROUP,
 } from '@angular/material/tabs';

src/material-experimental/mdc-tabs/tab-group.spec.ts

@@ -23,6 +23,8 @@ describe('MatTabGroup', () => {
         TemplateTabs,
         TabGroupWithAriaInputs,
         TabGroupWithIsActiveBinding,
+        NestedTabs,
+        TabGroupWithIndirectDescendantTabs,
       ],
     });
 
@@ -589,6 +591,34 @@ describe('MatTabGroup', () => {
     }));
   });
 
+  describe('nested tabs', () => {
+    it('should not pick up the tabs from descendant tab groups', fakeAsync(() => {
+      const fixture = TestBed.createComponent(NestedTabs);
+      fixture.detectChanges();
+      tick();
+      fixture.detectChanges();
+
+      const groups = fixture.componentInstance.groups.toArray();
+
+      expect(groups.length).toBe(2);
+      expect(groups[0]._tabs.map((tab: MatTab) => tab.textLabel))
+          .toEqual(['One', 'Two']);
+      expect(groups[1]._tabs.map((tab: MatTab) => tab.textLabel))
+        .toEqual(['Inner tab one', 'Inner tab two']);
+    }));
+
+    it('should pick up indirect descendant tabs', fakeAsync(() => {
+      const fixture = TestBed.createComponent(TabGroupWithIndirectDescendantTabs);
+      fixture.detectChanges();
+      tick();
+      fixture.detectChanges();
+
+      const tabs = fixture.componentInstance.tabGroup._tabs;
+      expect(tabs.map((tab: MatTab) => tab.textLabel)).toEqual(['One', 'Two']);
+    }));
+  });
+
+
   /**
    * Checks that the `selectedIndex` has been updated; checks that the label and body have their
    * respective `active` classes
@@ -825,7 +855,9 @@ class TabGroupWithSimpleApi {
     </mat-tab-group>
   `,
 })
-class NestedTabs {}
+class NestedTabs {
+  @ViewChildren(MatTabGroup) groups: QueryList<MatTabGroup>;
+}
 
 @Component({
   selector: 'template-tabs',
@@ -881,3 +913,18 @@ class TabGroupWithIsActiveBinding {
   `,
 })
 class TabsWithCustomAnimationDuration {}
+
+
+@Component({
+  template: `
+    <mat-tab-group>
+      <ng-container [ngSwitch]="true">
+        <mat-tab label="One">Tab one content</mat-tab>
+        <mat-tab label="Two">Tab two content</mat-tab>
+      </ng-container>
+    </mat-tab-group>
+  `,
+})
+class TabGroupWithIndirectDescendantTabs {
+  @ViewChild(MatTabGroup, {static: false}) tabGroup: MatTabGroup;
+}

src/material-experimental/mdc-tabs/tab-group.ts

@@ -18,7 +18,12 @@ import {
   Inject,
   Optional,
 } from '@angular/core';
-import {_MatTabGroupBase, MAT_TABS_CONFIG, MatTabsConfig} from '@angular/material/tabs';
+import {
+  _MatTabGroupBase,
+  MAT_TABS_CONFIG,
+  MatTabsConfig,
+  MAT_TAB_GROUP,
+} from '@angular/material/tabs';
 import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
 import {MatTab} from './tab';
 import {MatTabHeader} from './tab-header';
@@ -37,14 +42,18 @@ import {MatTabHeader} from './tab-header';
   encapsulation: ViewEncapsulation.None,
   changeDetection: ChangeDetectionStrategy.OnPush,
   inputs: ['color', 'disableRipple'],
+  providers: [{
+    provide: MAT_TAB_GROUP,
+    useExisting: MatTabGroup
+  }],
   host: {
     'class': 'mat-mdc-tab-group',
     '[class.mat-mdc-tab-group-dynamic-height]': 'dynamicHeight',
     '[class.mat-mdc-tab-group-inverted-header]': 'headerPosition === "below"',
   },
 })
 export class MatTabGroup extends _MatTabGroupBase {
-  @ContentChildren(MatTab) _tabs: QueryList<MatTab>;
+  @ContentChildren(MatTab, {descendants: true}) _allTabs: QueryList<MatTab>;
   @ViewChild('tabBodyWrapper', {static: false}) _tabBodyWrapper: ElementRef;
   @ViewChild('tabHeader', {static: false}) _tabHeader: MatTabHeader;
 

src/material/tabs/public-api.ts

@@ -18,7 +18,7 @@ export {
 } from './tab-body';
 export {MatTabHeader, _MatTabHeaderBase} from './tab-header';
 export {MatTabLabelWrapper} from './tab-label-wrapper';
-export {MatTab} from './tab';
+export {MatTab, MAT_TAB_GROUP} from './tab';
 export {MatTabLabel} from './tab-label';
 export {MatTabNav, MatTabLink, _MatTabNavBase, _MatTabLinkBase} from './tab-nav-bar/index';
 export {MatTabContent} from './tab-content';

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

@@ -23,6 +23,8 @@ describe('MatTabGroup', () => {
         TemplateTabs,
         TabGroupWithAriaInputs,
         TabGroupWithIsActiveBinding,
+        NestedTabs,
+        TabGroupWithIndirectDescendantTabs,
       ],
     });
 
@@ -588,6 +590,33 @@ describe('MatTabGroup', () => {
     }));
   });
 
+  describe('nested tabs', () => {
+    it('should not pick up the tabs from descendant tab groups', fakeAsync(() => {
+      const fixture = TestBed.createComponent(NestedTabs);
+      fixture.detectChanges();
+      tick();
+      fixture.detectChanges();
+
+      const groups = fixture.componentInstance.groups.toArray();
+
+      expect(groups.length).toBe(2);
+      expect(groups[0]._tabs.map((tab: MatTab) => tab.textLabel))
+          .toEqual(['One', 'Two']);
+      expect(groups[1]._tabs.map((tab: MatTab) => tab.textLabel))
+        .toEqual(['Inner tab one', 'Inner tab two']);
+    }));
+
+    it('should pick up indirect descendant tabs', fakeAsync(() => {
+      const fixture = TestBed.createComponent(TabGroupWithIndirectDescendantTabs);
+      fixture.detectChanges();
+      tick();
+      fixture.detectChanges();
+
+      const tabs = fixture.componentInstance.tabGroup._tabs;
+      expect(tabs.map((tab: MatTab) => tab.textLabel)).toEqual(['One', 'Two']);
+    }));
+  });
+
   /**
    * Checks that the `selectedIndex` has been updated; checks that the label and body have their
    * respective `active` classes
@@ -824,7 +853,9 @@ class TabGroupWithSimpleApi {
     </mat-tab-group>
   `,
 })
-class NestedTabs {}
+class NestedTabs {
+  @ViewChildren(MatTabGroup) groups: QueryList<MatTabGroup>;
+}
 
 @Component({
   selector: 'template-tabs',
@@ -880,3 +911,18 @@ class TabGroupWithIsActiveBinding {
   `,
 })
 class TabsWithCustomAnimationDuration {}
+
+
+@Component({
+  template: `
+    <mat-tab-group>
+      <ng-container [ngSwitch]="true">
+        <mat-tab label="One">Tab one content</mat-tab>
+        <mat-tab label="Two">Tab two content</mat-tab>
+      </ng-container>
+    </mat-tab-group>
+  `,
+})
+class TabGroupWithIndirectDescendantTabs {
+  @ViewChild(MatTabGroup, {static: false}) tabGroup: MatTabGroup;
+}

src/material/tabs/tab-group.ts

@@ -36,9 +36,10 @@ import {
   mixinDisableRipple,
   ThemePalette,
 } from '@angular/material/core';
-import {merge, Subscription} from 'rxjs';
-import {MatTab} from './tab';
 import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
+import {merge, Subscription} from 'rxjs';
+import {startWith} from 'rxjs/operators';
+import {MatTab, MAT_TAB_GROUP} from './tab';
 
 
 /** Used to generate unique ID's for each tab component */
@@ -88,10 +89,18 @@ interface MatTabGroupBaseHeader {
 // tslint:disable-next-line:class-name
 export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements AfterContentInit,
     AfterContentChecked, OnDestroy, CanColor, CanDisableRipple {
-  abstract _tabs: QueryList<MatTab>;
+
+  /**
+   * All tabs inside the tab group. This includes tabs that belong to groups that are nested
+   * inside the current one. We filter out only the tabs that belong to this group in `_tabs`.
+   */
+  abstract _allTabs: QueryList<MatTab>;
   abstract _tabBodyWrapper: ElementRef;
   abstract _tabHeader: MatTabGroupBaseHeader;
 
+  /** All of the tabs that belong to the group. */
+  _tabs: QueryList<MatTab> = new QueryList<MatTab>();
+
   /** The tab index that should be selected after the content has been checked. */
   private _indexToSelect: number | null = 0;
 
@@ -220,6 +229,7 @@ export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements
   }
 
   ngAfterContentInit() {
+    this._subscribeToAllTabChanges();
     this._subscribeToTabLabels();
 
     // Subscribe to changes in the amount of tabs, in order to be
@@ -243,11 +253,27 @@ export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements
         }
       }
 
-      this._subscribeToTabLabels();
       this._changeDetectorRef.markForCheck();
     });
   }
 
+  /** Listens to changes in all of the tabs. */
+  private _subscribeToAllTabChanges() {
+    // Since we use a query with `descendants: true` to pick up the tabs, we may end up catching
+    // some that are inside of nested tab groups. We filter them out manually by checking that
+    // the closest group to the tab is the current one.
+    this._allTabs.changes
+      .pipe(startWith(this._allTabs))
+      .subscribe((tabs: QueryList<MatTab>) => {
+        this._tabs.reset(tabs.filter(tab => {
+          // @breaking-change 10.0.0 Remove null check for `_closestTabGroup`
+          // once it becomes a required parameter in MatTab.
+          return !tab._closestTabGroup || tab._closestTabGroup === this;
+        }));
+        this._tabs.notifyOnChanges();
+      });
+  }
+
   ngOnDestroy() {
     this._tabsSubscription.unsubscribe();
     this._tabLabelSubscription.unsubscribe();
@@ -363,14 +389,18 @@ export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements
   // tslint:disable-next-line:validate-decorators
   changeDetection: ChangeDetectionStrategy.Default,
   inputs: ['color', 'disableRipple'],
+  providers: [{
+    provide: MAT_TAB_GROUP,
+    useExisting: MatTabGroup
+  }],
   host: {
     'class': 'mat-tab-group',
     '[class.mat-tab-group-dynamic-height]': 'dynamicHeight',
     '[class.mat-tab-group-inverted-header]': 'headerPosition === "below"',
   },
 })
 export class MatTabGroup extends _MatTabGroupBase {
-  @ContentChildren(MatTab) _tabs: QueryList<MatTab>;
+  @ContentChildren(MatTab, {descendants: true}) _allTabs: QueryList<MatTab>;
   @ViewChild('tabBodyWrapper', {static: false}) _tabBodyWrapper: ElementRef;
   @ViewChild('tabHeader', {static: false}) _tabHeader: MatTabGroupBaseHeader;
 

src/material/tabs/tab.ts

@@ -20,6 +20,9 @@ import {
   ViewChild,
   ViewContainerRef,
   ViewEncapsulation,
+  InjectionToken,
+  Inject,
+  Optional,
 } from '@angular/core';
 import {CanDisable, CanDisableCtor, mixinDisabled} from '@angular/material/core';
 import {Subject} from 'rxjs';
@@ -33,6 +36,12 @@ class MatTabBase {}
 const _MatTabMixinBase: CanDisableCtor & typeof MatTabBase =
     mixinDisabled(MatTabBase);
 
+/**
+ * Used to provide a tab group to a tab without causing a circular dependency.
+ * @docs-private
+ */
+export const MAT_TAB_GROUP = new InjectionToken<any>('MAT_TAB_GROUP');
+
 @Component({
   moduleId: module.id,
   selector: 'mat-tab',
@@ -107,7 +116,13 @@ export class MatTab extends _MatTabMixinBase implements OnInit, CanDisable, OnCh
    */
   isActive = false;
 
-  constructor(private _viewContainerRef: ViewContainerRef) {
+  constructor(
+    private _viewContainerRef: ViewContainerRef,
+    /**
+     * @deprecated `_closestTabGroup` parameter to become required.
+     * @breaking-change 10.0.0
+     */
+    @Optional() @Inject(MAT_TAB_GROUP) public _closestTabGroup?: any) {
     super();
   }
 

tools/public_api_guard/material/tabs.d.ts

@@ -28,10 +28,11 @@ export declare abstract class _MatTabBodyBase implements OnInit, OnDestroy {
 }
 
 export declare abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements AfterContentInit, AfterContentChecked, OnDestroy, CanColor, CanDisableRipple {
+    abstract _allTabs: QueryList<MatTab>;
     _animationMode?: string | undefined;
     abstract _tabBodyWrapper: ElementRef;
     abstract _tabHeader: MatTabGroupBaseHeader;
-    abstract _tabs: QueryList<MatTab>;
+    _tabs: QueryList<MatTab>;
     readonly animationDone: EventEmitter<void>;
     animationDuration: string;
     backgroundColor: ThemePalette;
@@ -86,6 +87,8 @@ export declare abstract class _MatTabNavBase extends MatPaginatedTabHeader imple
     updateActiveLink(_element?: ElementRef): void;
 }
 
+export declare const MAT_TAB_GROUP: InjectionToken<any>;
+
 export declare const MAT_TABS_CONFIG: InjectionToken<MatTabsConfig>;
 
 export declare class MatInkBar {
@@ -97,6 +100,7 @@ export declare class MatInkBar {
 }
 
 export declare class MatTab extends _MatTabMixinBase implements OnInit, CanDisable, OnChanges, OnDestroy {
+    _closestTabGroup?: any;
     _explicitContent: TemplateRef<any>;
     _implicitContent: TemplateRef<any>;
     readonly _stateChanges: Subject<void>;
@@ -108,7 +112,8 @@ export declare class MatTab extends _MatTabMixinBase implements OnInit, CanDisab
     position: number | null;
     templateLabel: MatTabLabel;
     textLabel: string;
-    constructor(_viewContainerRef: ViewContainerRef);
+    constructor(_viewContainerRef: ViewContainerRef,
+    _closestTabGroup?: any);
     ngOnChanges(changes: SimpleChanges): void;
     ngOnDestroy(): void;
     ngOnInit(): void;
@@ -140,9 +145,9 @@ export declare class MatTabContent {
 }
 
 export declare class MatTabGroup extends _MatTabGroupBase {
+    _allTabs: QueryList<MatTab>;
     _tabBodyWrapper: ElementRef;
     _tabHeader: MatTabGroupBaseHeader;
-    _tabs: QueryList<MatTab>;
     constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, defaultConfig?: MatTabsConfig, animationMode?: string);
 }