Skip to content
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

Exception thrown when holding ctrl and simultaneously scroll the wheel on the mouse fast enough #551

Closed
2 tasks done
tokarchyn opened this issue Oct 31, 2022 · 22 comments · Fixed by #596
Closed
2 tasks done
Assignees

Comments

@tokarchyn
Copy link

Describe the bug
Exception thrown when holding ctrl and simultaneously scroll the wheel on the mouse fast enough.

TypeError
Cannot read properties of undefined (reading 'clientX')
    at distanceAngle (https://7rfcn.csb.app/node_modules/
use-gesture/core/dist/actions-71ad3053.esm.js:242:17
    at PinchEngine.pointerMove (https://7rfcn.csb.app/node_modules/
use-gesture/core/dist/actions-71ad3053.esm.js:1157:21
    at HTMLDivElement.eval (https://7rfcn.csb.app/node_modules/
use-gesture/core/dist/actions-71ad3053.esm.js:316:19

Sandbox or Video
Card-zoom sandbox from the documentation: https://codesandbox.io/s/github/pmndrs/use-gesture/tree/main/demo/src/sandboxes/card-zoom

Information:

  • Use Gesture version: 10.2.21
  • Device: Laptop
  • OS: Windows 11
  • Browser Chrome

Checklist:

  • I've read the documentation.
  • If this is an issue with drag, I've tried setting touch-action: none to the draggable element.
@LSafer
Copy link

LSafer commented Nov 1, 2022

Same with Firefox with the exception P2 is undefined (also reading clientX)

@dbismut
Copy link
Collaborator

dbismut commented Nov 1, 2022

I'm not able to reproduce... Are both your machines running windows? Are you using a trackpad? @LSafer @tokarchyn

@LSafer
Copy link

LSafer commented Nov 1, 2022

@dbismut I had it on windows using mouse

@LSafer
Copy link

LSafer commented Nov 1, 2022

I think it got something to do with tracking pointerIds

My setup was: Next.js + useSpring + useGuesture + mouse and a touch screen
with the ref passed in the options and not using the returned binding function and with onDrag and onPinch being setup

When I tracked touches count on onDrag it says 1, when I use one finger or the mouse.
But after a while (not sure why), it starts saying 2. Even though, I am still using one finger.
It is not the reason for the error. But, I think it is another side effect of the same problem

That's all I got since I got tiered and used react-zoom-pan-pinch

@dbismut
Copy link
Collaborator

dbismut commented Nov 1, 2022

Tracking pointer ids can be a complex problem. Would you mind sharing a sandbox so I can have a look? Also are you using a Microsoft Surface by any chance?

@LSafer
Copy link

LSafer commented Nov 1, 2022

@dbismut Ok, it might take a while though and no I use second screen feature in my galaxy tab to test touch input (I use a windows desktop PC with keyboard and mouse)

@LSafer
Copy link

LSafer commented Nov 1, 2022

Sorry I couldn't reproduce it. I haven't committed the code until I changed it to a working solution

The code was using react-image-lightbox and I was searching for another solution since it doesn't work on react 18

@LSafer
Copy link

LSafer commented Nov 2, 2022

Umm, actually, the error occurs on this demo page:

https://codesandbox.io/s/github/pmndrs/use-gesture/tree/main/demo/src/sandboxes/card-zoom

Click Open in a new window and zoom using CTRL + mouse wheel

@dbismut
Copy link
Collaborator

dbismut commented Nov 2, 2022

@LSafer thank you very much I'll have to get some hardware to reproduce this. Just so you know, pointer ids bug exists because I wanted the lib to support multi hands gestures, so two hands being able to interact pinching two different objects at the same time. This requires some logic that's a bit hard to master cross-devices...

@LSafer
Copy link

LSafer commented Nov 2, 2022

@dbismut you're welcome.

The error must be something that takes a while like caching pointer ids or not forgetting
stale pointer ids or storing pointer ids in internal storage or calculating configuration at
setup and not updating it.

I tried everything to find a clear way of reproducing the error
and I am sorry but 'after a while' is all I got.

Also, I think you should make multi hands gesture an opt-in feature (regardless of the error)
I think, by default, people (and dumb people like me) don't want it (at least for simple use cases).

@LindaLuesink
Copy link

I have the same error on windows11 in chrome in the sandbox on desktop. You can reproduce it to drag the cart a bit to the right and zoom in and out with ctrl key and scrolling with the mousewheel.
image

@baptisteArno
Copy link

I have lots of users who get this error on Windows! Any chance we can suppress this error just by null checking where it reads clientX without having to reproduce the issue? @dbismut

@dbismut
Copy link
Collaborator

dbismut commented Mar 21, 2023

I guess we could indeed...

@dbismut
Copy link
Collaborator

dbismut commented Mar 23, 2023

@baptisteArno I've just tried Browserstack and the problem doesn't show when using my MagicMouse. I'll try fixing this with my eyes closed.

@dbismut dbismut mentioned this issue Apr 5, 2023
@dbismut
Copy link
Collaborator

dbismut commented Apr 5, 2023

Hi @baptisteArno and everyone. I've attempted a blind fix here #596
Any way you could test if it fixes anything?

@baptisteArno
Copy link

Can it be pulled from npm repo?

@dbismut
Copy link
Collaborator

dbismut commented Apr 6, 2023

This should work: https://stackoverflow.com/questions/33181297/npm-install-from-github-pull-request

So npm i --save pmndrs/use-gesture#pull/596/head.

If this doesn't work, you can also try adding this to your package.json:

    "@use-gesture/react": "https://pkg.csb.dev/pmndrs/use-gesture/commit/2694b736/@use-gesture/react",

It uses the build from codesandbox.

@baptisteArno
Copy link

Unfortunately, I don't have a way to check if it fixes the issue, as I can only "test" this in production, and I'd rather not to…

@lindabeslist Can you try it out?

@tokarchyn
Copy link
Author

@dbismut The error is gone 🙂I can easily reproduce an error on all previous versions, but not on pmndrs/use-gesture#pull/596/head.

@dbismut
Copy link
Collaborator

dbismut commented Apr 6, 2023

Great! Thanks for testing. I'll release this asap.

@LindaLuesink
Copy link

Indeed, the error is gone. Thank you for fixing :)

@dbismut
Copy link
Collaborator

dbismut commented Apr 7, 2023

Released 10.2.26!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants