Skip to content

Commit 499458c

Browse files
crisbetommalerba
authored andcommitted
fix(tabs): changed after checked error when using isActive in view (#12206)
* Fixes a "changed after checked" error being thrown if the consumer is using the `isActive` property of a tab somewhere in the view. * Reworks the `MatTab` to only have one `stateChanges` subject rather than one per property. Fixes #12197.
1 parent ba84d5b commit 499458c

File tree

3 files changed

+59
-29
lines changed

3 files changed

+59
-29
lines changed

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

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@ import {Component, OnInit, QueryList, ViewChild, ViewChildren} from '@angular/co
44
import {async, ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
55
import {By} from '@angular/platform-browser';
66
import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations';
7+
import {CommonModule} from '@angular/common';
78
import {Observable} from 'rxjs';
89
import {MatTab, MatTabGroup, MatTabHeaderPosition, MatTabsModule} from './index';
910

1011

1112
describe('MatTabGroup', () => {
1213
beforeEach(fakeAsync(() => {
1314
TestBed.configureTestingModule({
14-
imports: [MatTabsModule, NoopAnimationsModule],
15+
imports: [MatTabsModule, CommonModule, NoopAnimationsModule],
1516
declarations: [
1617
SimpleTabsTestApp,
1718
SimpleDynamicTabsTestApp,
@@ -21,6 +22,7 @@ describe('MatTabGroup', () => {
2122
TabGroupWithSimpleApi,
2223
TemplateTabs,
2324
TabGroupWithAriaInputs,
25+
TabGroupWithIsActiveBinding,
2426
],
2527
});
2628

@@ -195,8 +197,9 @@ describe('MatTabGroup', () => {
195197
.toBe(0, 'Expected no ripple to show up on label mousedown.');
196198
});
197199

198-
it('should set the isActive flag on each of the tabs', () => {
200+
it('should set the isActive flag on each of the tabs', fakeAsync(() => {
199201
fixture.detectChanges();
202+
tick();
200203

201204
const tabs = fixture.componentInstance.tabs.toArray();
202205

@@ -206,11 +209,12 @@ describe('MatTabGroup', () => {
206209

207210
fixture.componentInstance.selectedIndex = 2;
208211
fixture.detectChanges();
212+
tick();
209213

210214
expect(tabs[0].isActive).toBe(false);
211215
expect(tabs[1].isActive).toBe(false);
212216
expect(tabs[2].isActive).toBe(true);
213-
});
217+
}));
214218

215219
it('should fire animation done event', fakeAsync(() => {
216220
fixture.detectChanges();
@@ -567,7 +571,21 @@ describe('MatTabGroup', () => {
567571
const child = fixture.debugElement.query(By.css('.child'));
568572
expect(child.nativeElement).toBeDefined();
569573
}));
570-
});
574+
});
575+
576+
describe('special cases', () => {
577+
it('should not throw an error when binding isActive to the view', fakeAsync(() => {
578+
const fixture = TestBed.createComponent(TabGroupWithIsActiveBinding);
579+
580+
expect(() => {
581+
fixture.detectChanges();
582+
tick();
583+
fixture.detectChanges();
584+
}).not.toThrow();
585+
586+
expect(fixture.nativeElement.textContent).toContain('pizza is active');
587+
}));
588+
});
571589

572590
/**
573591
* Checks that the `selectedIndex` has been updated; checks that the label and body have their
@@ -828,3 +846,17 @@ class TabGroupWithAriaInputs {
828846
ariaLabel: string;
829847
ariaLabelledby: string;
830848
}
849+
850+
851+
@Component({
852+
template: `
853+
<mat-tab-group>
854+
<mat-tab label="Junk food" #pizza> Pizza, fries </mat-tab>
855+
<mat-tab label="Vegetables"> Broccoli, spinach </mat-tab>
856+
</mat-tab-group>
857+
858+
<div *ngIf="pizza.isActive">pizza is active</div>
859+
`
860+
})
861+
class TabGroupWithIsActiveBinding {
862+
}

src/lib/tabs/tab-group.ts

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -165,18 +165,27 @@ export class MatTabGroup extends _MatTabGroupMixinBase implements AfterContentIn
165165

166166
// If there is a change in selected index, emit a change event. Should not trigger if
167167
// the selected index has not yet been initialized.
168-
if (this._selectedIndex != indexToSelect && this._selectedIndex != null) {
169-
const tabChangeEvent = this._createChangeEvent(indexToSelect);
170-
this.selectedTabChange.emit(tabChangeEvent);
171-
// Emitting this value after change detection has run
172-
// since the checked content may contain this variable'
173-
Promise.resolve().then(() => this.selectedIndexChange.emit(indexToSelect));
168+
if (this._selectedIndex != indexToSelect) {
169+
const isFirstRun = this._selectedIndex == null;
170+
171+
if (!isFirstRun) {
172+
this.selectedTabChange.emit(this._createChangeEvent(indexToSelect));
173+
}
174+
175+
// Changing these values after change detection has run
176+
// since the checked content may contain references to them.
177+
Promise.resolve().then(() => {
178+
this._tabs.forEach((tab, index) => tab.isActive = index === indexToSelect);
179+
180+
if (!isFirstRun) {
181+
this.selectedIndexChange.emit(indexToSelect);
182+
}
183+
});
174184
}
175185

176186
// Setup the position for each tab and optionally setup an origin on the next selected tab.
177187
this._tabs.forEach((tab: MatTab, index: number) => {
178188
tab.position = index - indexToSelect;
179-
tab.isActive = index === indexToSelect;
180189

181190
// If there is already a selected tab, then set up an origin for the next selected tab
182191
// if it doesn't have one already.
@@ -256,11 +265,8 @@ export class MatTabGroup extends _MatTabGroupMixinBase implements AfterContentIn
256265
this._tabLabelSubscription.unsubscribe();
257266
}
258267

259-
this._tabLabelSubscription = merge(
260-
...this._tabs.map(tab => tab._disableChange),
261-
...this._tabs.map(tab => tab._labelChange)).subscribe(() => {
262-
this._changeDetectorRef.markForCheck();
263-
});
268+
this._tabLabelSubscription = merge(...this._tabs.map(tab => tab._stateChanges))
269+
.subscribe(() => this._changeDetectorRef.markForCheck());
264270
}
265271

266272
/** Clamps the given index to the bounds of 0 and the tabs length. */

src/lib/tabs/tab.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,8 @@ export class MatTab extends _MatTabMixinBase implements OnInit, CanDisable, OnCh
7373
return this._contentPortal;
7474
}
7575

76-
/** Emits whenever the label changes. */
77-
readonly _labelChange = new Subject<void>();
78-
79-
/** Emits whenever the disable changes */
80-
readonly _disableChange = new Subject<void>();
76+
/** Emits whenever the internal state of the tab changes. */
77+
readonly _stateChanges = new Subject<void>();
8178

8279
/**
8380
* The relatively indexed position where 0 represents the center, negative is left, and positive
@@ -101,18 +98,13 @@ export class MatTab extends _MatTabMixinBase implements OnInit, CanDisable, OnCh
10198
}
10299

103100
ngOnChanges(changes: SimpleChanges): void {
104-
if (changes.hasOwnProperty('textLabel')) {
105-
this._labelChange.next();
106-
}
107-
108-
if (changes.hasOwnProperty('disabled')) {
109-
this._disableChange.next();
101+
if (changes.hasOwnProperty('textLabel') || changes.hasOwnProperty('disabled')) {
102+
this._stateChanges.next();
110103
}
111104
}
112105

113106
ngOnDestroy(): void {
114-
this._disableChange.complete();
115-
this._labelChange.complete();
107+
this._stateChanges.complete();
116108
}
117109

118110
ngOnInit(): void {

0 commit comments

Comments
 (0)