Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popups and page performance degrade (memory leak) #439

Open
aligorji opened this issue Sep 2, 2019 · 1 comment
Open

Popups and page performance degrade (memory leak) #439

aligorji opened this issue Sep 2, 2019 · 1 comment

Comments

@aligorji
Copy link

aligorji commented Sep 2, 2019

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
Copy link
Author

aligorji commented Sep 7, 2019

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 aligorji changed the title Popups and page performance degrade Popups and page performance degrade (memory leak) Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant