Destroy animation players and clean resources (#10267)

* fix(overlay): destroy animation players and clean resources, #8450 Co-authored-by: Milko Venkov <[email protected]> Co-authored-by: Nikolay Alipiev <[email protected]>
IgniteUI · Oct 21, 2021 · bc37310 · bc37310
1 parent bb422f8
commit bc37310
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 10 deletions.
diff --git a/projects/igniteui-angular/src/lib/directives/toggle/toggle.directive.ts b/projects/igniteui-angular/src/lib/directives/toggle/toggle.directive.ts
@@ -322,7 +322,7 @@ export class IgxToggleDirective implements IToggleView, OnInit, OnDestroy {
         if (this.navigationService && this.id) {
             this.navigationService.remove(this.id);
         }
-        if (!this.collapsed && this._overlayId) {
+        if (this._overlayId) {
             this.overlayService.detach(this._overlayId);
         }
         this.unsubscribe();

diff --git a/projects/igniteui-angular/src/lib/services/overlay/overlay.ts b/projects/igniteui-angular/src/lib/services/overlay/overlay.ts
@@ -644,7 +644,9 @@ export class IgxOverlayService implements OnDestroy {
             // to eliminate flickering show the element just before animation start
             info.wrapperElement.style.visibility = 'hidden';
         }
-        this.closed.emit({ id: info.id, componentRef: info.componentRef, event: info.event });
+        if (!info.closeAnimationDetaching) {
+            this.closed.emit({ id: info.id, componentRef: info.componentRef, event: info.event });
+        }
         delete info.event;
     }
 
@@ -659,15 +661,14 @@ export class IgxOverlayService implements OnDestroy {
         if (info.componentRef) {
             this._appRef.detachView(info.componentRef.hostView);
             info.componentRef.destroy();
+            delete info.componentRef;
         }
         if (info.hook) {
             info.hook.parentElement.insertBefore(info.elementRef.nativeElement, info.hook);
             info.hook.parentElement.removeChild(info.hook);
             delete info.hook;
         }
-        if (info.wrapperElement) {
-            delete info.wrapperElement;
-        }
+
         const index = this._overlayInfos.indexOf(info);
         this._overlayInfos.splice(index, 1);
 
@@ -679,6 +680,22 @@ export class IgxOverlayService implements OnDestroy {
             }
             this.removeCloseOnEscapeListener();
         }
+
+        // clean all the resources attached to info
+        delete info.elementRef;
+        delete info.settings;
+        delete info.initialSize;
+        info.openAnimationDetaching = true;
+        info.openAnimationPlayer?.destroy();
+        delete info.openAnimationPlayer;
+        delete info.openAnimationInnerPlayer;
+        info.closeAnimationDetaching = true;
+        info.closeAnimationPlayer?.destroy();
+        delete info.closeAnimationPlayer;
+        delete info.closeAnimationInnerPlayer;
+        delete info.ngZone;
+        delete info.wrapperElement;
+        info = null;
     }
 
     private playOpenAnimation(info: OverlayInfo) {
@@ -924,7 +941,9 @@ export class IgxOverlayService implements OnDestroy {
     }
 
     private openAnimationDone(info: OverlayInfo) {
-        this.opened.emit({ id: info.id, componentRef: info.componentRef });
+        if (!info.openAnimationDetaching) {
+            this.opened.emit({ id: info.id, componentRef: info.componentRef });
+        }
         if (info.openAnimationPlayer) {
             info.openAnimationPlayer.reset();
             // calling reset does not change hasStarted to false. This is why we are doing it here via internal field
@@ -940,7 +959,6 @@ export class IgxOverlayService implements OnDestroy {
     }
 
     private closeAnimationDone(info: OverlayInfo) {
-        this.closeDone(info);
         if (info.closeAnimationPlayer) {
             info.closeAnimationPlayer.reset();
             // calling reset does not change hasStarted to false. This is why we are doing it here via internal field
@@ -954,16 +972,17 @@ export class IgxOverlayService implements OnDestroy {
             // calling reset does not change hasStarted to false. This is why we are doing it here via internal field
             (info.openAnimationPlayer as any)._started = false;
         }
+        this.closeDone(info);
     }
 
     private finishAnimations(info: OverlayInfo) {
         // TODO: should we emit here opened or closed events
-        if (info.openAnimationPlayer) {
+        if (info.openAnimationPlayer && info.openAnimationPlayer.hasStarted()) {
             info.openAnimationPlayer.reset();
             // calling reset does not change hasStarted to false. This is why we are doing it here via internal field
             (info.openAnimationPlayer as any)._started = false;
         }
-        if (info.closeAnimationPlayer) {
+        if (info.closeAnimationPlayer && info.closeAnimationPlayer.hasStarted()) {
             info.closeAnimationPlayer.reset();
             // calling reset does not change hasStarted to false. This is why we are doing it here via internal field
             (info.closeAnimationPlayer as any)._started = false;

diff --git a/projects/igniteui-angular/src/lib/services/overlay/utilities.ts b/projects/igniteui-angular/src/lib/services/overlay/utilities.ts
@@ -147,8 +147,14 @@ export interface OverlayInfo {
     initialSize?: Size;
     hook?: HTMLElement;
     openAnimationPlayer?: AnimationPlayer;
-    closeAnimationPlayer?: AnimationPlayer;
     openAnimationInnerPlayer?: any;
+    // calling animation.destroy in detach fires animation.done. This should not happen
+    // this is why we should trace if animation ever started
+    openAnimationDetaching?: boolean;
+    closeAnimationPlayer?: AnimationPlayer;
+    // calling animation.destroy in detach fires animation.done. This should not happen
+    // this is why we should trace if animation ever started
+    closeAnimationDetaching?: boolean;
     closeAnimationInnerPlayer?: any;
     ngZone: NgZone;
     transformX?: number;