From ef1c63f26e47a03cb98ff29987ac92d9ad0fc0c3 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 12 Oct 2019 10:19:49 +0200 Subject: [PATCH] fix(tabs): not picking up indirect descendant tabs in ivy 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. --- .../mdc-tabs/public-api.ts | 1 + .../mdc-tabs/tab-group.spec.ts | 49 ++++++++++++++++++- .../mdc-tabs/tab-group.ts | 13 ++++- src/material/tabs/public-api.ts | 2 +- src/material/tabs/tab-group.spec.ts | 48 +++++++++++++++++- src/material/tabs/tab-group.ts | 40 +++++++++++++-- src/material/tabs/tab.ts | 17 ++++++- tools/public_api_guard/material/tabs.d.ts | 11 +++-- 8 files changed, 167 insertions(+), 14 deletions(-) diff --git a/src/material-experimental/mdc-tabs/public-api.ts b/src/material-experimental/mdc-tabs/public-api.ts index e3a8f5da0ced..2f6488831390 100644 --- a/src/material-experimental/mdc-tabs/public-api.ts +++ b/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'; diff --git a/src/material-experimental/mdc-tabs/tab-group.spec.ts b/src/material-experimental/mdc-tabs/tab-group.spec.ts index f3ec0a621f16..cd703f0e08b1 100644 --- a/src/material-experimental/mdc-tabs/tab-group.spec.ts +++ b/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 { `, }) -class NestedTabs {} +class NestedTabs { + @ViewChildren(MatTabGroup) groups: QueryList; +} @Component({ selector: 'template-tabs', @@ -881,3 +913,18 @@ class TabGroupWithIsActiveBinding { `, }) class TabsWithCustomAnimationDuration {} + + +@Component({ + template: ` + + + Tab one content + Tab two content + + + `, +}) +class TabGroupWithIndirectDescendantTabs { + @ViewChild(MatTabGroup, {static: false}) tabGroup: MatTabGroup; +} diff --git a/src/material-experimental/mdc-tabs/tab-group.ts b/src/material-experimental/mdc-tabs/tab-group.ts index 0d958dee9e6d..ccb67e789294 100644 --- a/src/material-experimental/mdc-tabs/tab-group.ts +++ b/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,6 +42,10 @@ 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', @@ -44,7 +53,7 @@ import {MatTabHeader} from './tab-header'; }, }) export class MatTabGroup extends _MatTabGroupBase { - @ContentChildren(MatTab) _tabs: QueryList; + @ContentChildren(MatTab, {descendants: true}) _allTabs: QueryList; @ViewChild('tabBodyWrapper', {static: false}) _tabBodyWrapper: ElementRef; @ViewChild('tabHeader', {static: false}) _tabHeader: MatTabHeader; diff --git a/src/material/tabs/public-api.ts b/src/material/tabs/public-api.ts index 9679eaf65501..fc8dac396f64 100644 --- a/src/material/tabs/public-api.ts +++ b/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'; diff --git a/src/material/tabs/tab-group.spec.ts b/src/material/tabs/tab-group.spec.ts index 4ba5d45085ad..54d3b24b6634 100644 --- a/src/material/tabs/tab-group.spec.ts +++ b/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 { `, }) -class NestedTabs {} +class NestedTabs { + @ViewChildren(MatTabGroup) groups: QueryList; +} @Component({ selector: 'template-tabs', @@ -880,3 +911,18 @@ class TabGroupWithIsActiveBinding { `, }) class TabsWithCustomAnimationDuration {} + + +@Component({ + template: ` + + + Tab one content + Tab two content + + + `, +}) +class TabGroupWithIndirectDescendantTabs { + @ViewChild(MatTabGroup, {static: false}) tabGroup: MatTabGroup; +} diff --git a/src/material/tabs/tab-group.ts b/src/material/tabs/tab-group.ts index e7c103f1f667..cb178d8930d2 100644 --- a/src/material/tabs/tab-group.ts +++ b/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; + + /** + * 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; abstract _tabBodyWrapper: ElementRef; abstract _tabHeader: MatTabGroupBaseHeader; + /** All of the tabs that belong to the group. */ + _tabs: QueryList = new QueryList(); + /** 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) => { + 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,6 +389,10 @@ 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', @@ -370,7 +400,7 @@ export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements }, }) export class MatTabGroup extends _MatTabGroupBase { - @ContentChildren(MatTab) _tabs: QueryList; + @ContentChildren(MatTab, {descendants: true}) _allTabs: QueryList; @ViewChild('tabBodyWrapper', {static: false}) _tabBodyWrapper: ElementRef; @ViewChild('tabHeader', {static: false}) _tabHeader: MatTabGroupBaseHeader; diff --git a/src/material/tabs/tab.ts b/src/material/tabs/tab.ts index 066fe921c995..feca54a4d4ea 100644 --- a/src/material/tabs/tab.ts +++ b/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('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(); } diff --git a/tools/public_api_guard/material/tabs.d.ts b/tools/public_api_guard/material/tabs.d.ts index 2652c67ff79e..f10db7c8fde7 100644 --- a/tools/public_api_guard/material/tabs.d.ts +++ b/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; _animationMode?: string | undefined; abstract _tabBodyWrapper: ElementRef; abstract _tabHeader: MatTabGroupBaseHeader; - abstract _tabs: QueryList; + _tabs: QueryList; readonly animationDone: EventEmitter; 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; + export declare const MAT_TABS_CONFIG: InjectionToken; 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; _implicitContent: TemplateRef; readonly _stateChanges: Subject; @@ -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; _tabBodyWrapper: ElementRef; _tabHeader: MatTabGroupBaseHeader; - _tabs: QueryList; constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, defaultConfig?: MatTabsConfig, animationMode?: string); }