Skip to content

Commit b79ed1f

Browse files
committed
fix(material/tabs): endless toggling when selecting tabs in quick succession
`selectedTabChange` is an async `EventEmitter`, so its subscribers are invoked in a `setTimeout`. When the selection changes several times before those callbacks run, a callback can be delivered with an index that is no longer current. Consumers that write `event.index` back into `[selectedIndex]` then push the group back to the stale index, which emits again and results in an endless toggle. Emits the event from the same microtask as `selectedIndexChange` so that emit time and delivery time coincide, which is why `selectedIndexChange` is already unaffected. Fixes #24096
1 parent ab16b1f commit b79ed1f

2 files changed

Lines changed: 41 additions & 2 deletions

File tree

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,29 @@ describe('MatTabGroup', () => {
898898
'Child 3',
899899
]);
900900
});
901+
902+
it('should emit selectedTabChange in the same task as selectedIndexChange', async () => {
903+
const fixture = TestBed.createComponent(TabGroupWithBoundIndex);
904+
fixture.detectChanges();
905+
906+
const tabGroup = fixture.componentInstance.tabGroup;
907+
const emissions: string[] = [];
908+
909+
tabGroup.selectedIndexChange.subscribe(() => emissions.push('selectedIndexChange'));
910+
tabGroup.selectedTabChange.subscribe(() => emissions.push('selectedTabChange'));
911+
912+
fixture.componentInstance.selectedIndex = 2;
913+
fixture.changeDetectorRef.markForCheck();
914+
fixture.detectChanges();
915+
916+
// Flush the microtask queue. Both events should have been delivered by now. If
917+
// `selectedTabChange` is delivered in a later task, an index that has since been
918+
// superseded can be handed to the app, which can push a stale value back into
919+
// `selectedIndex` and cause an endless toggle.
920+
await Promise.resolve();
921+
922+
expect(emissions).toEqual(['selectedIndexChange', 'selectedTabChange']);
923+
});
901924
});
902925

903926
describe('nested tabs', () => {
@@ -1649,3 +1672,19 @@ class TabsWithAlignConfig {}
16491672
changeDetection: ChangeDetectionStrategy.Eager,
16501673
})
16511674
class TabsWithAlignCenter {}
1675+
1676+
@Component({
1677+
template: `
1678+
<mat-tab-group [selectedIndex]="selectedIndex">
1679+
<mat-tab label="One">One</mat-tab>
1680+
<mat-tab label="Two">Two</mat-tab>
1681+
<mat-tab label="Three">Three</mat-tab>
1682+
</mat-tab-group>
1683+
`,
1684+
imports: [MatTabsModule],
1685+
changeDetection: ChangeDetectionStrategy.Eager,
1686+
})
1687+
class TabGroupWithBoundIndex {
1688+
@ViewChild(MatTabGroup) tabGroup!: MatTabGroup;
1689+
selectedIndex = 0;
1690+
}

src/material/tabs/tab-group.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ export class MatTabGroup
282282

283283
/** Event emitted when the tab selection has changed. */
284284
@Output() readonly selectedTabChange: EventEmitter<MatTabChangeEvent> =
285-
new EventEmitter<MatTabChangeEvent>(true);
285+
new EventEmitter<MatTabChangeEvent>();
286286

287287
private _groupId: string;
288288

@@ -332,7 +332,6 @@ export class MatTabGroup
332332
const isFirstRun = this._selectedIndex == null;
333333

334334
if (!isFirstRun) {
335-
this.selectedTabChange.emit(this._createChangeEvent(indexToSelect));
336335
// Preserve the height so page doesn't scroll up during tab change.
337336
// Fixes https://stackblitz.com/edit/mat-tabs-scroll-page-top-on-tab-change
338337
const wrapper = this._tabBodyWrapper.nativeElement;
@@ -346,6 +345,7 @@ export class MatTabGroup
346345

347346
if (!isFirstRun) {
348347
this.selectedIndexChange.emit(indexToSelect);
348+
this.selectedTabChange.emit(this._createChangeEvent(indexToSelect));
349349
// Clear the min-height, this was needed during tab change to avoid
350350
// unnecessary scrolling.
351351
this._tabBodyWrapper.nativeElement.style.minHeight = '';

0 commit comments

Comments
 (0)