Skip to content

Commit 2f8fdae

Browse files
crisbetocrisbeto
committed
fix(tooltip): decouple removal logic from change detection
Currently the logic in the tooltip that removes it from the DOM is run either if the trigger is destroyed or the exit animation has finished. The problem is that if the trigger is detached from change detection, but hasn't been destroyed, the exit animation will never run and the element won't be cleaned up. These changes switch to using CSS animations and manipulating the DOM node directly to trigger the animation. Fixes #19365.
1 parent 5f12539 commit 2f8fdae

File tree

6 files changed

+178
-128
lines changed

6 files changed

+178
-128
lines changed

‎src/material/tooltip/BUILD.bazel‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ ng_module(
2929
"//src/cdk/portal",
3030
"//src/cdk/scrolling",
3131
"//src/material/core",
32-
"@npm//@angular/animations",
3332
"@npm//@angular/common",
3433
"@npm//@angular/core",
3534
"@npm//rxjs",
@@ -65,7 +64,6 @@ ng_test_library(
6564
"//src/cdk/overlay",
6665
"//src/cdk/platform",
6766
"//src/cdk/testing/private",
68-
"@npm//@angular/animations",
6967
"@npm//@angular/platform-browser",
7068
],
7169
)

‎src/material/tooltip/tooltip.html‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
<div class="mat-tooltip"
1+
<div #tooltip
2+
class="mat-tooltip"
23
[ngClass]="tooltipClass"
4+
[class._mat-animation-noopable]="_animationMode === 'NoopAnimations'"
35
[class.mat-tooltip-handset]="(_isHandset | async)?.matches"
4-
[@state]="_visibility"
5-
(@state.start)="_animationStart()"
6-
(@state.done)="_animationDone($event)">{{message}}</div>
6+
(animationend)="_animationEnd($event)">{{message}}</div>

‎src/material/tooltip/tooltip.scss‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ $mat-tooltip-handset-margin: 24px;
2424
padding-right: $mat-tooltip-horizontal-padding;
2525
overflow: hidden;
2626
text-overflow: ellipsis;
27+
opacity: 0;
28+
transform: scale(0);
29+
30+
// Use a very short animation if animations are disabled so the `animationend` event still fires.
31+
&._mat-animation-noopable {
32+
animation-duration: 1ms;
33+
}
2734

2835
@include cdk-high-contrast(active, off) {
2936
outline: solid 1px;
@@ -35,3 +42,38 @@ $mat-tooltip-handset-margin: 24px;
3542
padding-left: $mat-tooltip-handset-horizontal-padding;
3643
padding-right: $mat-tooltip-handset-horizontal-padding;
3744
}
45+
46+
@keyframes mat-tooltip-show {
47+
0% {
48+
opacity: 0;
49+
transform: scale(0);
50+
}
51+
52+
50% {
53+
opacity: 0.5;
54+
transform: scale(0.99);
55+
}
56+
57+
100% {
58+
opacity: 1;
59+
transform: scale(1);
60+
}
61+
}
62+
63+
@keyframes mat-tooltip-hide {
64+
0% {
65+
opacity: 1;
66+
}
67+
68+
100% {
69+
opacity: 0;
70+
}
71+
}
72+
73+
.mat-tooltip-show {
74+
animation: mat-tooltip-show 200ms cubic-bezier(0, 0, 0.2, 1) forwards;
75+
}
76+
77+
.mat-tooltip-hide {
78+
animation: mat-tooltip-hide 100ms cubic-bezier(0, 0, 0.2, 1) forwards;
79+
}

‎src/material/tooltip/tooltip.spec.ts‎

Lines changed: 41 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ import {
1616
ViewChild,
1717
NgZone,
1818
} from '@angular/core';
19-
import {AnimationEvent} from '@angular/animations';
2019
import {By} from '@angular/platform-browser';
21-
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
2220
import {Direction, Directionality} from '@angular/cdk/bidi';
2321
import {OverlayContainer, OverlayModule, CdkScrollable} from '@angular/cdk/overlay';
2422
import {Platform} from '@angular/cdk/platform';
@@ -29,6 +27,7 @@ import {
2927
dispatchMouseEvent,
3028
createKeyboardEvent,
3129
dispatchEvent,
30+
createFakeEvent,
3231
} from '@angular/cdk/testing/private';
3332
import {ESCAPE} from '@angular/cdk/keycodes';
3433
import {FocusMonitor} from '@angular/cdk/a11y';
@@ -56,7 +55,7 @@ describe('MatTooltip', () => {
5655
platform = {IOS: false, isBrowser: true, ANDROID: false};
5756

5857
TestBed.configureTestingModule({
59-
imports: [MatTooltipModule, OverlayModule, NoopAnimationsModule],
58+
imports: [MatTooltipModule, OverlayModule],
6059
declarations: [
6160
BasicTooltipDemo,
6261
ScrollableTooltipDemo,
@@ -114,12 +113,12 @@ describe('MatTooltip', () => {
114113
fixture.detectChanges();
115114

116115
// wait till animation has finished
117-
tick(500);
116+
finishCurrentTooltipAnimation(overlayContainerElement, true);
118117

119118
// Make sure tooltip is shown to the user and animation has finished
120119
const tooltipElement = overlayContainerElement.querySelector('.mat-tooltip') as HTMLElement;
121120
expect(tooltipElement instanceof HTMLElement).toBe(true);
122-
expect(tooltipElement.style.transform).toBe('scale(1)');
121+
expect(tooltipElement.classList).toContain('mat-tooltip-show');
123122

124123
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
125124

@@ -134,7 +133,7 @@ describe('MatTooltip', () => {
134133
expect(tooltipDirective._isTooltipVisible()).toBe(false);
135134

136135
// On animation complete, should expect that the tooltip has been detached.
137-
flushMicrotasks();
136+
finishCurrentTooltipAnimation(overlayContainerElement, false);
138137
assertTooltipInstance(tooltipDirective, false);
139138
}));
140139

@@ -144,17 +143,17 @@ describe('MatTooltip', () => {
144143
tick(0);
145144
expect(tooltipDirective._isTooltipVisible()).toBe(true);
146145
fixture.detectChanges();
147-
tick(500);
146+
finishCurrentTooltipAnimation(overlayContainerElement, true);
148147

149148
tooltipDirective._overlayRef!.detach();
150149
tick(0);
151150
fixture.detectChanges();
152151
expect(tooltipDirective._isTooltipVisible()).toBe(false);
153-
flushMicrotasks();
154152
assertTooltipInstance(tooltipDirective, false);
155153

156154
tooltipDirective.show();
157155
tick(0);
156+
finishCurrentTooltipAnimation(overlayContainerElement, true);
158157
expect(tooltipDirective._isTooltipVisible()).toBe(true);
159158
}));
160159

@@ -177,7 +176,7 @@ describe('MatTooltip', () => {
177176
TestBed
178177
.resetTestingModule()
179178
.configureTestingModule({
180-
imports: [MatTooltipModule, OverlayModule, NoopAnimationsModule],
179+
imports: [MatTooltipModule, OverlayModule],
181180
declarations: [BasicTooltipDemo],
182181
providers: [{
183182
provide: MAT_TOOLTIP_DEFAULT_OPTIONS,
@@ -212,7 +211,7 @@ describe('MatTooltip', () => {
212211
TestBed
213212
.resetTestingModule()
214213
.configureTestingModule({
215-
imports: [MatTooltipModule, OverlayModule, NoopAnimationsModule],
214+
imports: [MatTooltipModule, OverlayModule],
216215
declarations: [TooltipDemoWithoutPositionBinding],
217216
providers: [{
218217
provide: MAT_TOOLTIP_DEFAULT_OPTIONS,
@@ -372,16 +371,7 @@ describe('MatTooltip', () => {
372371
tooltipDirective.hide(0);
373372
fixture.detectChanges();
374373
tick();
375-
376-
// At this point the animation should be able to complete itself and trigger the
377-
// _animationDone function, but for unknown reasons in the test infrastructure,
378-
// this does not occur. Manually call the hook so the animation subscriptions get invoked.
379-
tooltipDirective._tooltipInstance!._animationDone({
380-
fromState: 'visible',
381-
toState: 'hidden',
382-
totalTime: 150,
383-
phaseName: 'done',
384-
} as AnimationEvent);
374+
finishCurrentTooltipAnimation(overlayContainerElement, false);
385375

386376
expect(() => {
387377
tooltipDirective.position = 'right';
@@ -395,7 +385,7 @@ describe('MatTooltip', () => {
395385

396386
tooltipDirective.show();
397387
tick(0); // Tick for the show delay (default is 0)
398-
expect(tooltipDirective._tooltipInstance!._visibility).toBe('visible');
388+
expect(tooltipDirective._tooltipInstance!.isVisible()).toBe(true);
399389

400390
fixture.detectChanges();
401391
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
@@ -472,33 +462,21 @@ describe('MatTooltip', () => {
472462
it('should not try to dispose the tooltip when destroyed and done hiding', fakeAsync(() => {
473463
tooltipDirective.show();
474464
fixture.detectChanges();
475-
tick(150);
465+
finishCurrentTooltipAnimation(overlayContainerElement, true);
476466

477467
const tooltipDelay = 1000;
478468
tooltipDirective.hide();
479469
tick(tooltipDelay); // Change the tooltip state to hidden and trigger animation start
470+
finishCurrentTooltipAnimation(overlayContainerElement, false);
480471

481-
// Store the tooltip instance, which will be set to null after the button is hidden.
482-
const tooltipInstance = tooltipDirective._tooltipInstance!;
483472
fixture.componentInstance.showButton = false;
484473
fixture.detectChanges();
485-
486-
// At this point the animation should be able to complete itself and trigger the
487-
// _animationDone function, but for unknown reasons in the test infrastructure,
488-
// this does not occur. Manually call this and verify that doing so does not
489-
// throw an error.
490-
tooltipInstance._animationDone({
491-
fromState: 'visible',
492-
toState: 'hidden',
493-
totalTime: 150,
494-
phaseName: 'done',
495-
} as AnimationEvent);
496474
}));
497475

498476
it('should complete the afterHidden stream when tooltip is destroyed', fakeAsync(() => {
499477
tooltipDirective.show();
500478
fixture.detectChanges();
501-
tick(150);
479+
finishCurrentTooltipAnimation(overlayContainerElement, true);
502480

503481
const spy = jasmine.createSpy('complete spy');
504482
const subscription = tooltipDirective._tooltipInstance!.afterHidden()
@@ -507,7 +485,7 @@ describe('MatTooltip', () => {
507485
tooltipDirective.hide(0);
508486
tick(0);
509487
fixture.detectChanges();
510-
tick(500);
488+
finishCurrentTooltipAnimation(overlayContainerElement, false);
511489

512490
expect(spy).toHaveBeenCalled();
513491
subscription.unsubscribe();
@@ -580,7 +558,7 @@ describe('MatTooltip', () => {
580558
tooltipDirective.show();
581559
tick(0);
582560
fixture.detectChanges();
583-
tick(500);
561+
finishCurrentTooltipAnimation(overlayContainerElement, true);
584562

585563
let tooltipWrapper =
586564
overlayContainerElement.querySelector('.cdk-overlay-connected-position-bounding-box')!;
@@ -589,13 +567,13 @@ describe('MatTooltip', () => {
589567
tooltipDirective.hide(0);
590568
tick(0);
591569
fixture.detectChanges();
592-
tick(500);
570+
finishCurrentTooltipAnimation(overlayContainerElement, false);
593571

594572
dir.value = 'ltr';
595573
tooltipDirective.show();
596574
tick(0);
597575
fixture.detectChanges();
598-
tick(500);
576+
finishCurrentTooltipAnimation(overlayContainerElement, true);
599577

600578
tooltipWrapper =
601579
overlayContainerElement.querySelector('.cdk-overlay-connected-position-bounding-box')!;
@@ -613,15 +591,15 @@ describe('MatTooltip', () => {
613591
tooltipDirective.show();
614592
tick(0);
615593
fixture.detectChanges();
616-
tick(500);
594+
finishCurrentTooltipAnimation(overlayContainerElement, true);
617595

618596
expect(tooltipDirective._isTooltipVisible()).toBe(true);
619597
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
620598

621599
document.body.click();
622600
tick(0);
623601
fixture.detectChanges();
624-
tick(500);
602+
finishCurrentTooltipAnimation(overlayContainerElement, false);
625603
fixture.detectChanges();
626604

627605
expect(tooltipDirective._isTooltipVisible()).toBe(false);
@@ -632,10 +610,9 @@ describe('MatTooltip', () => {
632610
tooltipDirective.show();
633611
tick(0);
634612
fixture.detectChanges();
635-
636613
document.body.click();
637614
fixture.detectChanges();
638-
tick(500);
615+
finishCurrentTooltipAnimation(overlayContainerElement, true);
639616

640617
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
641618
}));
@@ -717,7 +694,7 @@ describe('MatTooltip', () => {
717694
tick(0);
718695
expect(tooltipDirective._isTooltipVisible()).toBe(true);
719696
fixture.detectChanges();
720-
tick(500);
697+
finishCurrentTooltipAnimation(overlayContainerElement, true);
721698

722699
const overlayRef = tooltipDirective._overlayRef!;
723700

@@ -727,7 +704,7 @@ describe('MatTooltip', () => {
727704
tick(0);
728705
expect(tooltipDirective._isTooltipVisible()).toBe(true);
729706
fixture.detectChanges();
730-
tick(500);
707+
finishCurrentTooltipAnimation(overlayContainerElement, true);
731708

732709
expect(overlayRef.detach).not.toHaveBeenCalled();
733710
}));
@@ -864,12 +841,12 @@ describe('MatTooltip', () => {
864841
fixture.detectChanges();
865842

866843
// wait until animation has finished
867-
tick(500);
844+
finishCurrentTooltipAnimation(overlayContainerElement, true);
868845

869846
// Make sure tooltip is shown to the user and animation has finished
870847
const tooltipElement = overlayContainerElement.querySelector('.mat-tooltip') as HTMLElement;
871848
expect(tooltipElement instanceof HTMLElement).toBe(true);
872-
expect(tooltipElement.style.transform).toBe('scale(1)');
849+
expect(tooltipElement.classList).toContain('mat-tooltip-show');
873850

874851
// After hide called, a timeout delay is created that will to hide the tooltip.
875852
const tooltipDelay = 1000;
@@ -882,7 +859,7 @@ describe('MatTooltip', () => {
882859
expect(tooltipDirective._isTooltipVisible()).toBe(false);
883860

884861
// On animation complete, should expect that the tooltip has been detached.
885-
flushMicrotasks();
862+
finishCurrentTooltipAnimation(overlayContainerElement, false);
886863
assertTooltipInstance(tooltipDirective, false);
887864
}));
888865

@@ -912,9 +889,9 @@ describe('MatTooltip', () => {
912889

913890
assertTooltipInstance(fixture.componentInstance.tooltip, false);
914891

915-
tick(250); // Finish the delay.
892+
tick(500); // Finish the delay.
916893
fixture.detectChanges();
917-
tick(500); // Finish the animation.
894+
finishCurrentTooltipAnimation(overlayContainerElement, true); // Finish the animation.
918895

919896
assertTooltipInstance(fixture.componentInstance.tooltip, true);
920897
}));
@@ -953,7 +930,7 @@ describe('MatTooltip', () => {
953930
fixture.detectChanges();
954931
tick(500); // Finish the open delay.
955932
fixture.detectChanges();
956-
tick(500); // Finish the animation.
933+
finishCurrentTooltipAnimation(overlayContainerElement, true); // Finish the animation.
957934
assertTooltipInstance(fixture.componentInstance.tooltip, true);
958935

959936
dispatchFakeEvent(button, 'touchend');
@@ -963,7 +940,7 @@ describe('MatTooltip', () => {
963940

964941
tick(500); // Finish the delay.
965942
fixture.detectChanges();
966-
tick(500); // Finish the exit animation.
943+
finishCurrentTooltipAnimation(overlayContainerElement, false); // Finish the exit animation.
967944

968945
assertTooltipInstance(fixture.componentInstance.tooltip, false);
969946
}));
@@ -977,7 +954,7 @@ describe('MatTooltip', () => {
977954
fixture.detectChanges();
978955
tick(500); // Finish the open delay.
979956
fixture.detectChanges();
980-
tick(500); // Finish the animation.
957+
finishCurrentTooltipAnimation(overlayContainerElement, true); // Finish the animation.
981958
assertTooltipInstance(fixture.componentInstance.tooltip, true);
982959

983960
dispatchFakeEvent(button, 'touchcancel');
@@ -987,7 +964,7 @@ describe('MatTooltip', () => {
987964

988965
tick(500); // Finish the delay.
989966
fixture.detectChanges();
990-
tick(500); // Finish the exit animation.
967+
finishCurrentTooltipAnimation(overlayContainerElement, false); // Finish the exit animation.
991968

992969
assertTooltipInstance(fixture.componentInstance.tooltip, false);
993970
}));
@@ -1229,3 +1206,12 @@ function assertTooltipInstance(tooltip: MatTooltip, shouldExist: boolean): void
12291206
// happens due to the `_tooltipInstance` having a circular structure.
12301207
expect(!!tooltip._tooltipInstance).toBe(shouldExist);
12311208
}
1209+
1210+
function finishCurrentTooltipAnimation(overlayContainer: HTMLElement, isVisible: boolean) {
1211+
const tooltip = overlayContainer.querySelector('.mat-tooltip')!;
1212+
const event = createFakeEvent('animationend');
1213+
Object.defineProperty(event, 'animationName', {
1214+
get: () => `mat-tooltip-${isVisible ? 'show' : 'hide'}`
1215+
});
1216+
dispatchEvent(tooltip, event);
1217+
}

0 commit comments

Comments
 (0)