-
Notifications
You must be signed in to change notification settings - Fork 252
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
return current pageX, pageY with the event #11
Comments
You're saying this doesn't work from the handlerIn and handlerOut? http://api.jquery.com/event.pageX/ |
Correct, @roydukkey. Here's an example of the behavior, which is most visible when the target element has a large area: You'll notice that the tooltip is positioned based on the pointer position at the last interval check, not at its position then the handler is actually fired. This is not good for UX. |
…window) instead of layout viewport (document) -- UNTESTED When set to true, use `event.clientX/Y` instead of `pageX/Y` to calculate mouse pointer position and movement. This prevents mouse pointer position tracking updates when the mouse is stationary within the window but the document is scrolling, which can be unintuitive behavior in some use cases. Maybe implements briancherne#11? Documentation to be updated in a separate commit if deemed acceptable. NB: This is *completely* untested and mostly a proof of concept.
Whoops! Wrong linked issue. (Meant # 12.) Ignore above commit reference. |
+1 for this issue. Worked around this temporary by passing cX and cY on the apply: 69. return cfg.over.apply(ob,[ev, {x:cX, y:cY}]); |
@flesler, your workaround is one of a few ways to do it, though I think the most appropriate solution would keep the interface as close to normal "hover" as possible, i.e. just return an event by itself. It would be easy to implement: just before the handler is triggered, simply copy the X/Y values of the last mousemove event over the X/Y properties of the original mouseenter event that would be subsequently passed to said handler. The real issue, however, is that this would be a change that could break the way some folks might be using the plugin today. They might rely on the current behavior, even though it is counterintuitive, IMO -- if you really want something as similar as possible to normal hover behavior, you arguably want the coordinates on the passed event to match the actual position of the mouse pointer at handler-time. I'm more than willing to put in a pull request to add this behavior (including nice bold text in the changelog :) if @briancherne is in agreement with my copy-coordinates approach. Perhaps it can be an option, but I think it's more correct and in line with the spirit of the plugin to have this become the default behavior. |
I'm aware that this is not a good final solution. Tried copying the values to the event (pageX,pageY) and that breaks the behavior, after an over, more are triggered once you move. |
@briancherne :bump: Any thoughts on the above discussion? Also, @flesler, can you please elaborate on how/when copying the *X/*Y values to the passed event breaks behavior? I'm not sure I understand. Perhaps provide a reference implementation and/or jsfiddle? |
@usmonster - I'm in agreement with the copy-coordinates approach, as the default (and only) behavior. Just make sure to increment the version and note the (possible) backwards incompatibility for folks that were using the pageX/pageY of the original event. If someone complains we can consider adding a configuration option. |
While implementing this I realize the possibility that the most common use case may care about the current pointer coordinates only for the Copying current coordinates to the event passed to Anyone have a compelling use case to support either option? |
I keep going back and forth on consistency vs. intuition issue, and it's blocking the release... Basically, I'd just like some/any feedback as to whether it makes more sense to leave the Thoughts? @flesler, @roydukkey, @briancherne? |
In my opinion it makes sense to leave it the way it currently is, because out means, well when the mouse moves out. So my vote is to leave It would be easy to add Wouldn't it be fine to update the
|
Thanks for the rapid feedback, @roydukkey! I agree with the semantics of out, though the same reasoning could be applied to the over case. One can imagine valid use cases where someone might want to have access to the actual handler-time coordinates instead of (or in addition to) the event-time coordinates for both mouseenter/"over" and mouseleave/"out" cases. Applying the same kind of modification for both event/handler pairs would also preserve a kind of semantic and behavioral symmetry. All that said, I may be leaning slightly toward leaving the mouseleave/out case alone, at least for this release, since the necessity of the change is currently less compelling than for the mouseenter/over case. Still, I'll leave the issue open a bit for other feedback until a decision is made. As for the proposed alternatives, I doubt we'd add a |
Right. I did overlook the over case, but do you mind elaborating on how you feel the latter could be disruptive? Though the handler would be applied as |
No problem! To elaborate, the disruption I was talking about is more to the users of the plugin, since hoverIntent is meant to be a drop-in replacement for |
This overwrites the pageX and pageX properties of the mouse event passed to the `over` handler with the coordinates obtained from the most recent tracked mousemove. This is usually more useful than the possibly-stale coordinates of the pointer at "mouseenter" time. Thisi change may break compatibility with any handler that expects the original coordinates, though these are still available on the event.originalEvent. Closes briancherne#11.
It could be generally useful if the event passed to the handlers could also hold the current X & Y coordinates of the mouse at the time of firing. This could be added as part of the event.data, or more seamlessly (though potentially an unwanted side-effect for some) copied over the pageX/pageY of the returned event itself. This would make it much easier for those who want their handler to, for example, position something relative to the location of the pointer at the time of firing.
Thoughts? If I have time and there's interest, I can put an example implementation in a pull request.
The text was updated successfully, but these errors were encountered: