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

Get rid of unecessary clearTimeout() #126

Open
DerZyklop opened this issue Dec 9, 2019 · 1 comment · May be fixed by #127
Open

Get rid of unecessary clearTimeout() #126

DerZyklop opened this issue Dec 9, 2019 · 1 comment · May be fixed by #127

Comments

@DerZyklop
Copy link
Contributor

I have a webapp with a lot of poppers in a calendar-view. Each calender-event can have its own popper. On logout, the calender gets destroyed and so do all the poppers. Now we get a performance issue, because of a lot of clearTimeout()'s. I found them in ngOnDestroy in popper-directive.ts:

clearTimeout(this.scheduledShowTimeout);
clearTimeout(this.scheduledHideTimeout);

We dont have a single popper with a delay - so these clearTimeout()'s are useless for us. My idea: A simple

if (this.scheduledShowTimeout !== undefined) {
  clearTimeout(this.scheduledShowTimeout);
}

instead of

clearTimeout(this.scheduledShowTimeout);

Would fix it, right? But i‘m not sure if this has any side-effects for those who use delay on their poppers.

@DerZyklop DerZyklop changed the title Get rid of unecessary timeout Get rid of unecessary clearTimeout() Dec 9, 2019
DerZyklop added a commit to DerZyklop/ngx-popper that referenced this issue Dec 9, 2019
Performance improvement: 
Added condition to `clearTimeout(...)`, so it only runs if a timeout was started.
@DerZyklop DerZyklop linked a pull request Dec 9, 2019 that will close this issue
@tonysamperi
Copy link

@DerZyklop did this, if you still need a performance boost! https://tonysamperi.github.io/ngx-popperjs/

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

Successfully merging a pull request may close this issue.

2 participants