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

fix(click-outside): remove memory leak #319

Merged
merged 3 commits into from
Apr 23, 2024

Conversation

kkachniarz220
Copy link
Contributor

No description provided.

Copy link

nx-cloud bot commented Apr 5, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 00b681c. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 4 targets

Sent with 💌 from NxCloud.

@Directive({
selector: '[clickOutside]',
standalone: true,
providers: [provideDocumentClick()],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will create a listener for each directive, while the previous method, would create only one listener for the whole document, and each directive will use the same listener, which makes it more performant I think.

Copy link
Contributor Author

@kkachniarz220 kkachniarz220 Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about performance and i figured it out - Using multiple outsideClicks on one view will be rare.

When using outsideClick (which is standalone) we are not aware that it has global awareness.

@nartc nartc merged commit 49bff56 into ngxtension:main Apr 23, 2024
2 checks passed
eneajaho added a commit that referenced this pull request Apr 23, 2024
nartc added a commit that referenced this pull request Apr 23, 2024
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 this pull request may close these issues.

3 participants