feat: 020 — drag and toss pigs with physics - #22
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR implements drag-and-toss physics for pigs. It adds pointer-driven drag handlers to ChangesDrag-and-Toss Physics System
Sequence DiagramsequenceDiagram
actor User
participant PigSprite
participant App
participant usePigMovement
participant Physics as Physics Engine
User->>PigSprite: onPointerDown (x, y)
PigSprite->>usePigMovement: startDrag(pigId, x, y)
usePigMovement->>usePigMovement: Store drag state, clear pointer history
loop While pointer moves
User->>PigSprite: onPointerMove (x, y)
PigSprite->>PigSprite: Compute delta from start
alt Delta >= DRAG_THRESHOLD
PigSprite->>usePigMovement: moveDrag(x, y)
usePigMovement->>usePigMovement: Update pig position, record pointer sample
end
end
User->>PigSprite: onPointerUp
PigSprite->>usePigMovement: endDrag()
alt wasDrag === true (threshold exceeded)
usePigMovement->>usePigMovement: computeTossVelocity(pointer history)
usePigMovement->>usePigMovement: Set pig vx/vy to computed velocity
usePigMovement->>Physics: Pig coasts with friction
else wasDrag === false (pure click)
PigSprite->>App: onClick()
App->>App: Open PigDetail
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 6/10 reviews remaining, refill in 18 minutes and 36 seconds. 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/App.tsx`:
- Around line 34-41: The component-level function handleAddTask is performing
business I/O by calling focusWriter.appendTask(selectedFocus.id, text); move
this mutation behind a hook or action (e.g., create a useFocusActions hook with
an appendTask method) so the view remains I/O-free: implement a new hook
function (useFocusActions.appendTask) that accepts focusId and text, calls
focusWriter.appendTask and handles/logs errors, then have the component receive
that callback (either via the hook or as a prop) and replace direct calls to
focusWriter.appendTask in handleAddTask with invoking the provided
appendTask(selectedFocus.id, text); keep selectedFocus only as a view-level
prop/state and ensure error handling stays inside the hook.
In `@src/components/PigDetail.tsx`:
- Line 13: The onAddTask callback in PigDetail is currently fire-and-forget and
the component clears taskInput immediately; change the onAddTask signature to
return a Promise (resolve on success / reject or false on failure) and update
the caller (the append handler in App.tsx) to return that promise/result so
callers know success/failure; in PigDetail await the returned promise from
onAddTask and only clear taskInput when it resolves successfully (handle
rejection by leaving the draft intact and surfacing an error), and apply the
same change for the other occurrence referenced around the second block (lines
74-85) so both code paths wait for append success before clearing the input.
In `@src/components/PigSprite.tsx`:
- Around line 57-80: The pointer handlers lack cleanup for canceled/losing
pointer captures causing onDragEnd not to run; add handlers for onPointerCancel
and onLostPointerCapture that mirror the onPointerUp cleanup: call
e.currentTarget.releasePointerCapture(e.pointerId) if needed, clear
startPosRef.current, call onDragEnd() (and ignore its wasDrag result here),
reset isDraggingRef.current = false, and avoid calling onClick(); ensure these
handlers use the same refs and callbacks (startPosRef, isDraggingRef, onDragEnd)
as onPointerUp so dragIdRef in usePigMovement is always cleared.
- Around line 49-80: Restore native keyboard activation by adding a real onClick
handler to the button and suppressing it only when the preceding pointer
interaction was a drag: introduce a suppressClickRef (useRef<boolean>(false)),
set suppressClickRef.current = wasDrag inside the existing onPointerUp after
calling onDragEnd(), and add onClick={(e) => { if (suppressClickRef.current) {
suppressClickRef.current = false; e.preventDefault(); return; } onClick(); }} to
the button; keep the existing isDraggingRef, startPosRef,
onDragMove/onDragStart, and DRAG_THRESHOLD logic intact.
In `@src/hooks/usePigMovement.ts`:
- Around line 196-215: startDrag currently stores the raw pointer position;
change it to record the initial pointer-to-pig offset (store pointer.x - pig.x
and pointer.y - pig.y in dragStartRef) so the pig doesn't snap when dragging
begins. In moveDrag, compute the new pig position by subtracting that stored
offset from the pointer coordinates, then clamp the computed x and y to the
valid range [0, screenWidth - PIG_SIZE] and [0, screenHeight - PIG_SIZE] before
calling setPigs. Update references to dragIdRef, dragStartRef, moveDrag,
startDrag, setPigs and pigsRef accordingly so pointerHistory logic and
velocity/direction (direction4(p.vx, p.vy)) remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c78de0dc-bab1-495a-ab2a-d74e655d5cfc
📒 Files selected for processing (6)
src/components/App.test.tsxsrc/components/App.tsxsrc/components/PigDetail.tsxsrc/components/PigSprite.tsxsrc/hooks/usePigMovement.test.tssrc/hooks/usePigMovement.ts
Comment on lines
+34
to
+41
| async function handleAddTask(text: string) { | ||
| if (!selectedFocus) return; | ||
| try { | ||
| await focusWriter.appendTask(selectedFocus.id, text); | ||
| } catch { | ||
| // focusWriter already logs the typed error | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Move the new task mutation out of the component.
This adds another direct write call inside src/components, which keeps business I/O in the view layer instead of a hook/API boundary. Please push appendTask behind a hook/action and pass the resulting callback into the component.
As per coding guidelines, "Frontend React components in src/components/ should be view-only with no fetch and no direct I/O."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/App.tsx` around lines 34 - 41, The component-level function
handleAddTask is performing business I/O by calling
focusWriter.appendTask(selectedFocus.id, text); move this mutation behind a hook
or action (e.g., create a useFocusActions hook with an appendTask method) so the
view remains I/O-free: implement a new hook function
(useFocusActions.appendTask) that accepts focusId and text, calls
focusWriter.appendTask and handles/logs errors, then have the component receive
that callback (either via the hook or as a prop) and replace direct calls to
focusWriter.appendTask in handleAddTask with invoking the provided
appendTask(selectedFocus.id, text); keep selectedFocus only as a view-level
prop/state and ensure error handling stays inside the hook.
| readonly viewportH: number; | ||
| readonly onClose: () => void; | ||
| readonly onClearTask: (index: number) => void; | ||
| readonly onAddTask: (text: string) => void; |
There was a problem hiding this comment.
Don't clear the draft until the append result is known.
onAddTask is typed as fire-and-forget, but the input is cleared immediately after it runs. In src/components/App.tsx:34-40, the new append path catches failures, so a failed write drops the user's draft with no recovery path. Make this callback report success/failure (or return a rejecting promise) and only clear taskInput on success.
Also applies to: 74-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/PigDetail.tsx` at line 13, The onAddTask callback in PigDetail
is currently fire-and-forget and the component clears taskInput immediately;
change the onAddTask signature to return a Promise (resolve on success / reject
or false on failure) and update the caller (the append handler in App.tsx) to
return that promise/result so callers know success/failure; in PigDetail await
the returned promise from onAddTask and only clear taskInput when it resolves
successfully (handle rejection by leaving the draft intact and surfacing an
error), and apply the same change for the other occurrence referenced around the
second block (lines 74-85) so both code paths wait for append success before
clearing the input.
Comment on lines
49
to
+80
| <button | ||
| type="button" | ||
| className="pig-sprite" | ||
| style={{ left: x, top: y + bob }} | ||
| onClick={onClick} | ||
| style={{ | ||
| left: x, | ||
| top: y + bob, | ||
| cursor: isDraggingRef.current ? "grabbing" : "pointer", | ||
| }} | ||
| onPointerDown={(e) => { | ||
| e.currentTarget.setPointerCapture(e.pointerId); | ||
| startPosRef.current = { x: e.clientX, y: e.clientY }; | ||
| isDraggingRef.current = false; | ||
| onDragStart(e.clientX, e.clientY); | ||
| }} | ||
| onPointerMove={(e) => { | ||
| if (!startPosRef.current) return; | ||
| const dx = e.clientX - startPosRef.current.x; | ||
| const dy = e.clientY - startPosRef.current.y; | ||
| if (!isDraggingRef.current && Math.sqrt(dx * dx + dy * dy) >= DRAG_THRESHOLD) { | ||
| isDraggingRef.current = true; | ||
| } | ||
| if (isDraggingRef.current) { | ||
| onDragMove(e.clientX, e.clientY); | ||
| } | ||
| }} | ||
| onPointerUp={(e) => { | ||
| e.currentTarget.releasePointerCapture(e.pointerId); | ||
| startPosRef.current = null; | ||
| const { wasDrag } = onDragEnd(); | ||
| isDraggingRef.current = false; | ||
| if (!wasDrag) onClick(); | ||
| }} |
There was a problem hiding this comment.
Restore native keyboard activation for the button.
Removing the button's onClick means Enter/Space no longer opens PigDetail; the only open path now lives in onPointerUp, which never runs for keyboard users. Keep a real click handler for non-pointer activation and suppress it only after an actual drag.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/PigSprite.tsx` around lines 49 - 80, Restore native keyboard
activation by adding a real onClick handler to the button and suppressing it
only when the preceding pointer interaction was a drag: introduce a
suppressClickRef (useRef<boolean>(false)), set suppressClickRef.current =
wasDrag inside the existing onPointerUp after calling onDragEnd(), and add
onClick={(e) => { if (suppressClickRef.current) { suppressClickRef.current =
false; e.preventDefault(); return; } onClick(); }} to the button; keep the
existing isDraggingRef, startPosRef, onDragMove/onDragStart, and DRAG_THRESHOLD
logic intact.
Comment on lines
+57
to
+80
| onPointerDown={(e) => { | ||
| e.currentTarget.setPointerCapture(e.pointerId); | ||
| startPosRef.current = { x: e.clientX, y: e.clientY }; | ||
| isDraggingRef.current = false; | ||
| onDragStart(e.clientX, e.clientY); | ||
| }} | ||
| onPointerMove={(e) => { | ||
| if (!startPosRef.current) return; | ||
| const dx = e.clientX - startPosRef.current.x; | ||
| const dy = e.clientY - startPosRef.current.y; | ||
| if (!isDraggingRef.current && Math.sqrt(dx * dx + dy * dy) >= DRAG_THRESHOLD) { | ||
| isDraggingRef.current = true; | ||
| } | ||
| if (isDraggingRef.current) { | ||
| onDragMove(e.clientX, e.clientY); | ||
| } | ||
| }} | ||
| onPointerUp={(e) => { | ||
| e.currentTarget.releasePointerCapture(e.pointerId); | ||
| startPosRef.current = null; | ||
| const { wasDrag } = onDragEnd(); | ||
| isDraggingRef.current = false; | ||
| if (!wasDrag) onClick(); | ||
| }} |
There was a problem hiding this comment.
Handle canceled pointer captures.
There is no onPointerCancel / onLostPointerCapture cleanup path here. If the gesture is interrupted, onDragEnd() never runs, and dragIdRef in src/hooks/usePigMovement.ts:218-246 stays set, leaving that pig frozen until another interaction resets the drag state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/PigSprite.tsx` around lines 57 - 80, The pointer handlers lack
cleanup for canceled/losing pointer captures causing onDragEnd not to run; add
handlers for onPointerCancel and onLostPointerCapture that mirror the
onPointerUp cleanup: call e.currentTarget.releasePointerCapture(e.pointerId) if
needed, clear startPosRef.current, call onDragEnd() (and ignore its wasDrag
result here), reset isDraggingRef.current = false, and avoid calling onClick();
ensure these handlers use the same refs and callbacks (startPosRef,
isDraggingRef, onDragEnd) as onPointerUp so dragIdRef in usePigMovement is
always cleared.
Comment on lines
+196
to
+215
| const startDrag = useCallback((pigId: string, x: number, y: number) => { | ||
| dragIdRef.current = pigId; | ||
| dragStartRef.current = { x, y }; | ||
| pointerHistoryRef.current = [{ x, y, t: performance.now() }]; | ||
| }, []); | ||
|
|
||
| const moveDrag = useCallback((x: number, y: number) => { | ||
| if (!dragIdRef.current) return; | ||
| const now = performance.now(); | ||
| pointerHistoryRef.current.push({ x, y, t: now }); | ||
| // Keep only last 200ms of history to bound memory. | ||
| pointerHistoryRef.current = pointerHistoryRef.current.filter((s) => s.t >= now - 200); | ||
|
|
||
| setPigs((prev) => { | ||
| const next = prev.map((p) => | ||
| p.id === dragIdRef.current ? { ...p, x, y, direction: direction4(p.vx, p.vy) } : p, | ||
| ); | ||
| pigsRef.current = next; | ||
| return next; | ||
| }); |
There was a problem hiding this comment.
Preserve the grab offset and clamp the dragged position.
moveDrag() writes raw clientX/clientY into p.x/p.y, so the pig snaps its top-left to the cursor as soon as dragging starts and can be dragged partially past the right/bottom edges. Record the initial pointer-to-pig offset in startDrag() and clamp the computed position to [0, screen - PIG_SIZE] before updating state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/usePigMovement.ts` around lines 196 - 215, startDrag currently
stores the raw pointer position; change it to record the initial pointer-to-pig
offset (store pointer.x - pig.x and pointer.y - pig.y in dragStartRef) so the
pig doesn't snap when dragging begins. In moveDrag, compute the new pig position
by subtracting that stored offset from the pointer coordinates, then clamp the
computed x and y to the valid range [0, screenWidth - PIG_SIZE] and [0,
screenHeight - PIG_SIZE] before calling setPigs. Update references to dragIdRef,
dragStartRef, moveDrag, startDrag, setPigs and pigsRef accordingly so
pointerHistory logic and velocity/direction (direction4(p.vx, p.vy)) remain
unchanged.
Extract computeTossVelocity as pure function (testable). Add
DRAG_THRESHOLD, TOSS_VELOCITY_WINDOW_MS, FRICTION constants.
usePigMovement returns {pigs, startDrag, moveDrag, endDrag};
dragged pig position follows pointer, pointer history tracked for
velocity on release. tickPig applies FRICTION each frame; speed
cap raised to PIG_SPEED*6 for post-toss deceleration. PigSprite
uses pointer capture for drag; click fires only when movement
is below DRAG_THRESHOLD.
archae0pteryx
force-pushed
the
feat/020-drag-toss
branch
from
May 3, 2026 15:39
d5a1555 to
7762966
Compare
This was referenced May 3, 2026
This was referenced May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
computeTossVelocity(samples, windowMs, now)as pure function; velocity capped atPIG_SPEED * 6DRAG_THRESHOLD = 4,TOSS_VELOCITY_WINDOW_MS = 80,FRICTION = 0.97,PIG_SPEEDusePigMovementreturn type changed fromPigState[]to{ pigs, startDrag, moveDrag, endDrag }tickPigappliesFRICTIONeach frame; speed cap raised toPIG_SPEED * 6PigSpriteusessetPointerCapturefor reliable drag; click fires only when movement <DRAG_THRESHOLDTest plan
DRAG_THRESHOLDvalue,computeTossVelocitywith constant speed, window filtering, empty/single-sample edge cases, velocity clamping (scalar + diagonal)task checkgreenCloses #18
Summary by CodeRabbit
Release Notes