search

briancherne / jquery-hoverIntent

hoverIntent jQuery Plug-in
https://briancherne.github.io/jquery-hoverIntent/
MIT License
826 stars 253 forks source link

Namespaces, unbinding and the .on() syntax #55

Open louisameline opened 8 years ago

louisameline commented 8 years ago

Hi,

it would be nice to manipulate hoverIntent with jQuery's .on syntax which offers namespacing and easy unbinding with .off(). When there would be no listeners left, the bindings should be automatically removed to prevent memory leaks (no need for the manual destruction suggested in #43).

$(selector).on('hoverIntent.namespace' [, selector] , handlerIn, handlerOut);
// or
$(selector).on('hoverIntent.namespace' [, selector] , handlerInOut);
// and then
$(selector).off('hoverIntent.namespace');
// or 
$(selector).off('.namespace');

It should be possible to hook on jQuery's event system to make it happen. Thank you

dim2k2006 commented 7 years ago

Is there any information about this issue? Was this functionality implemented?

usmonster commented 7 years ago

Hi @louisameline and @dim2k2006! Thanks for the ping on this. This might be a good idea, though can you please let me know if unbinding events via $(selector).off('.hoverIntent') would be a sufficient solution for your use case?

I'd just like some more discussion before deciding whether or not this is a common enough use case to include it in the upcoming milestone. :)

louisameline commented 7 years ago

Hello, no, I need to apply my own namespaces. It feels wrong that you ask me to use your namespace anyway, it's precisely done for the user to customize his stuff. Jquery's event system is built exactly to handle libraries like hoverintent and you're missing features by not using it. I can't feel comfortable with this library if you don't handle events the way jquery does, this is a jquery plugin after all. And I really mean no offense but as a developer, I thought I'd let you know that the fact that this library misses that point doesn't make me confident about the author's skills and doesn't make me want to use it. I'm not saying that's a fact, I mean it's just the impression it gives. But that might just me. Thanks anyway for what you did so far.

briancherne commented 7 years ago

Hi @louisameline! I'm sorry, but I actually took offense... which is usually what happens when you ask someone not to ;)

You should be confident in the author's and maintainer's skills, and the quality of the code, when a plugin (never part of the core library) is still compatible and actively in-use 10 YEARS after it was created... and the only open issues are of the feature-request variety.

We've made changes to modernize hoverIntent before. Per this issue (#55) I'm not opposed to extending how hoverIntent is applied to elements so long as it can be done without breaking existing implementations. Someone just needs to write the pull request.

louisameline commented 7 years ago

Hello, I apologize for hurting your feelings.

It's a good thing that this library works after 10 years, kudos for that :) What I tried to say, with the wrong words, is that IMHO it would need a fresh paint coat to meet the API quality I expect from a modern plugin.

As for breaking existing implementations, that's what major versions are for in semver, I see no issue here. The current API is unpractical and will be redundant with jQuery's event system, so it should be removed from a v2.

As a side note, the unbinding thing is real issue though, it leads to memory leaks. I was initially discouraged to make a PR as I was not sure if this project was still maintained. It seems that a v1.9 by @usmonster has been in the works for a long time, but his fork has many branches, maybe he could clarify where we can find the latest project files? It would be nice if there was a dev branch here on this project instead.

Thanks.

usmonster commented 7 years ago

Hi @louisameline!

It seems that a v1.9 by @usmonster has been in the works for a long time, but his fork has many branches, maybe he could clarify where we can find the latest project files?

To address your question/misunderstandings:

As for the change you propose, I agree with your reasoning to use the "jQuery way" of event handling, especially in preference over #43. Still, you shouldn't be surprised that we'd of course want to discuss your specific use case to see if there's general enough need, to make sure the problem (memory leak) exists on the current version, and only then decide if & how to prioritize it.

All that said, I think a PR would be welcome for this, if you're willing to make one. If not for v1.9, I can see this in a v2.0, if @briancherne agrees.

louisameline commented 7 years ago

Ok, thank you for your explanations.