Destroying the presentation from 3rd party code. #50
Conversation
|
I think this is a great idea. Would you be able to add tests for this? Also worth considering here whether it would make sense to |
|
I've been preparing for this change by implementing a destroy event handler in each of my plugins. The destroy event handler unbinds any global event listeners that would otherwise get left behind. Here's an example: https://github.com/opendevise/bespoke-cursor/blob/master/lib/bespoke-cursor.js#L18-L20 |
|
It's also important to note that this makes testing life easier. |
| @@ -52,6 +52,11 @@ var from = function(selectorOrElement, plugins) { | |||
| return eventData; | |||
| }, | |||
|
|
|||
| destroy = function(customData) { | |||
| fire('destroy', createEventData(activeSlide, customData)); | |||
| parent.parentNode.removeChild(parent); | |||
There was a problem hiding this comment.
This should be a conditional in the case that the parent isn't attached to a DOM node (such as in a test).
if (parent.parentNode) {
parent.parentNode.removeChild(parent);
}
|
I think the method should have a void return value. I'm also not convinced that we need to pass the active slide. I guess it doesn't hurt, but it's not really the subject of this particular event. I think it should be a no-args event. |
|
I've integrated this suggestion. However, I decided not to have core remove the node from the DOM. That should be something that a listener does. Core just fires the event. I decided to send the current slide as payload as that could be useful when cleaning up the DOM. After the event is fired, I unregister all listeners (honoring the nature of destroy). |
|
See fbc6363 |
First of all, thank you very much for this great library.
I've been using it for my project, which involves showing multiple presentations on one page. It looks like an index of presentations, user can play them in sequential order and I'd like to make it in very seamless way, without page reload. So far, everything was good, but I noticed that some plugins like bespoke-hash bind some handlers to the window object events, and don't unbind them when bespoke instance gets destroyed (and later replaced with a new one). And there's no clear way for such plugins to unbind their handlers, since there's no destroy event in the core library.
I propose to add the destroy() method, which will fire the 'destroy' event prior to the presentation element removal. This way all plugin developers would know that there's a standard event, which they could handle and implement the clean-up.
If you find this reasonable and decide to accept this (or similar) code, I promise to create pull requests for all popular bespoke plugins with destroy handlers implementations.
Thanks!