-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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: Disable three finger pinch zoom in penMode #4725
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/excalidraw/excalidraw/Ezn4AtG6xLgbJ17B7JsHdHsYE7h6 |
This reverts commit c276279.
src/components/App.tsx
Outdated
if (this.state.penMode && isExcalidrawElement(this.state.elementType)) { | ||
return; | ||
} |
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.
can we just disable on gesture.pointers.size >= 2
(line 1936)?
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.
@dwelle: returning from onGestureChange
if gesture.pointers.size > 2 && penMode && isExcalidrawElement
results in a smooth drawing experience. If I try really hard, I can produce unwanted zoom and jump, but even deliberately trying it is hard... I need to slide my hand onto the screen before starting to draw, to achieve the jump. This way two-finger pinch-zoom and panning both works even in penMode!
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.
By my suggestion I meant that we already bail on gesture.events.size === 2
a line above, so I think we can just make the check into >= 2
and not check penMode
or anything else.
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.
So you don't want 3 finger pinch zoom, not even in "selection" mode?
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.
We'll see, but right now it's not implemented anyway. And bear in mind that the gesturechange
handler is for mac trackpad. Touchscreens are handled elsewhere.
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 made the change, but I can't test on a Mac touchpad.
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 tested on Mac and it works fine (at least there's no behavioral change from production).
src/components/App.tsx
Outdated
this.state.elementType === "freedraw" && this.state.penMode | ||
? 1 | ||
: distance / gesture.initialDistance; | ||
const scaleFactor = distance / gesture.initialDistance; |
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.
did you revert his (in f5fc2eb) by mistake?
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.
mhm, judging from the PR title change, it was intentional. But allowing pinch-zooming in pen mode is causing issues while drawing again (for me, palm is regularly detected as a pinch zoom).
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 removed it intentionally... it did not cause issues for me, but if it is causing issues for you, I am sure It will for many others as well. So sadly I'll put this back. Having pinch zoom during freedraw is very valuable. you can zoom in to work on small detail, and zoom out for bigger strokes. Needing to change tools breaks the flow.
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.
Having pinch zoom during freedraw is very valuable. you can zoom in to work on small detail, and zoom out for bigger strokes. Needing to change tools breaks the flow.
I agree. I was thinking there might be some heuristic to detect with high accuracy whether you're actually pinching with two fingers, but that's for another PR.
Both of you rock. I'll make sure to hit the funding options. |
Thanks @jgonggrijp! ❤️ |
for-obsidian #411