fix: remove 250ms movement lockout and refactor gesture handling#77
fix: remove 250ms movement lockout and refactor gesture handling#77Nakshatra480 wants to merge 3 commits intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughRefactors touch gesture handling in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/hooks/useTrackpadGesture.ts`:
- Around line 39-56: The processMovement function currently prioritizes
scrollMode over dragging which causes drag gestures to be treated as scrolls;
update processMovement so that the dragging state is checked before the
scrollMode/ongoingTouches length branch: if dragging.current is true (or
ongoingTouches.current.length === 1 && dragging.current) send the 'move' event
via send({ type: 'move', dx: ..., dy: ... }) and return, otherwise proceed with
the existing two-finger/pinch and scrollMode logic (keep getTouchDistance,
lastPinchDist.current, pinching.current, sensitivity and ongoingTouches.current
checks intact) so drags always emit move events even when scrollMode is enabled.
🧹 Nitpick comments (2)
src/hooks/useTrackpadGesture.ts (2)
39-56: Unused parametereinprocessMovement.The
e: React.TouchEventparameter is accepted but never referenced inside the function body.🧹 Remove unused parameter
- const processMovement = (sumX: number, sumY: number, e: React.TouchEvent) => { + const processMovement = (sumX: number, sumY: number) => {And at the call site (Line 151):
- processMovement(sumX, sumY, e); + processMovement(sumX, sumY);
24-34: Consider cleaning updraggingTimeouton unmount to avoid firingsendafter the component is gone.This is pre-existing, but since the hook is being refactored, it may be worth adding a cleanup effect. A stale timeout could call
sendafter unmount.Example: add cleanup via useEffect
Add at the end of the hook body (before the
return):import { useRef, useState, useEffect } from 'react'; // ... inside the hook, before return: useEffect(() => { return () => { if (draggingTimeout.current) { clearTimeout(draggingTimeout.current); } }; }, []);
ba8c938 to
faab94f
Compare
2c8d67f to
1d09f65
Compare
|
@imxade Please review this PR where I've implemented the following improvements:
Thanks for your time and effort. |
Closes #76
Overview
The trackpad previously felt laggy or "stuck" during rapid swipes and multi-touch transitions due to an artificial 250ms lockout period.
Changes
lastEndTimeStampcheck inhandleTouchMove. This check was blocking all movement events for 250ms every time a finger was lifted, causing noticeable input delay.processMovementhelper function. This makes the code more readable and easier to maintain without changing the core behavior.lastEndTimeStampreference and its associated logic to keep the hook clean and focused.Technical Rationale
The
moved.currentflag already correctly handles the distinction between a "Tap" and a "Move" based on touch distance and initial duration. The additional timestamp check created a blackout period that interrupted the coordinate stream, especially during fluid gestures like two-finger scrolling. Removing this restores natural, real-time responsiveness.Verification Results
Summary by CodeRabbit