briancherne / jquery-hoverIntent

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

Don't trigger during scrolling when pointer doesn't move (was: Hover events are triggered on window scroll.) #12

Open jhiggins-thrillist opened 11 years ago

jhiggins-thrillist commented 11 years ago

Without moving the mouse and scrolling the window, the mouse events are triggered. This happens since the mouse technically isn't moving.

usmonster commented 11 years ago

Does the same behavior happen when using .hover()?

jhiggins-thrillist commented 11 years ago

@usmonster yes, of course.

usmonster commented 11 years ago

Sorry, I wasn't sure if this was a bug report or feature request. :] Are you saying the hover events should not be triggered on window scroll but are? Or that they should be triggered but aren't? Can you give an example?

jhiggins-thrillist commented 11 years ago

This should probably be a feature request.

You can see an example here: http://jsfiddle.net/jhiggins/tE4KD/

Place your mouse in the circle, and use a mouse wheel or trackpad to scroll vertically over and past the blue block. The hover events are triggered, since this script is only looking at mouse movements.

A good use of this would be to avoid painting with hover events while using something like infinite scroll.

Thanks!

usmonster commented 11 years ago

Ah, I see, so if I understand correctly the feature request would be something like "change behavior (or add an option) to suppress mouse tracking when scrolling and mouse isn't actually moving" ? In other words, conditionally use event.clientX/clientY instead of (or in addition to) pageX/pageY?

roydukkey commented 11 years ago

I believe in most cases the current behavior is desired, but a new option for this would be useful in situations. @jhiggins-thrillist or @usmonster are you planning on submitting a pull request?

usmonster commented 11 years ago

I probably won't for this one, as I don't think I'd have the resources to adequately test it. I can imagine weird things happening for certain edge cases. Still, if others would want to test a version of it, I can quickly make a fork.

usmonster commented 11 years ago

Whoops, please ignore the accidental reference to issue # 11, but I forked and roughly implemented what I think @jhiggins-thrillist was requesting. Please check it out, maybe test it if you want: here.

usmonster commented 11 years ago

Updated code (should actually work now) and plugged it into @jhiggins-thrillist's fiddle: http://jsfiddle.net/tE4KD/7/

Still a proof-of-concept, but I think it addresses the issue. Depending on expected behavior, though, the if (scrolling) { return; } could be moved elsewhere in the logic--e.g., if the mouse pointer is over a hoverIntented element when scrolling stops, one might want mousemove (instead of mouseenter) to trigger handlerIn.. Up for discussion.

roydukkey commented 11 years ago

I agree the behavior you define would in most cases be desired, but it might be tricky to implement. You'll have to account for the little amount the user could move the mouse during scrolling; the mouse is most likely moving even during scrolling.

usmonster commented 11 years ago

Actually, the trickiest part of this is that there's currently a bug/browser inconsistency on WebKit/Blink browsers that would make implementing such behavior require a bit more code to work--please star the issue so it gets more attention!

It's still doable, though, and I can even reuse the same compare logic (maybe with a tiny modification) to detect mousemove intent during scrolling. Otherwise I could add a similar function to detect when scrolling is stopped and only then trigger handlerIn after the next mousemove.

Which approach/behavior seems best? (A) Detecting mouse velocity during scrolling (based on cfg.sensitivity and compare logic) and only triggering handlerIn after a certain post-scrollstart distance is reached, or (B) "pausing" hoverIntent behavior altogether until I've detected scrolling has stopped?

(Personally, I think option (B) would be more intuitive.)

briancherne commented 11 years ago

I actually like that hoverIntent fails in the same way as "hover" and I feel like this should be a defect submitted against every browser in the world! :) This isn't hubris, it's just one of my pet peeves when scrolling on the internet (needing to find a happy swim lane for my cursor while scrolling).

I would argue that option (A) is more consistent with hoverIntent and how scrolling currently breaks everything. But I like option (B) more. Scrolling should block hoverIntent from calling handlerIn.

However, if the user were to stop scrolling and the cursor was resting over an item with hoverIntent, I think it should call handlerIn.