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
fix(useMouse): position won't be changed on page scroll when type
is page
, closes #2922
#3244
Conversation
packages/core/useMouse/index.ts
Outdated
* Customize scrollTarget to listen to `scroll` events for type `page` | ||
* Set to `null` or `undefined` to disable | ||
* | ||
* @default null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not align with the actual implementation. We could say this is enabled by default when type is page
, and is only effective on type page
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not pretty sure that adding a scrollTarget is appropriate. Since pageX
and pageY
may not change if the scrollTarget
is not window
, it may be better to replace it with scroll?: boolean
, which defaults to true when type = "page"
.
When the scroll target is not a window
but a custom element (e.g. #app
) , user may need to customize extractors
in order to get the mouse distance from the top coordinate.
In this case, there's a limit to what vueuse can handle, and it may only be able to cover the case where the scrollTarget
is window
and the type
is page
.
I'm more confused about the API design here, so I wonder if I can change the options to scroll?: boolean
and only handle the case where the window
is the scrollTarget
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think scroll?: boolean
makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the user really needs to use reactive mouse position in a custom element with the help of useMouse
, it won't be possible. If we use scrollTarget
with a user-defined type
extractor, we can achieve it, but it's a bit tricky for the user to implement.
In this situation, use useElementBounding
and useMouse
together can also resolve the problem(like the solution in this PR)
I'll change the option to scroll?: boolean
first in current PR!
:)
packages/core/useMouse/index.ts
Outdated
@@ -99,6 +111,15 @@ export function useMouse(options: UseMouseOptions = {}) { | |||
} | |||
} | |||
|
|||
const scrollHandler = () => { | |||
if (!_prevMouseEvent || !window || type !== 'page') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!_prevMouseEvent || !window || type !== 'page') | |
if (!_prevMouseEvent || !window) |
Before submitting the PR, please make sure you do the following
fixes #123
).Description
When
type
is set topage
(which is default), it's expected to updatex
,y
when page is scrolled(related to issue #2922 ). I've added ascrollTarget
option to enable scroll detection whentype
ispage
.The mouse position of page should be updated when page scroll by recalculate the position in
scrollTarget
'sscroll
event callback.Additional context
I wonder if the new option
scrollTarget
that I added is right. Maybe just add ascroll?: boolean
will be better? Because thepage
type should related to windowscroll
event, and it's unnecessary to customizescrollTarget
.🤖 Generated by Copilot at b410832
Add
scrollTarget
option touseMouse
hook to track mouse or touch position relative to the page. UpdateuseMouse/index.ts
to handle scroll events and adjust position accordingly.🤖 Generated by Copilot at b410832
scrollTarget
option toUseMouseOptions
interface to customize the element forscroll
events (link)_prevMouseEvent
variable to store the last mouse or touch event on thetarget
element (link)event
parameter to_prevMouseEvent
inhandler
function formousemove
ortouchmove
events (link)scrollHandler
function to updatex
andy
reactive values based on_prevMouseEvent
andwindow
scroll position (link)scrollHandlerWrapper
function to wrapscrollHandler
witheventFilter
option if provided (link)scroll
event listener onscrollTarget
element usinguseEventListener
hook withscrollHandlerWrapper
as callback (link)