search

briancherne / jquery-hoverIntent

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

Check over/out methods gracefully. #52

Closed AndersDJohnson closed 8 years ago

AndersDJohnson commented 8 years ago

If either out or over methods aren't provided or aren't functions, can we not throw exceptions?

roydukkey commented 8 years ago

Why would you want to throw exceptions? HoverIntent will work fine without declaring an out function. I might only understand throwing an exception when both over and out are omitted, but I think I'd much prefer to just see the plugin quietly continue. What are your concerns about the current behavior?

AndersDJohnson commented 8 years ago

@roydukkey I said "can we not" as in, we are and shouldn't be - hence "gracefully" in title. I'm seeing this exception when defining only an "over" method and not an "out" method in the options:

Uncaught TypeError: Cannot read property 'apply' of undefined
delay @ jquery.hoverIntent.js:82(anonymous function) @ jquery.hoverIntent.js:108

with version 1.8.1 from Bower.

We should probably check that it's defined before attempting to call it here: https://github.com/briancherne/jquery-hoverIntent/blob/2790012922c1e4d1aee555a2ed9ead77b3c662cf/jquery.hoverIntent.js#L86

Other similar bugs may exist, e.g. https://github.com/briancherne/jquery-hoverIntent/blob/2790012922c1e4d1aee555a2ed9ead77b3c662cf/jquery.hoverIntent.js#L74

usmonster commented 8 years ago

Hello @adjohnson916! I'm closing this as a duplicate of #42, which was fixed in #51 and will be available in the upcoming release (or on the master branch now, if you want a preview). Please let me know if I've misunderstood the issue here.

Thanks for reporting the issue and for using the plugin!

AndersDJohnson commented 8 years ago

@usmonster Thanks. I'm not sure I want out to default to over, I'd prefer a no-op, but it's better at least. What about when over is undefined, but out isn't - then what's the actual vs. expected behaviors?

usmonster commented 8 years ago

@adjohnson916 An undefined out will default to over to remain consistent with jQuery's hover semantics. This was a correction to match the documented behavior and will likely not change, since one is expected to pass an explicit $.noop in your use case.

As for the behavior when there is an out without an over, again, according to the documentation, over is a required parameter for hoverIntent. There may be a case for defaulting over to $.noop when the plugin is invoked with an object, though this kind of invocation is often a mistake that might otherwise go unnoticed. As such, the plugin does not check for nor attempt to handle this case, so the current fail-fast behavior is both expected and desired.

It's true that this is the only place where it differs in jQuery .hover's behavior, but I believe this was a design choice considering this line in the docs:

Note, nothing prevents you from sending an empty function as the handlerIn or handlerOut functions.

Hope this answer your questions!