ng2-semantic-ui icon indicating copy to clipboard operation
ng2-semantic-ui copied to clipboard

Popups and page performance degrade (memory leak)

Open aligorji opened this issue 6 years ago • 1 comments
trafficstars

This problem #194 still exists For example on the following page https://edcarroll.github.io/ng2-semantic-ui/#/modules/popup In first example Showing and hiding popup after about 20 times! (but very fast pointer hover on button) causes the screen to become extremely slow (even scrolling page is difficult) Until the page is reload

aligorji avatar Sep 02 '19 22:09 aligorji

After review... I found the problem. This problem occurs when the popup open and close quickly (and popupDelay is small) In the code below: src\modules\popup\components\popup.ts

public close():void {
...
   clearTimeout(this.closingTimeout);
   this.closingTimeout = window.setTimeout(() => this.onClose.emit(), this.config.transitionDuration);

   this._isOpen = false;

The popup is not fully closed yet and requires some animation time. In the meantime, run isOpen = false before this.onClose.emit(), which allows the open method to run again. And clearTimeout(this.closingTimeout); clear previoussetTimeout in above code. and this will cause the "sometimes" this.onClose.emit() not raise! and not destroy PositioningService instance and not remove _documentListener: src\modules\popup\classes\popup-controller.ts

...
this.popup.onClose.subscribe(() => this.cleanup());
...
        // Add a listener to the document body to handle closing.
        this._documentListener = this._renderer
            .listen("document", "click", (e: MouseEvent) =>
                this.onDocumentClick(e));
    protected cleanup(): void {
        clearTimeout(this._openingTimeout);

        if (this._componentRef.instance && this._componentRef.instance.positioningService) {
            this._componentRef.instance.positioningService.destroy();   <== here
        }

        this._componentFactory.detachFromApplication(this._componentRef);

        if (this._documentListener) {
            this._documentListener();   <== here
        }
    }

And finally => memory leak! and hundreds of events are not deleted... I think ... solved this way ... anyone have a comment ?! src\modules\popup\components\popup.ts

public close():void {
......
this.closingTimeout = window.setTimeout(() => {
   this.onClose.emit();
   this._isOpen = false;   <== here
}, this.config.transitionDuration);

src\modules\popup\classes\popup-controller.ts

public open(): void {
        if (this.popup.isOpen) {
            return;
        }
....
public close(): void {

        // Cancel the opening timer to stop the popup opening after close has been called.
        clearTimeout(this._openingTimeout);

        if (!this.popup.isOpen) {
            return;
        }
....

aligorji avatar Sep 07 '19 05:09 aligorji