Refactor: replaced mannual calculation to library** =@use-gesture/react library
#43
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Trackpad)
participant TouchArea as TouchArea
participant GestureLib as "@use-gesture/react"
participant Hook as useTrackpadGesture
participant Server as InputHandler
User->>TouchArea: perform gesture (touch/pointer)
TouchArea->>Hook: spread handlers (DOM events)
Hook->>GestureLib: gesture binder receives events
GestureLib->>Hook: emits drag/pinch/tap etc.
Hook->>Server: send({ type: 'move'|'scroll'|'swipe'|'click'|'text', ... })
Server->>Server: handleMessage() wrapped in try/catch -> execute mapped input actions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/components/Trackpad/TouchArea.tsx`:
- Around line 3-8: TouchAreaProps declares isTracking but the TouchArea
component never consumes it; remove the unused required prop or make it optional
to avoid forcing callers to pass a noop value. Update the TouchAreaProps
interface by deleting the isTracking: boolean entry (or change it to
isTracking?: boolean) and then run a quick search for TouchAreaProps/type usages
to adjust any call sites or tests that currently supply isTracking so they no
longer must provide it; ensure handlers and status remain unchanged.
In `@src/hooks/useTrackpadGesture.ts`:
- Around line 52-61: The setTimeout in onDragEnd can call send after unmount;
fix by tracking the timeout ID in the useTrackpadGesture hook (e.g., a timeoutId
ref) and either clearTimeout(timeoutId.current) before scheduling and on the
hook cleanup, or add a mounted ref check that prevents calling send if
unmounted. Update onDragEnd to store the returned timer id when scheduling the
second click, clear any prior timer before creating a new one, and add a cleanup
effect that clears timeoutId.current (or flips mounted to false) so send is
never invoked after unmount.
- Around line 36-50: In the touches === 3 branch the code zeroes both axes when
one threshold is hit, so a diagonal swipe drops the second direction; update the
logic in the swipe handling inside the touches === 3 block (where
swipeAccumulator.current, SWIPE_THRESHOLD and send are used) to only reset the
axis that triggered the send (i.e., set swipeAccumulator.current.x = 0 when
sending an x swipe and swipeAccumulator.current.y = 0 when sending a y swipe) so
the other axis value is preserved and can still trigger its own send.
In `@src/server/InputHandler.ts`:
- Around line 47-62: Wrap each switch case body in InputHandler.ts inside its
own block to prevent scope leakage for block-scoped declarations: for the
'scroll' case (where const invert is declared) and the 'zoom' case (and any
other cases declaring const/let), surround the case's statements with { ... } so
that const invert and other variables are block-scoped to their case; update the
case labels for 'scroll' and the 'zoom' handling in the switch inside the
InputHandler class (or function) accordingly, ensuring no identifiers like
invert are declared twice across sibling cases.
- Around line 81-109: The 'swipe' case in InputHandler.ts risks leaving
modifiers pressed if keyboard.type() throws; wrap each modifier sequence in a
try/finally so that any pressed keys are always released — for example, after
calling keyboard.pressKey(Key.LeftSuper) and keyboard.pressKey(Key.LeftControl)
ensure a finally block calls keyboard.releaseKey(Key.LeftControl) and
keyboard.releaseKey(Key.LeftSuper); do the same for the other branches that
press LeftSuper (up/down/right) so every pressKey has a matching releaseKey in a
finally block around keyboard.type() calls.
🧹 Nitpick comments (2)
src/server/InputHandler.ts (1)
83-83: Remove debugconsole.logor downgrade to a debug-level log.This will fire on every 3-finger swipe in production. If a structured logger is available, use it at debug level; otherwise, remove the line.
src/hooks/useTrackpadGesture.ts (1)
4-5:sendparameter typed asany— loses type safety withInputMessage.Since
InputMessageis already defined insrc/server/InputHandler.ts, importing and using it here would catch malformed messages at compile time (e.g., a missingdirectionon swipe, or a typo in thetypefield).♻️ Suggested change
+import type { InputMessage } from '../server/InputHandler'; + export const useTrackpadGesture = ( - send: (msg: any) => void, + send: (msg: InputMessage) => void, scrollMode: boolean, sensitivity: number = 1.5 ) => {
aniket866
force-pushed
the
added-more-guestures
branch
from
3321e6f to
eca54c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/hooks/useTrackpadGesture.ts`:
- Around line 21-47: onDrag currently ignores drags when maxFingers.current >= 3
so three-finger swipe gestures are dropped; implement a branch in the onDrag
handler that detects three-finger gestures by accumulating scaledDx/scaledDy
into a three-finger gesture accumulator (e.g., threeFingerDelta or similar
state), apply a configurable threshold to determine direction
(left/right/up/down) and then call send(...) with the appropriate gesture event
(e.g., 'switch-desktop' / 'task-view' / 'show-desktop' or a single
'three_finger_swipe' with direction payload), and ensure the accumulator is
reset on drag end (onDrag end handler) and that sensitivity, maxFingers.current,
and scrollMode logic remain unchanged; if three-finger behavior was
intentionally omitted, update the PR description instead.
🧹 Nitpick comments (2)
src/hooks/useTrackpadGesture.ts (1)
50-61:onPinchStartsets tracking but doesn't coordinate withonDragStart/onDragEndformaxFingers.
onPinchStartsetsisTracking = truebut never touchesmaxFingers. If a pinch fires without a drag (or after drag ended and resetmaxFingersto 0), a subsequentonDragEndtap check would seemaxFingers === 0and skip the click — which is probably fine. But ifonPinchEndfires beforeonDragEnd,isTrackingis set tofalsewhile the drag may still be active. Consider whether the two tracking states should be independent.src/components/Trackpad/TouchArea.tsx (1)
6-6:handlers: anydefeats type safety.Consider using a more specific type.
use-gesture'sbind()returnsReact.DOMAttributes<HTMLDivElement>(or similar). At minimum,React.HTMLAttributes<HTMLDivElement>would catch typos and invalid handler props at compile time.- handlers: any; // Accepts the spreadable event handlers from `@use-gesture/react` + handlers: React.DOMAttributes<HTMLDivElement>;
|
hi @imxade I have completed the the migration from mannual math calculation to Thankyou so much |
anchalkapoor424
left a comment
anchalkapoor424
left a comment
There was a problem hiding this comment.
I have reviewed and almost everything is good to go on.
nice idea by maintainer to replace manual calculation to library based approach,
|
@aniket866 it's not behaving properly in my case (NixOS, Wayland) two finger scrolling zooms tapping doesn't register a click |
hi @imxade , thankyou for review my PR |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/hooks/useTrackpadGesture.ts`:
- Around line 28-52: The onDrag handler in useTrackpadGesture is using the
per-frame delta value to detect if movement exceeds TAP_MOVE_THRESHOLD, which
causes slow drags with small frame-by-frame movements to be incorrectly
identified as taps. Replace the threshold check that uses delta (obtained from
destructuring the delta property) with a check using the cumulative movement
value instead. Extract movement from the gesture event parameter and use
Math.abs(movement[0]) and Math.abs(movement[1]) in the comparison against
TAP_MOVE_THRESHOLD, while continuing to use delta for the scroll and move event
payloads.
- Around line 54-72: The bug is that touchCountRef is only updated in onDrag so
zero-movement taps read a stale value; update touchCountRef when pointers start
by capturing the current pointer/finger count in onPointerDown (or onTouchStart)
so onPointerUp uses the correct value; locate touchCountRef, onPointerUp and
onDrag in useTrackpadGesture.ts and add logic in onPointerDown (or the gesture
library's touch-start handler) to set touchCountRef.current to the active
pointer count before computing button and calling send({type: 'click', ...})
using TAP_TIME for tap detection.
In `@src/routes/trackpad.tsx`:
- Around line 50-55: onLeftClick and onRightClick currently call send(...) with
press: true and never send a release, leaving buttons held on the server; update
those callbacks to mirror the tap handler pattern in useTrackpadGesture.ts by
sending the press=true event immediately via send({ type: 'click', button:
'left'|'right', press: true }) and then scheduling a release with a short
setTimeout that calls send({ type: 'click', button: 'left'|'right', press: false
}); ensure you reference the existing send function and the same delay
value/pattern used by the tap handler to keep behavior consistent.
🧹 Nitpick comments (4)
src/server/InputHandler.ts (2)
50-55: Scroll threshold removed — small noisy deltas will now fire scroll calls.The previous implementation had a
Math.abs(amount) >= 0.1guard. The new code firesmouse.scrollDown/mouse.scrollRightfor any truthymsg.dy/msg.dx, including very small fractional values that@use-gesture/reactmay emit on every frame. This could cause jittery scrolling or unnecessary IPC overhead.Consider re-adding a minimum threshold:
Proposed fix
case 'scroll': { const invert = CONFIG.MOUSE_INVERT ? -1 : 1; - if (msg.dy) await mouse.scrollDown(msg.dy * invert); - if (msg.dx) await mouse.scrollRight(msg.dx * invert); + const dy = msg.dy ? msg.dy * invert : 0; + const dx = msg.dx ? msg.dx * invert : 0; + if (Math.abs(dy) >= 0.5) await mouse.scrollDown(dy); + if (Math.abs(dx) >= 0.5) await mouse.scrollRight(dx); break; }
50-55:await mouse.getPosition()on every move event adds latency on the hot path.Every single pointer-move event round-trips through
mouse.getPosition()(Line 26) before setting the new position. For a high-frequency drag stream this can introduce noticeable cursor lag, especially on systems where nut-js queries the OS synchronously under the hood. If@nut-tree-fork/nut-jsexposes a relative-move API (e.g.,mouse.move(...)with a direction/offset), that would avoid the read entirely.Also applies to: 24-33
src/components/Trackpad/TouchArea.tsx (2)
6-6:handlers: anydiscards type safety.The gesture binder from
@use-gesture/reactreturns a well-typed object. Typing this asanysilently swallows mismatches. A more precise type preserves safety without coupling to internal library types:Proposed fix
interface TouchAreaProps { scrollMode: boolean; isTracking: boolean; - handlers: any; + handlers: React.DOMAttributes<HTMLDivElement>; status: 'connecting' | 'connected' | 'disconnected'; }
17-21: Consider addingrole="application"for the interactive touch surface.Biome flags this
<div>fornoStaticElementInteractionssince it receives pointer/touch handlers but has no ARIA role. For a virtual trackpad surface,role="application"is the most semantically appropriate choice, signaling that the region has custom interaction semantics.Proposed fix
<div className="flex-1 bg-neutral-800 relative touch-none select-none flex items-center justify-center p-4" + role="application" {...handlers} onContextMenu={(e) => e.preventDefault()} >
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
hi @imxade now this PR involves
I tried to implement zoom but sensitivity is too high causing accidental zoom-in and zoom-out, so I think we need to work on zoom separately for adding smooth zoom with showing zoom percentage , that will be totally different task so I will fix that soon in another PR. I have made this PR ready to be merged removing all merge conflicts , so can be merged , let me know if any issue
thankyou , @imxade |
3 participants
First of all replaced mannual calculation to library =
@use-gesture/reactlibraryreason: faster** processing ,less prone to errors by mannual calculations
Deleted Manual Math Logic (The biggest chunk)
Removed:
src/utils/math.tsReason: Previously, the app had to manually calculate distances, angles, and velocities between touch points to detect pinches and zooms. This required a lot of raw math code.
Replacement: The new
@use-gesture/reactlibrary handles all this math internally. We no longer need to maintain complex vector geometry functions.File:
src/hooks/useTrackpadGesture.tsChange: We deleted the large blocks of code that manually listened to onTouchStart, onTouchMove, and onTouchEnd and tried to figure out "is this a scroll or a drag?".
Summary of Updates: updated versions of:
package.json (Added dependency)src/utils/math.ts (Cleaned up)src/hooks/useTrackpadGesture.ts (Rewritten with library)src/components/Trackpad/TouchArea.tsx (Updated props)src/routes/trackpad.tsx (Updated hook usage)src/server/InputHandler.ts (Verified)Closes #41
Thankyou
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores