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

Add contains check for canvas when removing #229

Merged
merged 2 commits into from
Apr 30, 2024
Merged

Conversation

Bwc9876
Copy link
Contributor

@Bwc9876 Bwc9876 commented Apr 29, 2024

If we're running in a context where the current document.body has been switched, done will still try to remove the canvas from the new document.body, which will never work. This PR adds a check to ensure the canvas is contained within the body before removing it.

@Bwc9876 Bwc9876 mentioned this pull request Apr 29, 2024
2 tasks
@catdad
Copy link
Owner

catdad commented Apr 30, 2024

Makes sense, thanks for contributing!

@catdad catdad merged commit 92e2120 into catdad:master Apr 30, 2024
1 check passed
@catdad
Copy link
Owner

catdad commented Apr 30, 2024

It's worth mentioning though, body doesn't generally reset itself, you probably asked for that to happen one way or another, and it's generally a good idea to call confetti.reset() when doing that (as well as probably clean up other things too... though I can understand how that might be inconvenient to do).

@catdad
Copy link
Owner

catdad commented Apr 30, 2024

This was released in 1.9.3

@Bwc9876
Copy link
Contributor Author

Bwc9876 commented May 2, 2024

It's worth mentioning though, body doesn't generally reset itself, you probably asked for that to happen one way or another, and it's generally a good idea to call confetti.reset() when doing that (as well as probably clean up other things too... though I can understand how that might be inconvenient to do).

Ah gotcha, thank you!

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.

None yet

2 participants