-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Added destroy lifecycle method #24
Conversation
Hey @kmalakoff, that would be a good addition. Here are some things that could be improved:
|
All of those sound good assuming one instance of Clipboard is managed as a group. FYI: I originally tried to use ClipboardAction directly (understanding the event delegation tradeoff) given #25 since it wold be easier to integrate with React so definitely a destroy on the ClipboardAction could be good to expose a lower-level API. That said, after multiple attempts, I couldn't get it to work so settled with the jQuery approach using Clipboard. |
Just started reviewing :) |
Thank you, pull request merged! |
Merged, tested and documented. Would you mind checking the final result and leave your opinions? Thanks for contributing @kmalakoff. |
Yeah, works well. Thanks! |
With single page apps, you may want to manage the lifecycle of the DOM more precisely. I have added a pull request for a destroy method that can be used to clean up the events when pages change.