Skip to content

Commit 1e2a8fc

Browse files
crisbetocrisbeto
committed
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.
1 parent 45a4300 commit 1e2a8fc

File tree

8 files changed

+161
-14
lines changed

8 files changed

+161
-14
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,5 @@ export {
2727
MatTabHeaderPosition,
2828
MatTabsConfig,
2929
MAT_TABS_CONFIG,
30+
MAT_TAB_GROUP,
3031
} from '@angular/material/tabs';

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

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ describe('MatTabGroup', () => {
2323
TemplateTabs,
2424
TabGroupWithAriaInputs,
2525
TabGroupWithIsActiveBinding,
26+
NestedTabs,
27+
TabGroupWithIndirectDescendantTabs,
2628
],
2729
});
2830

@@ -589,6 +591,34 @@ describe('MatTabGroup', () => {
589591
}));
590592
});
591593

594+
describe('nested tabs', () => {
595+
it('should not pick up the tabs from descendant tab groups', fakeAsync(() => {
596+
const fixture = TestBed.createComponent(NestedTabs);
597+
fixture.detectChanges();
598+
tick();
599+
fixture.detectChanges();
600+
601+
const groups = fixture.componentInstance.groups.toArray();
602+
603+
expect(groups.length).toBe(2);
604+
expect(groups[0]._tabs.map((tab: MatTab) => tab.textLabel))
605+
.toEqual(['One', 'Two']);
606+
expect(groups[1]._tabs.map((tab: MatTab) => tab.textLabel))
607+
.toEqual(['Inner tab one', 'Inner tab two']);
608+
}));
609+
610+
it('should pick up indirect descendant tabs', fakeAsync(() => {
611+
const fixture = TestBed.createComponent(TabGroupWithIndirectDescendantTabs);
612+
fixture.detectChanges();
613+
tick();
614+
fixture.detectChanges();
615+
616+
const tabs = fixture.componentInstance.tabGroup._tabs;
617+
expect(tabs.map((tab: MatTab) => tab.textLabel)).toEqual(['One', 'Two']);
618+
}));
619+
});
620+
621+
592622
/**
593623
* Checks that the `selectedIndex` has been updated; checks that the label and body have their
594624
* respective `active` classes
@@ -825,7 +855,9 @@ class TabGroupWithSimpleApi {
825855
</mat-tab-group>
826856
`,
827857
})
828-
class NestedTabs {}
858+
class NestedTabs {
859+
@ViewChildren(MatTabGroup) groups: QueryList<MatTabGroup>;
860+
}
829861

830862
@Component({
831863
selector: 'template-tabs',
@@ -881,3 +913,18 @@ class TabGroupWithIsActiveBinding {
881913
`,
882914
})
883915
class TabsWithCustomAnimationDuration {}
916+
917+
918+
@Component({
919+
template: `
920+
<mat-tab-group>
921+
<ng-container [ngSwitch]="true">
922+
<mat-tab label="One">Tab one content</mat-tab>
923+
<mat-tab label="Two">Tab two content</mat-tab>
924+
</ng-container>
925+
</mat-tab-group>
926+
`,
927+
})
928+
class TabGroupWithIndirectDescendantTabs {
929+
@ViewChild(MatTabGroup, {static: false}) tabGroup: MatTabGroup;
930+
}

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@ import {
1818
Inject,
1919
Optional,
2020
} from '@angular/core';
21-
import {_MatTabGroupBase, MAT_TABS_CONFIG, MatTabsConfig} from '@angular/material/tabs';
21+
import {
22+
_MatTabGroupBase,
23+
MAT_TABS_CONFIG,
24+
MatTabsConfig,
25+
MAT_TAB_GROUP,
26+
} from '@angular/material/tabs';
2227
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
2328
import {MatTab} from './tab';
2429
import {MatTabHeader} from './tab-header';
@@ -37,14 +42,18 @@ import {MatTabHeader} from './tab-header';
3742
encapsulation: ViewEncapsulation.None,
3843
changeDetection: ChangeDetectionStrategy.OnPush,
3944
inputs: ['color', 'disableRipple'],
45+
providers: [{
46+
provide: MAT_TAB_GROUP,
47+
useExisting: MatTabGroup
48+
}],
4049
host: {
4150
'class': 'mat-mdc-tab-group',
4251
'[class.mat-mdc-tab-group-dynamic-height]': 'dynamicHeight',
4352
'[class.mat-mdc-tab-group-inverted-header]': 'headerPosition === "below"',
4453
},
4554
})
4655
export class MatTabGroup extends _MatTabGroupBase {
47-
@ContentChildren(MatTab) _tabs: QueryList<MatTab>;
56+
@ContentChildren(MatTab, {descendants: true}) _allTabs: QueryList<MatTab>;
4857
@ViewChild('tabBodyWrapper', {static: false}) _tabBodyWrapper: ElementRef;
4958
@ViewChild('tabHeader', {static: false}) _tabHeader: MatTabHeader;
5059

‎src/material/tabs/public-api.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export {
1818
} from './tab-body';
1919
export {MatTabHeader, _MatTabHeaderBase} from './tab-header';
2020
export {MatTabLabelWrapper} from './tab-label-wrapper';
21-
export {MatTab} from './tab';
21+
export {MatTab, MAT_TAB_GROUP} from './tab';
2222
export {MatTabLabel} from './tab-label';
2323
export {MatTabNav, MatTabLink, _MatTabNavBase, _MatTabLinkBase} from './tab-nav-bar/index';
2424
export {MatTabContent} from './tab-content';

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

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ describe('MatTabGroup', () => {
2323
TemplateTabs,
2424
TabGroupWithAriaInputs,
2525
TabGroupWithIsActiveBinding,
26+
NestedTabs,
27+
TabGroupWithIndirectDescendantTabs,
2628
],
2729
});
2830

@@ -588,6 +590,33 @@ describe('MatTabGroup', () => {
588590
}));
589591
});
590592

593+
describe('nested tabs', () => {
594+
it('should not pick up the tabs from descendant tab groups', fakeAsync(() => {
595+
const fixture = TestBed.createComponent(NestedTabs);
596+
fixture.detectChanges();
597+
tick();
598+
fixture.detectChanges();
599+
600+
const groups = fixture.componentInstance.groups.toArray();
601+
602+
expect(groups.length).toBe(2);
603+
expect(groups[0]._tabs.map((tab: MatTab) => tab.textLabel))
604+
.toEqual(['One', 'Two']);
605+
expect(groups[1]._tabs.map((tab: MatTab) => tab.textLabel))
606+
.toEqual(['Inner tab one', 'Inner tab two']);
607+
}));
608+
609+
it('should pick up indirect descendant tabs', fakeAsync(() => {
610+
const fixture = TestBed.createComponent(TabGroupWithIndirectDescendantTabs);
611+
fixture.detectChanges();
612+
tick();
613+
fixture.detectChanges();
614+
615+
const tabs = fixture.componentInstance.tabGroup._tabs;
616+
expect(tabs.map((tab: MatTab) => tab.textLabel)).toEqual(['One', 'Two']);
617+
}));
618+
});
619+
591620
/**
592621
* Checks that the `selectedIndex` has been updated; checks that the label and body have their
593622
* respective `active` classes
@@ -824,7 +853,9 @@ class TabGroupWithSimpleApi {
824853
</mat-tab-group>
825854
`,
826855
})
827-
class NestedTabs {}
856+
class NestedTabs {
857+
@ViewChildren(MatTabGroup) groups: QueryList<MatTabGroup>;
858+
}
828859

829860
@Component({
830861
selector: 'template-tabs',
@@ -880,3 +911,18 @@ class TabGroupWithIsActiveBinding {
880911
`,
881912
})
882913
class TabsWithCustomAnimationDuration {}
914+
915+
916+
@Component({
917+
template: `
918+
<mat-tab-group>
919+
<ng-container [ngSwitch]="true">
920+
<mat-tab label="One">Tab one content</mat-tab>
921+
<mat-tab label="Two">Tab two content</mat-tab>
922+
</ng-container>
923+
</mat-tab-group>
924+
`,
925+
})
926+
class TabGroupWithIndirectDescendantTabs {
927+
@ViewChild(MatTabGroup, {static: false}) tabGroup: MatTabGroup;
928+
}

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

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ import {
3636
mixinDisableRipple,
3737
ThemePalette,
3838
} from '@angular/material/core';
39-
import {merge, Subscription} from 'rxjs';
40-
import {MatTab} from './tab';
4139
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
40+
import {merge, Subscription} from 'rxjs';
41+
import {startWith} from 'rxjs/operators';
42+
import {MatTab, MAT_TAB_GROUP} from './tab';
4243

4344

4445
/** Used to generate unique ID's for each tab component */
@@ -88,10 +89,13 @@ interface MatTabGroupBaseHeader {
8889
// tslint:disable-next-line:class-name
8990
export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements AfterContentInit,
9091
AfterContentChecked, OnDestroy, CanColor, CanDisableRipple {
91-
abstract _tabs: QueryList<MatTab>;
92+
abstract _allTabs: QueryList<MatTab>;
9293
abstract _tabBodyWrapper: ElementRef;
9394
abstract _tabHeader: MatTabGroupBaseHeader;
9495

96+
/** All of the tabs that belong to the group. */
97+
_tabs: QueryList<MatTab> = new QueryList<MatTab>();
98+
9599
/** The tab index that should be selected after the content has been checked. */
96100
private _indexToSelect: number | null = 0;
97101

@@ -220,6 +224,7 @@ export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements
220224
}
221225

222226
ngAfterContentInit() {
227+
this._subscribeToAllTabChanges();
223228
this._subscribeToTabLabels();
224229

225230
// Subscribe to changes in the amount of tabs, in order to be
@@ -243,11 +248,26 @@ export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements
243248
}
244249
}
245250

246-
this._subscribeToTabLabels();
247251
this._changeDetectorRef.markForCheck();
248252
});
249253
}
250254

255+
/** Listens to changes in all of the tabs. */
256+
private _subscribeToAllTabChanges() {
257+
// Since we use a query with `descendants: true` to pick up the tabs, we may end up
258+
// catching some that are inside of nested tab groups so we need to filter them out manually.
259+
this._allTabs.changes
260+
.pipe(startWith(this._allTabs))
261+
.subscribe((tabs: QueryList<MatTab>) => {
262+
this._tabs.reset(tabs.filter(tab => {
263+
// @breaking-change 10.0.0 Remove null check for `_closestTabGroup`
264+
// once it becomes a required parameter in MatTab.
265+
return !tab._closestTabGroup || tab._closestTabGroup === this;
266+
}));
267+
this._tabs.notifyOnChanges();
268+
});
269+
}
270+
251271
ngOnDestroy() {
252272
this._tabsSubscription.unsubscribe();
253273
this._tabLabelSubscription.unsubscribe();
@@ -362,14 +382,18 @@ export abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements
362382
encapsulation: ViewEncapsulation.None,
363383
changeDetection: ChangeDetectionStrategy.OnPush,
364384
inputs: ['color', 'disableRipple'],
385+
providers: [{
386+
provide: MAT_TAB_GROUP,
387+
useExisting: MatTabGroup
388+
}],
365389
host: {
366390
'class': 'mat-tab-group',
367391
'[class.mat-tab-group-dynamic-height]': 'dynamicHeight',
368392
'[class.mat-tab-group-inverted-header]': 'headerPosition === "below"',
369393
},
370394
})
371395
export class MatTabGroup extends _MatTabGroupBase {
372-
@ContentChildren(MatTab) _tabs: QueryList<MatTab>;
396+
@ContentChildren(MatTab, {descendants: true}) _allTabs: QueryList<MatTab>;
373397
@ViewChild('tabBodyWrapper', {static: false}) _tabBodyWrapper: ElementRef;
374398
@ViewChild('tabHeader', {static: false}) _tabHeader: MatTabGroupBaseHeader;
375399

‎src/material/tabs/tab.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ import {
2020
ViewChild,
2121
ViewContainerRef,
2222
ViewEncapsulation,
23+
InjectionToken,
24+
Inject,
25+
Optional,
2326
} from '@angular/core';
2427
import {CanDisable, CanDisableCtor, mixinDisabled} from '@angular/material/core';
2528
import {Subject} from 'rxjs';
@@ -33,6 +36,12 @@ class MatTabBase {}
3336
const _MatTabMixinBase: CanDisableCtor & typeof MatTabBase =
3437
mixinDisabled(MatTabBase);
3538

39+
/**
40+
* Used to provide a tab group to a tab without causing a circular dependency.
41+
* @docs-private
42+
*/
43+
export const MAT_TAB_GROUP = new InjectionToken<any>('MAT_TAB_GROUP');
44+
3645
@Component({
3746
moduleId: module.id,
3847
selector: 'mat-tab',
@@ -95,7 +104,13 @@ export class MatTab extends _MatTabMixinBase implements OnInit, CanDisable, OnCh
95104
*/
96105
isActive = false;
97106

98-
constructor(private _viewContainerRef: ViewContainerRef) {
107+
constructor(
108+
private _viewContainerRef: ViewContainerRef,
109+
/**
110+
* @deprecated `_closestTabGroup` parameter to become required.
111+
* @breaking-change 10.0.0
112+
*/
113+
@Optional() @Inject(MAT_TAB_GROUP) public _closestTabGroup?: any) {
99114
super();
100115
}
101116

‎tools/public_api_guard/material/tabs.d.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@ export declare abstract class _MatTabBodyBase implements OnInit, OnDestroy {
2828
}
2929

3030
export declare abstract class _MatTabGroupBase extends _MatTabGroupMixinBase implements AfterContentInit, AfterContentChecked, OnDestroy, CanColor, CanDisableRipple {
31+
abstract _allTabs: QueryList<MatTab>;
3132
_animationMode?: string | undefined;
3233
abstract _tabBodyWrapper: ElementRef;
3334
abstract _tabHeader: MatTabGroupBaseHeader;
34-
abstract _tabs: QueryList<MatTab>;
35+
_tabs: QueryList<MatTab>;
3536
readonly animationDone: EventEmitter<void>;
3637
animationDuration: string;
3738
backgroundColor: ThemePalette;
@@ -86,6 +87,8 @@ export declare abstract class _MatTabNavBase extends MatPaginatedTabHeader imple
8687
updateActiveLink(_element?: ElementRef): void;
8788
}
8889

90+
export declare const MAT_TAB_GROUP: InjectionToken<any>;
91+
8992
export declare const MAT_TABS_CONFIG: InjectionToken<MatTabsConfig>;
9093

9194
export declare class MatInkBar {
@@ -97,6 +100,7 @@ export declare class MatInkBar {
97100
}
98101

99102
export declare class MatTab extends _MatTabMixinBase implements OnInit, CanDisable, OnChanges, OnDestroy {
103+
_closestTabGroup?: any;
100104
_explicitContent: TemplateRef<any>;
101105
_implicitContent: TemplateRef<any>;
102106
readonly _stateChanges: Subject<void>;
@@ -108,7 +112,8 @@ export declare class MatTab extends _MatTabMixinBase implements OnInit, CanDisab
108112
position: number | null;
109113
templateLabel: MatTabLabel;
110114
textLabel: string;
111-
constructor(_viewContainerRef: ViewContainerRef);
115+
constructor(_viewContainerRef: ViewContainerRef,
116+
_closestTabGroup?: any);
112117
ngOnChanges(changes: SimpleChanges): void;
113118
ngOnDestroy(): void;
114119
ngOnInit(): void;
@@ -140,9 +145,9 @@ export declare class MatTabContent {
140145
}
141146

142147
export declare class MatTabGroup extends _MatTabGroupBase {
148+
_allTabs: QueryList<MatTab>;
143149
_tabBodyWrapper: ElementRef;
144150
_tabHeader: MatTabGroupBaseHeader;
145-
_tabs: QueryList<MatTab>;
146151
constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, defaultConfig?: MatTabsConfig, animationMode?: string);
147152
}
148153

0 commit comments

Comments
 (0)