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(toasts): closing multiple toasts #1019

Merged
merged 1 commit into from
Oct 8, 2019

Conversation

andrew-frueh
Copy link
Collaborator

Ensures cleanup functions are called on close with multiple toasts. The takeUntil pipe was preventing the cleanup functions from being called more than once when multiple toasts were open.

@joeskeen can you verify there aren't any unintended consequences from removing that pipe? I'm not entirely sure why it was there in the first place, it may have been an artifact of an earlier version.

closes #1018

ensures cleanup functions are called on close with multiple toasts

closes HealthCatalyst#1018
@joeskeen
Copy link
Contributor

joeskeen commented Oct 8, 2019

I'm pretty sure that takeUntil was introduced to fix the IE bug... I'll take a look at it and make sure that this doesn't cause a regression.

@joeskeen joeskeen self-assigned this Oct 8, 2019
@andrew-frueh
Copy link
Collaborator Author

Interesting. I tested on IE11 with BrowserStack and the fix looks like it's working. Let me know if you see anything different.

Copy link
Collaborator

@corykon corykon left a comment

Choose a reason for hiding this comment

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

Tested in IE 11 and Edge. Not seeing any side effects 👍

@andrew-frueh
Copy link
Collaborator Author

Thanks for testing that @corykon

@andrew-frueh andrew-frueh merged commit 2811014 into HealthCatalyst:dev Oct 8, 2019
@andrew-frueh andrew-frueh deleted the toast-cleanup branch October 8, 2019 19:55
@benjanderson
Copy link
Contributor

🎉 This PR is included in version 6.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding toasts cause error
4 participants