Skip to content

Commit 28e3d36

Browse files
crisbetojelbourn
authored andcommitted
fix(tabs): duplicate animation events on some browsers (#13674)
Fixes all the animation-related events from the tab body being fired twice. Since a lot of the tabs functionality is tied to animations, this could cause multiple consecutive style recalculations, in addition to the `animationDone` event being fired too many times.
1 parent a320af2 commit 28e3d36

File tree

4 files changed

+30
-21
lines changed

4 files changed

+30
-21
lines changed

src/lib/tabs/tab-body.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44
params: {animationDuration: animationDuration}
55
}"
66
(@translateTab.start)="_onTranslateTabStarted($event)"
7-
(@translateTab.done)="_onTranslateTabComplete($event)">
7+
(@translateTab.done)="_translateTabComplete.next($event)">
88
<ng-template matTabBodyHost></ng-template>
99
</div>

src/lib/tabs/tab-body.ts

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ import {
2828
import {AnimationEvent} from '@angular/animations';
2929
import {TemplatePortal, CdkPortalOutlet, PortalHostDirective} from '@angular/cdk/portal';
3030
import {Directionality, Direction} from '@angular/cdk/bidi';
31-
import {Subscription} from 'rxjs';
31+
import {Subscription, Subject} from 'rxjs';
3232
import {matTabsAnimations} from './tabs-animations';
33-
import {startWith} from 'rxjs/operators';
33+
import {startWith, distinctUntilChanged} from 'rxjs/operators';
3434

3535
/**
3636
* These position states are used internally as animation states for the tab body. Setting the
@@ -125,6 +125,9 @@ export class MatTabBody implements OnInit, OnDestroy {
125125
/** Tab body position state. Used by the animation trigger for the current state. */
126126
_position: MatTabBodyPositionState;
127127

128+
/** Emits when an animation on the tab is complete. */
129+
_translateTabComplete = new Subject<AnimationEvent>();
130+
128131
/** Event emitted when the tab begins to animate towards the center as the active tab. */
129132
@Output() readonly _onCentering: EventEmitter<number> = new EventEmitter<number>();
130133

@@ -171,6 +174,21 @@ export class MatTabBody implements OnInit, OnDestroy {
171174
changeDetectorRef.markForCheck();
172175
});
173176
}
177+
178+
// Ensure that we get unique animation events, because the `.done` callback can get
179+
// invoked twice in some browsers. See https://github.com/angular/angular/issues/24084.
180+
this._translateTabComplete.pipe(distinctUntilChanged((x, y) => {
181+
return x.fromState === y.fromState && x.toState === y.toState;
182+
})).subscribe(event => {
183+
// If the transition to the center is complete, emit an event.
184+
if (this._isCenterPosition(event.toState) && this._isCenterPosition(this._position)) {
185+
this._onCentered.emit();
186+
}
187+
188+
if (this._isCenterPosition(event.fromState) && !this._isCenterPosition(this._position)) {
189+
this._afterLeavingCenter.emit();
190+
}
191+
});
174192
}
175193

176194
/**
@@ -185,27 +203,17 @@ export class MatTabBody implements OnInit, OnDestroy {
185203

186204
ngOnDestroy() {
187205
this._dirChangeSubscription.unsubscribe();
206+
this._translateTabComplete.complete();
188207
}
189208

190-
_onTranslateTabStarted(e: AnimationEvent): void {
191-
const isCentering = this._isCenterPosition(e.toState);
209+
_onTranslateTabStarted(event: AnimationEvent): void {
210+
const isCentering = this._isCenterPosition(event.toState);
192211
this._beforeCentering.emit(isCentering);
193212
if (isCentering) {
194213
this._onCentering.emit(this._elementRef.nativeElement.clientHeight);
195214
}
196215
}
197216

198-
_onTranslateTabComplete(e: AnimationEvent): void {
199-
// If the transition to the center is complete, emit an event.
200-
if (this._isCenterPosition(e.toState) && this._isCenterPosition(this._position)) {
201-
this._onCentered.emit();
202-
}
203-
204-
if (this._isCenterPosition(e.fromState) && !this._isCenterPosition(this._position)) {
205-
this._afterLeavingCenter.emit();
206-
}
207-
}
208-
209217
/** The text direction of the containing app. */
210218
_getLayoutDirection(): Direction {
211219
return this._dir && this._dir.value === 'rtl' ? 'rtl' : 'ltr';

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ describe('MatTabGroup', () => {
225225
fixture.detectChanges();
226226
tick();
227227

228-
expect(fixture.componentInstance.animationDone).toHaveBeenCalled();
228+
expect(fixture.componentInstance.animationDone).toHaveBeenCalledTimes(1);
229229
}));
230230

231231
it('should add the proper `aria-setsize` and `aria-posinset`', () => {

src/lib/tabs/tab-group.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,15 +328,16 @@ export class MatTabGroup extends _MatTabGroupMixinBase implements AfterContentIn
328328

329329
/** Removes the height of the tab body wrapper. */
330330
_removeTabBodyWrapperHeight(): void {
331-
this._tabBodyWrapperHeight = this._tabBodyWrapper.nativeElement.clientHeight;
332-
this._tabBodyWrapper.nativeElement.style.height = '';
331+
const wrapper = this._tabBodyWrapper.nativeElement;
332+
this._tabBodyWrapperHeight = wrapper.clientHeight;
333+
wrapper.style.height = '';
333334
this.animationDone.emit();
334335
}
335336

336337
/** Handle click events, setting new selected index if appropriate. */
337-
_handleClick(tab: MatTab, tabHeader: MatTabHeader, idx: number) {
338+
_handleClick(tab: MatTab, tabHeader: MatTabHeader, index: number) {
338339
if (!tab.disabled) {
339-
this.selectedIndex = tabHeader.focusIndex = idx;
340+
this.selectedIndex = tabHeader.focusIndex = index;
340341
}
341342
}
342343

0 commit comments

Comments
 (0)