-
Notifications
You must be signed in to change notification settings - Fork 1
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
Destroying plugins that create DOM elements outside of plugin selector #2
Comments
mmm, I have tried to create handlers so that you could add your own custom functionality for this kind of stuff. My first inclination is that an plugin which is adding nodes outside of the original node is a bit backwards in their logic. They should instead be wrapping the contents with an inner node - which is quite common for animations and stuff like that: <!-- original markup -->
<div class="my-component">
<a>...</a>
<b>...</b>
<footer>...</footer>
</div> Then you do something like <div class="my-component">
<div class="added-for-animation">
<a>...</a>
<b>...</b>
<footer>...</footer>
</div>
</div> If your plugin is adding extra markup outside our component, then you should use the $('.my-component').responsivePlugin({
pluginName: "some-plugin",
destroyMethodName: "destroy",
breakpointOptions: {
"only screen and (min-width: 48em)": {
options: {},
callbackAfter: function() {
// remember this === $('.my-component')
this.parent().replaceWith(this);
}
}
}
}); |
Totally agree with you about the extra nodes outside of the original selector being backwards. In fact, I nuked the plugin and am just rolling my own solution. This is something that's likely to come up from time to time though, so I thought it was worth noting. Correct me if I'm wrong, but the callbackAfter will fire only on a breakpoint match, not an unmatch event. So in the example above the the plugin would initialize adding the extra markup and then the callbackAfter function would effectively remove it, which would likely break the plugin's functionality. Would it make sense to add a callbackCleanup callback that would fire on unmatch? Again, I've just done a quick read through of your plugin, so I could be totally off here. Also, unrelated, but I noticed a number of references to "destryMethodName" which seems to be a typo for "destroyMethodName". Maybe i'm missing something though. Thanks for your response and taking the time to look into this. |
Holy crap! I had a super-bad typo in the code which I have just [embarrassingly] discovered. I have updated it - please pull the latest version. Also, you are correct in that my last example will not work for you. Instead, try passing a function to the "destroyMethodName" option: $('.my-component').responsivePlugin({
pluginName: "some-plugin",
destroyMethodName: function() {
// Check to make sure the parentNode is what you expect before moving forward...
if( this.parent().hadClass('foo') )
this.parent().replaceWith(this);
}
breakpointOptions: {
"only screen and (min-width: 48em)": {}
}
}); I am not opposed to changing the API for this either and am open for suggestions. Maybe a set of callbacks is in order:
|
I've been trying to come up with a solution for destroying plugins that wrap the original selector with additional markup. The saveClone method only accounts for changes made within the original selector. I'm going to play around with saving the state of the parent node as well and checking to see if any changes have been made within, but I was wondering if anyone's looked into this already or if there are plans to add support for this feature.
The text was updated successfully, but these errors were encountered: