Skip to content

Commit

Permalink
fix(material/tabs): picking up mat-tab-label from child tabs (#23560)
Browse files Browse the repository at this point in the history
We use `ContenChild` to get a hold of the projected `mat-tab-label` inside a tab, but the problem is that this will also pick up label inside of nested tabs.

These changes add some safeguards so the labels don't get mixed up.

Fixes #23558.
  • Loading branch information
crisbeto authored Sep 17, 2021
1 parent 7539cf2 commit 505c0d1
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/material-experimental/mdc-tabs/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ export {
MatTabsConfig,
MAT_TABS_CONFIG,
MAT_TAB_GROUP,
MAT_TAB,
ScrollDirection,
} from '@angular/material/tabs';
36 changes: 36 additions & 0 deletions src/material-experimental/mdc-tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('MDC-based MatTabGroup', () => {
NestedTabs,
TabGroupWithIndirectDescendantTabs,
TabGroupWithSpaceAbove,
NestedTabGroupWithLabel,
],
});

Expand Down Expand Up @@ -673,6 +674,19 @@ describe('MDC-based MatTabGroup', () => {

expect(fixture.nativeElement.textContent).toContain('pizza is active');
}));

it('should not pick up mat-tab-label from a child tab', fakeAsync(() => {
const fixture = TestBed.createComponent(NestedTabGroupWithLabel);
fixture.detectChanges();
tick();
fixture.detectChanges();

const labels = fixture.nativeElement.querySelectorAll('.mdc-tab__text-label');
const contents = Array.from<HTMLElement>(labels).map(label => label.textContent?.trim());

expect(contents).toEqual(
['Parent 1', 'Parent 2', 'Parent 3', 'Child 1', 'Child 2', 'Child 3']);
}));
});

describe('nested tabs', () => {
Expand Down Expand Up @@ -1166,3 +1180,25 @@ class TabGroupWithInkBarFitToContent {
class TabGroupWithSpaceAbove {
@ViewChild(MatTabGroup) tabGroup: MatTabGroup;
}


@Component({
template: `
<mat-tab-group>
<mat-tab label="Parent 1">
<mat-tab-group>
<mat-tab label="Child 1">Content 1</mat-tab>
<mat-tab>
<ng-template mat-tab-label>Child 2</ng-template>
Content 2
</mat-tab>
<mat-tab label="Child 3">Child 3</mat-tab>
</mat-tab-group>
</mat-tab>
<mat-tab label="Parent 2">Parent 2</mat-tab>
<mat-tab label="Parent 3">Parent 3</mat-tab>
</mat-tab-group>
`
})
class NestedTabGroupWithLabel {
}
3 changes: 2 additions & 1 deletion src/material-experimental/mdc-tabs/tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
TemplateRef,
ContentChild,
} from '@angular/core';
import {MatTab as BaseMatTab} from '@angular/material/tabs';
import {MatTab as BaseMatTab, MAT_TAB} from '@angular/material/tabs';
import {MatTabContent} from './tab-content';
import {MatTabLabel} from './tab-label';

Expand All @@ -28,6 +28,7 @@ import {MatTabLabel} from './tab-label';
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.None,
exportAs: 'matTab',
providers: [{provide: MAT_TAB, useExisting: MatTab}]
})
export class MatTab extends BaseMatTab {
/**
Expand Down
2 changes: 1 addition & 1 deletion src/material/tabs/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export {
export {MatTabHeader, _MatTabHeaderBase} from './tab-header';
export {MatTabLabelWrapper} from './tab-label-wrapper';
export {MatTab, MAT_TAB_GROUP} from './tab';
export {MatTabLabel} from './tab-label';
export {MatTabLabel, MAT_TAB} from './tab-label';
export {MatTabNav, MatTabLink, _MatTabNavBase, _MatTabLinkBase} from './tab-nav-bar/index';
export {MatTabContent} from './tab-content';
export {ScrollDirection} from './paginated-tab-header';
Expand Down
36 changes: 36 additions & 0 deletions src/material/tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('MatTabGroup', () => {
NestedTabs,
TabGroupWithIndirectDescendantTabs,
TabGroupWithSpaceAbove,
NestedTabGroupWithLabel,
],
});

Expand Down Expand Up @@ -673,6 +674,19 @@ describe('MatTabGroup', () => {

expect(fixture.nativeElement.textContent).toContain('pizza is active');
}));

it('should not pick up mat-tab-label from a child tab', fakeAsync(() => {
const fixture = TestBed.createComponent(NestedTabGroupWithLabel);
fixture.detectChanges();
tick();
fixture.detectChanges();

const labels = fixture.nativeElement.querySelectorAll('.mat-tab-label-content');
const contents = Array.from<HTMLElement>(labels).map(label => label.textContent?.trim());

expect(contents).toEqual(
['Parent 1', 'Parent 2', 'Parent 3', 'Child 1', 'Child 2', 'Child 3']);
}));
});

describe('nested tabs', () => {
Expand Down Expand Up @@ -1099,3 +1113,25 @@ class TabGroupWithIndirectDescendantTabs {
class TabGroupWithSpaceAbove {
@ViewChild(MatTabGroup) tabGroup: MatTabGroup;
}


@Component({
template: `
<mat-tab-group>
<mat-tab label="Parent 1">
<mat-tab-group>
<mat-tab label="Child 1">Content 1</mat-tab>
<mat-tab>
<ng-template mat-tab-label>Child 2</ng-template>
Content 2
</mat-tab>
<mat-tab label="Child 3">Child 3</mat-tab>
</mat-tab-group>
</mat-tab>
<mat-tab label="Parent 2">Parent 2</mat-tab>
<mat-tab label="Parent 3">Parent 3</mat-tab>
</mat-tab-group>
`
})
class NestedTabGroupWithLabel {
}
24 changes: 22 additions & 2 deletions src/material/tabs/tab-label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, InjectionToken} from '@angular/core';
import {
Directive,
Inject,
InjectionToken,
Optional,
TemplateRef,
ViewContainerRef,
} from '@angular/core';
import {CdkPortal} from '@angular/cdk/portal';

/**
Expand All @@ -16,9 +23,22 @@ import {CdkPortal} from '@angular/cdk/portal';
*/
export const MAT_TAB_LABEL = new InjectionToken<MatTabLabel>('MatTabLabel');

/**
* Used to provide a tab label to a tab without causing a circular dependency.
* @docs-private
*/
export const MAT_TAB = new InjectionToken<any>('MAT_TAB');

/** Used to flag tab labels for use with the portal directive */
@Directive({
selector: '[mat-tab-label], [matTabLabel]',
providers: [{provide: MAT_TAB_LABEL, useExisting: MatTabLabel}],
})
export class MatTabLabel extends CdkPortal {}
export class MatTabLabel extends CdkPortal {
constructor(
templateRef: TemplateRef<any>,
viewContainerRef: ViewContainerRef,
@Inject(MAT_TAB) @Optional() public _closestTab: any) {
super(templateRef, viewContainerRef);
}
}
15 changes: 8 additions & 7 deletions src/material/tabs/tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
import {CanDisable, mixinDisabled} from '@angular/material/core';
import {Subject} from 'rxjs';
import {MAT_TAB_CONTENT} from './tab-content';
import {MAT_TAB_LABEL, MatTabLabel} from './tab-label';
import {MAT_TAB_LABEL, MatTabLabel, MAT_TAB} from './tab-label';


// Boilerplate for applying mixins to MatTab.
Expand All @@ -49,6 +49,7 @@ export const MAT_TAB_GROUP = new InjectionToken<any>('MAT_TAB_GROUP');
changeDetection: ChangeDetectionStrategy.Default,
encapsulation: ViewEncapsulation.None,
exportAs: 'matTab',
providers: [{provide: MAT_TAB, useExisting: MatTab}]
})
export class MatTab extends _MatTabBase implements OnInit, CanDisable, OnChanges, OnDestroy {
/** Content for the tab label given by `<ng-template mat-tab-label>`. */
Expand Down Expand Up @@ -133,12 +134,12 @@ export class MatTab extends _MatTabBase implements OnInit, CanDisable, OnChanges
* TS 4.0 doesn't allow properties to override accessors or vice-versa.
* @docs-private
*/
protected _setTemplateLabelInput(value: MatTabLabel) {
// Only update the templateLabel via query if there is actually
// a MatTabLabel found. This works around an issue where a user may have
// manually set `templateLabel` during creation mode, which would then get clobbered
// by `undefined` when this query resolves.
if (value) {
protected _setTemplateLabelInput(value: MatTabLabel|undefined) {
// Only update the label if the query managed to find one. This works around an issue where a
// user may have manually set `templateLabel` during creation mode, which would then get
// clobbered by `undefined` when the query resolves. Also note that we check that the closest
// tab matches the current one so that we don't pick up labels from nested tabs.
if (value && value._closestTab === this) {
this._templateLabel = value;
}
}
Expand Down
10 changes: 8 additions & 2 deletions tools/public_api_guard/material/tabs.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ export const _MAT_INK_BAR_POSITIONER: InjectionToken<_MatInkBarPositioner>;
// @public
function _MAT_INK_BAR_POSITIONER_FACTORY(): _MatInkBarPositioner;

// @public
export const MAT_TAB: InjectionToken<any>;

// @public
const MAT_TAB_CONTENT: InjectionToken<MatTabContent>;

Expand Down Expand Up @@ -114,7 +117,7 @@ export class MatTab extends _MatTabBase implements OnInit, CanDisable, OnChanges
ngOnInit(): void;
origin: number | null;
position: number | null;
protected _setTemplateLabelInput(value: MatTabLabel): void;
protected _setTemplateLabelInput(value: MatTabLabel | undefined): void;
readonly _stateChanges: Subject<void>;
get templateLabel(): MatTabLabel;
set templateLabel(value: MatTabLabel);
Expand Down Expand Up @@ -316,10 +319,13 @@ export type MatTabHeaderPosition = 'above' | 'below';

// @public
export class MatTabLabel extends CdkPortal {
constructor(templateRef: TemplateRef<any>, viewContainerRef: ViewContainerRef, _closestTab: any);
// (undocumented)
_closestTab: any;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<MatTabLabel, "[mat-tab-label], [matTabLabel]", never, {}, {}, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<MatTabLabel, never>;
static ɵfac: i0.ɵɵFactoryDeclaration<MatTabLabel, [null, null, { optional: true; }]>;
}

// @public
Expand Down

0 comments on commit 505c0d1

Please sign in to comment.