feat: 024 — display subsystem refactor + coordinate fix [WIP] - #27
Merged
Conversation
…or pig spawn
Replaces app/overlay_manager.rs + app/pig_hittest.rs with display/ module tree:
display/monitor.rs — LogicalMonitor, compute_span, disambiguate_names (6 unit tests)
display/hit_test.rs — PigHitTester (moved)
display/overlay.rs — window lifecycle; uses builder inner_size/position so WKWebView
initialises at correct dimensions (was being reset to 800×600)
display/mod.rs — DisplayManager, PrimaryRegion, drag_active AtomicBool
Key fixes:
- Window builder inner_size/position instead of post-build set_size (macOS override bug)
- Emits display-region event; React confines pig spawn to primary display CSS region
- drag_active flag in hit-test thread keeps overlay interactive across monitor boundary
- PIG_SPEED raised, minimum velocity floor so pigs never appear frozen
- Dead-zone boundary: effectiveMaxY caps pig y at primary display height in main x-range
- Confirm-delete removed (issue 027 tracks setting); Gather Pigs tray item added
- New-focus window: dark opaque background + readable inputs
STATUS: WIP — cross-monitor drag and boundary behaviour still needs work.
Pigs visible on single display; cross-monitor roaming partially functional.
📝 WalkthroughWalkthroughReplaces the old overlay manager with a new display subsystem: monitors normalized to logical pixels, DisplayManager manages overlay windows and per-overlay hit-test threads with a drag-active flag exposed via a Tauri command, region-aware pig spawning/movement and drag-and-toss physics added, tray and UI wiring updated, and unit tests/docs included. ChangesDisplay subsystem, overlay lifecycle, and drag/toss integration
Sequence Diagram(s)sequenceDiagram
participant JS as App (JS)
participant Tauri as Tauri IPC
participant Rust as DisplayManager/Overlay
participant HitTest as Hit-Test Thread
Note over Rust: Startup
Rust->>Rust: enumerate monitors → LogicalMonitor
Rust->>Rust: disambiguate_names
Rust->>Rust: DisplayManager::apply → compute span & PrimaryRegion
Rust-->>JS: emit "display-region" (PrimaryRegion)
Note over JS,Rust: Drag start
JS->>JS: PigSprite.onPointerDown
JS->>Tauri: invoke("set_pig_drag_active",{active:true})
Tauri->>Rust: set DragLockState = true
Rust->>HitTest: hit-test thread sees drag_active → force interactive
JS->>JS: startDrag → widen hit-rects
Note over JS,Rust: Drag move & release
loop pointermove
JS->>JS: moveDrag (follow cursor, record deltas)
end
JS->>Tauri: invoke("set_pig_drag_active",{active:false})
Tauri->>Rust: set DragLockState = false
JS->>JS: endDrag → compute toss velocity, inject into tickPig, restore hit-rects
Note over JS: Continuous physics
loop rAF
JS->>JS: tickPig(primaryRegion): apply friction floor, clamp to region bounds, bounce edges
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
archae0pteryx
marked this pull request as ready for review
May 3, 2026 19:41
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hooks/usePigMovement.ts (2)
1-2: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMove the Tauri call behind an
api/client.This hook now owns both state/effects and transport details. Wrapping
update_pig_rectsinsrc/api/keeps the hook easier to test and matches the repo's boundary rule for hooks. As per coding guidelines, "Frontend hooks insrc/hooks/should handle state + effects and callapi/clients."Also applies to: 213-219
238-251:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't spawn pigs until the real display region is available.
regionRefstarts asdefaultRegion(), and Line 334 uses that value immediately forinitPig(...). The"display-region"event fromsrc/components/App.tsx:34-42arrives later andsetRegiononly updates the ref, so pigs created during startup or overlay recreation can still spawn in the wrong area and stay there.Also applies to: 327-338
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/usePigMovement.ts` around lines 238 - 251, regionRef is initialized with defaultRegion() so initPig(...) can run before the real "display-region" arrives; change regionRef to start as null (useRef<SpawnRegion | null>(null)) and make setRegion assign the real region and mark it ready (e.g., regionRef.current = r). Update all places that call initPig or spawn pigs (the code that currently reads regionRef.current) to guard: if regionRef.current is null (or equals the defaultRegion sentinel) then defer or skip spawning until setRegion runs. Alternatively add a regionReadyRef flag set in setRegion and check regionReadyRef.current before calling initPig; ensure functions like initPig, any spawn loops, and the rAF startup path check this guard so pigs are not created with the placeholder region.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 9-45: The changelog headings (e.g., "### Added — 024 display
subsystem (WIP, PR `#27` — cross-monitor drag still broken)", "### Changed — 024",
"### Added" under [1.2.1], etc.) lack the required blank line after the heading
which triggers markdownlint MD022; fix this by inserting a single empty line
immediately after each "### ..." heading so each heading is followed by a blank
line before the subsequent list or paragraph, ensuring every heading (such as
the Added/Changed sections and the "[1.2.1] — 2026-05-03 — Phase 3 polish"
heading) conforms to MD022.
In `@CONTEXT.md`:
- Line 80: Step 7 ("Delete a Focus. *(015)*") in CONTEXT.md is outdated: it
mentions a native confirmation but the app now performs an immediate delete from
the tray via the delete_focus action. Update the step text to reflect the
current immediate-delete behavior (Menu bar item → Focus submenu → "Delete…" →
delete_focus → pig disappears) or alternatively append a parenthetical noting
confirmation is deferred/optional and tracked by issue `#027`; ensure you update
the "Delete a Focus" line and any mention of "native confirmation" so
documentation matches current behavior.
In `@issues/024-display-subsystem-agent-prompt.md`:
- Around line 34-40: The markdown has lint failures MD040, MD031, and MD029:
edit the fenced code block so it specifies a language (use ```rust), ensure
there is a blank line before and after that fenced block, and normalize the
ordered-list numbering so items use consistent numbering (e.g., all "1." or
incrementing numbers) to satisfy MD029; apply the same fixes to the other
occurrences referenced (lines noted as 64-67 and 94-95) so all fenced Rust
blocks have language and surrounding blank lines and all ordered lists are
normalized.
- Line 3: In issues/024-display-subsystem-agent-prompt.md replace the
author-specific absolute path text with a repo-relative reference or the phrase
“the repository root”; locate the hard-coded path occurrence in that file and
change it to something portable (e.g., “the repository root” or a relative path
like ./) so the documentation no longer contains a user-specific absolute path.
In `@issues/024-display-subsystem.md`:
- Around line 47-53: The fenced code block that lists display module files (the
block containing lines like "mod.rs — DisplayManager", "monitor.rs — pure types
+ math", "overlay.rs — window lifecycle", and "hit_test.rs — PigHitTester") is
missing a language tag and may fail markdown lint; fix it by adding a language
identifier (e.g., "text") immediately after the opening triple backticks of that
fenced block in issues/024-display-subsystem.md so the block becomes ```text ...
``` to satisfy the linter.
In `@issues/done/016-real-sprite-sheet.md`:
- Around line 14-16: The documented sprite-sheet row order in
issues/done/016-real-sprite-sheet.md is out of sync with the implementation in
PigSprite.tsx; update the doc to match the component's mapping by stating the
default row order as front=0, right=1, back=2, left=3 and keep the frame sizing
rules (frame width = sheet_width / 4, frame height = sheet_height / 4) so future
sprite-sheet changes use the same mapping as the PigSprite.tsx rendering logic.
In `@issues/done/018-larger-pig-hitbox.md`:
- Line 15: The MD037 lint error is caused by using emphasis (**) around inline
code spans in the bullet; remove the surrounding bold markers so the code spans
remain plain inline code (e.g., change **`src/hooks/usePigMovement.ts`** to
`src/hooks/usePigMovement.ts`) and do the same for any other `...` spans in that
bullet; keep the suggested content (export const HITBOX_PADDING = 16 and the
syncRects changes referencing PIG_SIZE and dpr) but ensure only backticks are
used for code spans and no surrounding emphasis.
In `@src-tauri/src/display/mod.rs`:
- Around line 32-64: DisplayManager is exposed as a concrete type but callers
should depend on a trait to allow testing and swapping implementations;
introduce a trait (e.g. DisplayService or DisplayManagerTrait) that declares the
public API methods presently on DisplayManager (new, drag_active, apply, etc.),
implement that trait for the existing DisplayManager struct, and update module
boundaries/call sites to accept &dyn DisplayService or Arc<dyn DisplayService +
Send + Sync> (or a generic bound) instead of the concrete DisplayManager; ensure
DisplayManagerState and any functions that construct or store the manager use
the trait object or generic trait bound so tests can inject mocks and the
concrete Tauri-backed DisplayManager remains an implementation detail.
- Around line 65-70: The filter is using enumerate() position instead of each
monitor's stable identity; update the selection to check each LogicalMonitor's
index field against config.enabled_indices (i.e., replace the enumerate/filter
by iterating monitors and keeping those where
config.enabled_indices.contains(&m.index) or equivalent), ensuring you compare
the monitor's LogicalMonitor.index with enabled_indices when building enabled
(referencing monitors, enabled, config.enabled_indices, and
LogicalMonitor.index).
In `@src-tauri/src/display/monitor.rs`:
- Around line 150-188: The tests for compute_span assume portrait monitors
always sit at y = 0 and miss layouts with negative y; add a new unit test (e.g.,
compute_span_portrait_above_landscape) that creates a portrait monitor with a
negative y coordinate (above the landscape primary) and asserts the expected
span.x, span.y, span.width, and span.height to cover min_y/max_y logic; update
or reuse mon(...) test helpers used in compute_span_portrait_left_of_landscape
to place the portrait at a negative y and verify compute_span handles vertical
offsets correctly.
In `@src-tauri/src/display/overlay.rs`:
- Around line 90-119: The poller thread for overlay-0 can continue running
across a destroy/recreate because it only checks
get_webview_window(OVERLAY_LABEL); fix by giving the poller a stable stop
condition and ensuring the old thread is signalled (and can exit) before
recreating the window: capture a unique identifier when spawning (e.g.,
win_clone.id() or a generation token) and have the loop also compare that
id/token each iteration (or use a shared AtomicBool/Cancellation flag that the
creator sets before recreating), referencing win_clone, tester_thread,
drag_active, get_webview_window, and set_ignore_cursor_events so the thread
reliably exits instead of racing with a newly spawned poller.
In `@src/components/App.tsx`:
- Around line 26-42: Both useEffect hooks leak subscriptions if the component
unmounts before listen(...) resolves; replace the current "cleanup" mutable
variable approach by capturing the Promise returned from listen (for
"gather-pigs" and for "display-region") and return a cleanup that calls unlisten
inside a .then() (e.g., const unlistenPromise = listen(...); return () =>
unlistenPromise.then(unlisten => unlisten());) so unlisten is deferred until the
Promise resolves; apply this change to the effect using gather and the effect
using setRegion/SpawnRegion.
In `@src/components/PigSprite.tsx`:
- Around line 1-9: The file currently performs platform I/O via invoke(...) in
the helper setDragActive used by the PigSprite component; move that I/O out of
src/components by removing invoke from this file and instead accept a prop or
external hook that performs the call. Specifically, delete or replace the local
setDragActive(invoke...) and have PigSprite consume a prop like
onSetDragActive(active: boolean) or call a hook provided from outside (e.g.,
usePigDragController) so that PigSprite remains presentational; implement the
actual invoke(...) implementation in the parent/container or the hook and pass
the function down to PigSprite.
- Around line 95-102: The onLostPointerCapture handler currently only clears
drag state when isDraggingRef.current is true, which can leave the Rust-side
drag_active flag set if pointer capture is lost before threshold; update the
onLostPointerCapture callback to always call setDragActive(false), clear
startPosRef.current, call onDragEnd(), and set isDraggingRef.current = false
regardless of the current isDraggingRef value so the overlay and Rust flag are
always reset. Ensure you update the handler that references isDraggingRef,
setDragActive, startPosRef, and onDragEnd to perform these actions
unconditionally.
In `@src/hooks/usePigMovement.ts`:
- Around line 279-293: The gather() implementation places pigs in a single
vertical column and lets y grow unbounded so pigs fall off short displays;
update gather (in usePigMovement) to clamp/wrap positions inside
regionRef.current bounds: compute availableHeight = r.h - 2*margin, determine
pigsPerColumn = Math.max(1, Math.floor(availableHeight / (PIG_SIZE + 24))), then
for each pig index i compute column = Math.floor(i / pigsPerColumn) and row = i
% pigsPerColumn and set x = r.x + r.w - margin - PIG_SIZE - column*(PIG_SIZE +
spacing) and y = r.y + margin + row*(PIG_SIZE + 24); update pigsRef.current and
setPigs with these bounded coordinates and reset vx/vy.
In `@src/styles.css`:
- Around line 226-230: The .new-focus-form styles currently include
window-scoped rules (e.g., min-height: 100vh and the window chrome styles) that
affect all uses of the component; add a dedicated modifier class (e.g.,
new-focus-form--window) and move the viewport/full-window rules (min-height:
100vh, background, border-radius, padding, box-sizing) under
.new-focus-form.new-focus-form--window in the stylesheet, then update the window
instance of the component in NewFocusForm (apply className "new-focus-form
new-focus-form--window" only where a full-window layout is intended) so other
usages keep their normal flow.
---
Outside diff comments:
In `@src/hooks/usePigMovement.ts`:
- Around line 238-251: regionRef is initialized with defaultRegion() so
initPig(...) can run before the real "display-region" arrives; change regionRef
to start as null (useRef<SpawnRegion | null>(null)) and make setRegion assign
the real region and mark it ready (e.g., regionRef.current = r). Update all
places that call initPig or spawn pigs (the code that currently reads
regionRef.current) to guard: if regionRef.current is null (or equals the
defaultRegion sentinel) then defer or skip spawning until setRegion runs.
Alternatively add a regionReadyRef flag set in setRegion and check
regionReadyRef.current before calling initPig; ensure functions like initPig,
any spawn loops, and the rAF startup path check this guard so pigs are not
created with the placeholder region.
🪄 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: 288b7f4c-1ee1-4e55-a8ed-93f91abbb426
📒 Files selected for processing (25)
CHANGELOG.mdCONTEXT.mdPRD.mdissues/024-display-subsystem-agent-prompt.mdissues/024-display-subsystem.mdissues/027-confirm-delete-setting.mdissues/done/013-tray-icon-focus-list.mdissues/done/016-real-sprite-sheet.mdissues/done/018-larger-pig-hitbox.mdissues/done/019-pigdetail-redesign.mdissues/done/020-drag-and-toss.mdsrc-tauri/src/app/mod.rssrc-tauri/src/app/overlay_manager.rssrc-tauri/src/app/tray.rssrc-tauri/src/display/hit_test.rssrc-tauri/src/display/mod.rssrc-tauri/src/display/monitor.rssrc-tauri/src/display/overlay.rssrc-tauri/src/lib.rssrc-tauri/src/ui_bridge/mod.rssrc/components/App.test.tsxsrc/components/App.tsxsrc/components/PigSprite.tsxsrc/hooks/usePigMovement.tssrc/styles.css
💤 Files with no reviewable changes (1)
- src-tauri/src/app/overlay_manager.rs
Comment on lines
+226
to
+230
| background: rgba(22, 22, 26, 0.97); | ||
| border-radius: 12px; | ||
| padding: 20px; | ||
| min-height: 100vh; | ||
| box-sizing: border-box; |
There was a problem hiding this comment.
Scope full-window layout styles to a window-specific class.
Line 229 (min-height: 100vh) and the window chrome styles are attached to .new-focus-form, but this class is reused by src/components/NewFocusForm.tsx (Line 47-86). That can force non-window usages into viewport-height layout and break surrounding UI flow.
Suggested fix
.new-focus-form {
display: flex;
flex-direction: column;
gap: 10px;
margin: 0;
+}
+
+.new-focus-form--window {
background: rgba(22, 22, 26, 0.97);
border-radius: 12px;
padding: 20px;
min-height: 100vh;
box-sizing: border-box;
color: `#f5f5f7`;
}// apply only in dedicated new-focus window component
<form className="new-focus-form new-focus-form--window" ...>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/styles.css` around lines 226 - 230, The .new-focus-form styles currently
include window-scoped rules (e.g., min-height: 100vh and the window chrome
styles) that affect all uses of the component; add a dedicated modifier class
(e.g., new-focus-form--window) and move the viewport/full-window rules
(min-height: 100vh, background, border-radius, padding, box-sizing) under
.new-focus-form.new-focus-form--window in the stylesheet, then update the window
instance of the component in NewFocusForm (apply className "new-focus-form
new-focus-form--window" only where a full-window layout is intended) so other
usages keep their normal flow.
Bugs fixed: - display/mod.rs: filter enabled monitors by LogicalMonitor.index, not enumerate position — fixes wrong monitors selected on display toggle - PigSprite: onLostPointerCapture guards on startPosRef (not isDraggingRef) so drag_active flag clears even when capture lost before threshold - App.tsx: use Promise-chaining cleanup for Tauri listeners to prevent subscription leak on early unmount - usePigMovement: gather() wraps into columns when pigs exceed display height - styles.css: extract .new-focus-form--window modifier for viewport-height styles; base class no longer forces min-height: 100vh on all usages Architecture: - display/mod.rs: introduce DisplayService trait (drag_active + apply); DisplayManagerState now holds Arc<dyn DisplayService> - display/overlay.rs + tray.rs + app/mod.rs: concrete Wry runtime throughout (was generic <R: Runtime>) - overlay.rs: add Arc<AtomicBool> stop flag to OverlayEntry; signal stop before removing entry to prevent dual-poller race on window recreate - PigSprite: remove invoke() from view component; I/O moved to App.tsx hook Tests: - display/monitor.rs: add compute_span_portrait_above_landscape (negative-y) Docs: - CHANGELOG.md: fix MD022 blank lines after headings - CONTEXT.md: fix step 7 delete flow (was: native confirmation; now: immediate) - issues/024-*: fix MD040/MD031/MD029, remove absolute path - issues/done/016: update sprite row order to match PigSprite.tsx (front=0) - issues/done/018: fix MD037 emphasis around inline code
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
issues/done/016-real-sprite-sheet.md (1)
34-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the stale 2-row mapping note.
This bullet still says only
left=1, right=2, which now conflicts with the row order above and withsrc/components/PigSprite.tsx’s actualfront/right/back/leftmapping.Suggested doc fix
- - `row` derived from `direction`: `left=1, right=2` (up/down unused in v1.2 — pigs only move in 2D horizontal plane for now). + - `row` derived from `direction`: `front=0, right=1, back=2, left=3` (matches `PigSprite.tsx` `DIRECTION_ROW`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@issues/done/016-real-sprite-sheet.md` around lines 34 - 35, Update the documentation note that says "row derived from direction: left=1, right=2" so it matches the actual sprite-row ordering used in the code (the front/right/back/left mapping implemented in src/components/PigSprite.tsx) and remove the outdated claim that up/down are unused; ensure the bullet reflects the full front/right/back/left row mapping and that direction is handled by row selection (not a scaleX(-1) transform) to avoid the conflict with PigSprite.tsx's mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 48-53: Update the "Changed" changelog line for usePigMovement so
the return shape matches the shipped API: replace the current description that
lists only `{ pigs, startDrag, moveDrag, endDrag }` with the full returned
object including `gather` and `setRegion` (e.g. `{ pigs, startDrag, moveDrag,
endDrag, gather, setRegion }`) and ensure wording and punctuation follow the
surrounding entries for consistency.
In `@issues/024-display-subsystem.md`:
- Around line 45-52: Update the issue doc to match the actual landed display
module tree and coordinate behavior: replace the claim that the polling thread
lives in display/hit_test.rs with that it lives in display/overlay.rs (where the
poller is implemented), update the file list to show mod.rs, monitor.rs,
overlay.rs, and hit_test.rs as implemented (with PigHitTester in hit_test.rs and
the poller in overlay.rs), and remove/replace the note that outer_position()
needs logical conversion—document that both cursor_position() and
outer_position() are treated as physical pixels in the poller; also apply the
same corrections to lines referenced at 76–79.
In `@issues/done/018-larger-pig-hitbox.md`:
- Around line 13-17: The "done" note is outdated and refers to code locations
and formatting that don't match the shipped implementation: replace the escaped
inline backticks and correct the references to where the hitbox math actually
lives. Update the note to mention the exported HITBOX_PADDING constant in
src/hooks/usePigMovement.ts and state that buildHitRects(...) (not syncRects)
applies the math to produce rects sent to Rust via update_pig_rects; explain the
centered hitbox math (use size = (PIG_SIZE + HITBOX_PADDING) * dpr and offset
x/y by -(HITBOX_PADDING/2) * dpr) and confirm PigSprite click behavior remains
unchanged and PigHitTester::is_hit requires no Rust changes.
In `@src-tauri/src/display/monitor.rs`:
- Around line 69-79: The disambiguation currently truncates fractional logical
positions by casting position components to i64 in disambiguate_names, which can
collapse distinct fractional offsets into identical "(x, y)" suffixes; update
disambiguate_names to preserve fractional coordinates by formatting the position
tuple's f64 components (e.g., using a fixed precision like "{:.2}" or full float
formatting) instead of casting to i64, referencing the monitors array, labels
vector, and monitors[i].position so duplicate labels become unique with precise
coordinates.
In `@src-tauri/src/display/overlay.rs`:
- Around line 85-87: The current code emits the "display-region" event from
overlay.rs using window.emit(...) during the Tauri .setup() backend
initialization (so primary_region is sent before the React listener in App.tsx
attaches and is lost); change to either emit only after the frontend signals
readiness (implement a ready handshake: listen for a "frontend-ready" event in
Rust and call window.emit("display-region", primary_region.clone()) then) or
remove the early emit and implement a pull-based command (expose a
get_display_region command that returns primary_region to be called from App.tsx
on startup); update any references to window.emit("display-region", ...) and
ensure App.tsx calls get_display_region or sends "frontend-ready" before relying
on the region.
- Around line 111-118: cursor_position() and outer_position() return physical
pixels while PigHitTester expects logical/CSS coordinates, so the hit test in
tester_thread.is_hit is using mismatched units; convert cursor and window origin
to logical coordinates by dividing both cursor coordinates and
outer_position()'s x/y by the window's scale factor (use
win_clone.scale_factor().unwrap_or(1.0)) before computing local_x and local_y
and then call tester_thread.is_hit(local_x, local_y) with those logical values.
In `@src/components/App.tsx`:
- Around line 27-45: The App component currently performs platform I/O via
handleSetDragActive (invoke("set_pig_drag_active", ...)) and two listen(...)
subscriptions ("gather-pigs" and "display-region"); move that logic into a
dedicated hook or service (e.g., useTauriBridge or tauriBridge) outside
src/components, expose functions like setPigDragActive(active: boolean) and
subscription APIs like onGather(callback) and onDisplayRegion(callback) that
return cleanup functions, and update App to be purely presentational by
receiving the callbacks/state (e.g., onSetDragActive, region, onGather) as
props; ensure the new hook handles invoking, listen generic typing
(SpawnRegion), and unlisten cleanup so App no longer imports invoke or listen or
calls setRegion/gather directly.
In `@src/hooks/usePigMovement.ts`:
- Around line 249-251: The initial spawn uses defaultRegion() and never updates
existing pigs when the real display region arrives because setRegion only
mutates regionRef; fix by gating the initial spawn on receiving the first real
region payload (the "display-region" event) or by making setRegion (the
useCallback that updates regionRef.current) trigger a one-time respawn/reclamp
of existing pigs when it receives the first non-default region; update the
focuses effect (and the logic referenced around lines 331-341) to either wait
for a boolean like hasReceivedRegion before seeding pigs or call the
respawn/reclamp routine from setRegion when first invoked to move any off-screen
pigs into the primary region.
- Around line 279-296: The gather() callback teleports pigs by updating React
state and pigsRef.current but does not update the Rust hit tester, causing stale
hit rects; after computing next and assigning pigsRef.current (inside gather),
invoke the existing hit-rect synchronization routine (the same function you use
in the periodic movement loop—e.g., syncHitRects / updateRustHitTester /
sendHitRectsToRust) to push the new rectangles immediately to Rust so the hit
tester matches the new positions; if no single helper exists, reuse the
movement-loop logic that serializes pigsRef.current and sends it to Rust and
call that from gather after setPigs/pigsRef.current update.
---
Duplicate comments:
In `@issues/done/016-real-sprite-sheet.md`:
- Around line 34-35: Update the documentation note that says "row derived from
direction: left=1, right=2" so it matches the actual sprite-row ordering used in
the code (the front/right/back/left mapping implemented in
src/components/PigSprite.tsx) and remove the outdated claim that up/down are
unused; ensure the bullet reflects the full front/right/back/left row mapping
and that direction is handled by row selection (not a scaleX(-1) transform) to
avoid the conflict with PigSprite.tsx's mapping.
🪄 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: 3ab5aa38-9a2d-455c-a077-f3edd8366095
📒 Files selected for processing (16)
CHANGELOG.mdCONTEXT.mdissues/024-display-subsystem-agent-prompt.mdissues/024-display-subsystem.mdissues/done/016-real-sprite-sheet.mdissues/done/018-larger-pig-hitbox.mdsrc-tauri/src/app/mod.rssrc-tauri/src/app/tray.rssrc-tauri/src/display/mod.rssrc-tauri/src/display/monitor.rssrc-tauri/src/display/overlay.rssrc/components/App.tsxsrc/components/NewFocusWindow.tsxsrc/components/PigSprite.tsxsrc/hooks/usePigMovement.tssrc/styles.css
Comment on lines
+48
to
+53
| ### Changed | ||
|
|
||
| - `PigDetail` card: fully opaque background, `min-width: 340px`, `padding: 16px`, task list scrolls at `max-height: 240px` | ||
| - `usePigMovement` returns `{ pigs, startDrag, moveDrag, endDrag }` instead of bare `PigState[]` | ||
| - `tickPig` speed cap raised from `PIG_SPEED × 1.2` to `PIG_SPEED × 6` to allow post-toss deceleration | ||
|
|
There was a problem hiding this comment.
The hook return shape in this entry is already stale.
usePigMovement no longer returns only { pigs, startDrag, moveDrag, endDrag }; this PR also adds gather and setRegion. Please update the changelog so the recorded API matches what shipped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` around lines 48 - 53, Update the "Changed" changelog line for
usePigMovement so the return shape matches the shipped API: replace the current
description that lists only `{ pigs, startDrag, moveDrag, endDrag }` with the
full returned object including `gather` and `setRegion` (e.g. `{ pigs,
startDrag, moveDrag, endDrag, gather, setRegion }`) and ensure wording and
punctuation follow the surrounding entries for consistency.
Comment on lines
+45
to
+52
| Replace `app/overlay_manager.rs` + `app/pig_hittest.rs` with a `display/` module tree: | ||
|
|
||
| ```text | ||
| src-tauri/src/display/ | ||
| mod.rs — DisplayManager (public surface; re-exports; impl RectUpdater) | ||
| monitor.rs — pure types + math: LogicalMonitor, compute_span, disambiguate_names | ||
| overlay.rs — window lifecycle: create, resize, show, destroy | ||
| hit_test.rs — PigHitTester + polling thread |
There was a problem hiding this comment.
Refresh this issue doc to match the landed display implementation.
The file list here still says the polling thread lives in src-tauri/src/display/hit_test.rs, and the fix list still describes outer_position() as needing logical conversion before comparing with the cursor. In the current code, src-tauri/src/display/hit_test.rs is just PigHitTester, while the poller lives in src-tauri/src/display/overlay.rs and treats both cursor_position() and outer_position() as physical pixels. Leaving this spec stale will send the remaining drag work after the wrong module and the wrong coordinate assumptions.
Also applies to: 76-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@issues/024-display-subsystem.md` around lines 45 - 52, Update the issue doc
to match the actual landed display module tree and coordinate behavior: replace
the claim that the polling thread lives in display/hit_test.rs with that it
lives in display/overlay.rs (where the poller is implemented), update the file
list to show mod.rs, monitor.rs, overlay.rs, and hit_test.rs as implemented
(with PigHitTester in hit_test.rs and the poller in overlay.rs), and
remove/replace the note that outer_position() needs logical conversion—document
that both cursor_position() and outer_position() are treated as physical pixels
in the poller; also apply the same corrections to lines referenced at 76–79.
Comment on lines
+13
to
+17
| Add a separate `HITBOX_PADDING` constant in \`usePigMovement.ts\` (e.g. 16 px) and use \`PIG_SIZE + HITBOX_PADDING\` when computing the rects sent to Rust via \`update_pig_rects\`. The visual sprite size stays unchanged; only the hit-detection footprint grows. | ||
|
|
||
| - `src/hooks/usePigMovement.ts`: add `export const HITBOX_PADDING = 16`; in `syncRects` use `size: (PIG_SIZE + HITBOX_PADDING) * dpr` and offset `x`/`y` by `-(HITBOX_PADDING / 2) * dpr` so the hitbox is centred on the sprite. | ||
| - No Rust changes needed — \`PigHitTester::is_hit\` already works on whatever rects it receives. | ||
| - \`PigSprite\` click handler already fires on the \`<button>\` element; the button's visible area stays 48 px. The extra hit area is only for the Rust polling thread (click-through toggle), not for the React click event. |
There was a problem hiding this comment.
This “done” note no longer matches the implementation.
The inline code spans are escaped, so they render as literal backticks, and the geometry math now lives in buildHitRects(...), not directly inside syncRects(...) in src/hooks/usePigMovement.ts. Since this issue is marked done, the implementation notes should point at the code that actually shipped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@issues/done/018-larger-pig-hitbox.md` around lines 13 - 17, The "done" note
is outdated and refers to code locations and formatting that don't match the
shipped implementation: replace the escaped inline backticks and correct the
references to where the hitbox math actually lives. Update the note to mention
the exported HITBOX_PADDING constant in src/hooks/usePigMovement.ts and state
that buildHitRects(...) (not syncRects) applies the math to produce rects sent
to Rust via update_pig_rects; explain the centered hitbox math (use size =
(PIG_SIZE + HITBOX_PADDING) * dpr and offset x/y by -(HITBOX_PADDING/2) * dpr)
and confirm PigSprite click behavior remains unchanged and PigHitTester::is_hit
requires no Rust changes.
Comment on lines
+69
to
+79
| pub fn disambiguate_names(monitors: &mut [LogicalMonitor]) { | ||
| let labels: Vec<String> = monitors.iter().map(|m| m.label.clone()).collect(); | ||
| for i in 0..monitors.len() { | ||
| let has_dup = labels | ||
| .iter() | ||
| .enumerate() | ||
| .any(|(j, l)| j != i && *l == labels[i]); | ||
| if has_dup { | ||
| let (x, y) = monitors[i].position; | ||
| monitors[i].label = format!("{} ({}, {})", labels[i], x as i64, y as i64); | ||
| } |
There was a problem hiding this comment.
Preserve fractional coordinates when disambiguating duplicate labels.
LogicalMonitor::from_tauri() stores logical positions as f64, but this suffix truncates them with as i64. Two same-name displays with distinct fractional logical offsets can still collapse to the same "(x, y)" label, which defeats the tray disambiguation this helper is supposed to guarantee.
Possible fix
- monitors[i].label = format!("{} ({}, {})", labels[i], x as i64, y as i64);
+ monitors[i].label = format!("{} ({x:.2}, {y:.2})", labels[i]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub fn disambiguate_names(monitors: &mut [LogicalMonitor]) { | |
| let labels: Vec<String> = monitors.iter().map(|m| m.label.clone()).collect(); | |
| for i in 0..monitors.len() { | |
| let has_dup = labels | |
| .iter() | |
| .enumerate() | |
| .any(|(j, l)| j != i && *l == labels[i]); | |
| if has_dup { | |
| let (x, y) = monitors[i].position; | |
| monitors[i].label = format!("{} ({}, {})", labels[i], x as i64, y as i64); | |
| } | |
| pub fn disambiguate_names(monitors: &mut [LogicalMonitor]) { | |
| let labels: Vec<String> = monitors.iter().map(|m| m.label.clone()).collect(); | |
| for i in 0..monitors.len() { | |
| let has_dup = labels | |
| .iter() | |
| .enumerate() | |
| .any(|(j, l)| j != i && *l == labels[i]); | |
| if has_dup { | |
| let (x, y) = monitors[i].position; | |
| monitors[i].label = format!("{} ({x:.2}, {y:.2})", labels[i]); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/display/monitor.rs` around lines 69 - 79, The disambiguation
currently truncates fractional logical positions by casting position components
to i64 in disambiguate_names, which can collapse distinct fractional offsets
into identical "(x, y)" suffixes; update disambiguate_names to preserve
fractional coordinates by formatting the position tuple's f64 components (e.g.,
using a fixed precision like "{:.2}" or full float formatting) instead of
casting to i64, referencing the monitors array, labels vector, and
monitors[i].position so duplicate labels become unique with precise coordinates.
Comment on lines
+85
to
+87
| // Always emit primary region so React knows where to spawn pigs. | ||
| // Re-emitted on display toggle so React updates the spawn zone. | ||
| let _ = window.emit("display-region", primary_region.clone()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the overlay.rs file to understand the context around lines 85-87
cd src-tauri && cat -n src/display/overlay.rs | head -100Repository: killallgit/adhd-ranch
Length of output: 4198
🏁 Script executed:
# Check App.tsx to see when the listener is set up
cd src && grep -n "display-region" components/App.tsx -B 3 -A 3Repository: killallgit/adhd-ranch
Length of output: 261
🏁 Script executed:
# Search for any other display-region event usage patterns
fd -t rs -e rs . src-tauri && xargs grep -l "display-region" 2>/dev/null | head -20Repository: killallgit/adhd-ranch
Length of output: 235
🏁 Script executed:
# Look for any event listener setup or ready/handshake mechanisms
rg "listen|on_" src/components/App.tsx -A 2 | head -50Repository: killallgit/adhd-ranch
Length of output: 589
🏁 Script executed:
# Find where ensure_shown is called - this tells us the timing
rg "ensure_shown" src-tauri --type rust -B 3 -A 3Repository: killallgit/adhd-ranch
Length of output: 1283
🏁 Script executed:
# Check the main app initialization to see the startup sequence
fd "main.rs" src-tauri && cat -n src-tauri/src/main.rs | head -150Repository: killallgit/adhd-ranch
Length of output: 236
🏁 Script executed:
# Look for any ready/init callbacks or synchronization points
rg "setup|init|ready" src-tauri --type rust | head -20Repository: killallgit/adhd-ranch
Length of output: 1428
🏁 Script executed:
# Check if there's any display-region emission on React mount or ready event
rg "ready|initialized|mount" src/components/App.tsx -B 2 -A 2Repository: killallgit/adhd-ranch
Length of output: 676
🏁 Script executed:
# Get more context on display/mod.rs to see when ensure_shown is actually called
cat -n src-tauri/src/display/mod.rs | head -200Repository: killallgit/adhd-ranch
Length of output: 6119
🏁 Script executed:
# Check if there's any call from app initialization or if it's event-driven
rg "display::" src-tauri/src/app/mod.rs -B 3 -A 3Repository: killallgit/adhd-ranch
Length of output: 661
🏁 Script executed:
# Look for any frontend re-request of display-region or explicit initialization
rg "invoke|command" src/components/App.tsx | head -20Repository: killallgit/adhd-ranch
Length of output: 175
🏁 Script executed:
# Look deeper into app/mod.rs setup handler to see what happens there
cat -n src-tauri/src/app/mod.rs | grep -A 100 "\.setup"Repository: killallgit/adhd-ranch
Length of output: 4734
🏁 Script executed:
# Check if DisplayService.apply is called in setup or elsewhere
rg "\.apply\(" src-tauri --type rust -B 3 -A 3Repository: killallgit/adhd-ranch
Length of output: 1047
🏁 Script executed:
# Look for event listeners or commands that might trigger apply
rg "listen|on_window_event|monitor" src-tauri/src/app/mod.rsRepository: killallgit/adhd-ranch
Length of output: 676
Emit display-region after the frontend is ready, not during backend setup.
This event fires in the Tauri .setup() handler (line 126 in app/mod.rs) before the React listener in App.tsx can attach. Tauri doesn't buffer events, so the first emit is lost. Pigs fall back to the default spawn zone until a display toggle re-emits the event. Use a ready handshake or pull-based get_display_region command instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/display/overlay.rs` around lines 85 - 87, The current code
emits the "display-region" event from overlay.rs using window.emit(...) during
the Tauri .setup() backend initialization (so primary_region is sent before the
React listener in App.tsx attaches and is lost); change to either emit only
after the frontend signals readiness (implement a ready handshake: listen for a
"frontend-ready" event in Rust and call window.emit("display-region",
primary_region.clone()) then) or remove the early emit and implement a
pull-based command (expose a get_display_region command that returns
primary_region to be called from App.tsx on startup); update any references to
window.emit("display-region", ...) and ensure App.tsx calls get_display_region
or sends "frontend-ready" before relying on the region.
Comment on lines
+111
to
+118
| // cursor_position() and outer_position() are both physical pixels. | ||
| let origin = win_clone | ||
| .outer_position() | ||
| .map(|p| (p.x as f64, p.y as f64)) | ||
| .unwrap_or((0.0, 0.0)); | ||
| let local_x = cursor.x - origin.0; | ||
| let local_y = cursor.y - origin.1; | ||
| tester_thread.is_hit(local_x, local_y) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src-tauri/src/display/overlay.rs | head -150Repository: killallgit/adhd-ranch
Length of output: 6341
🏁 Script executed:
find src-tauri -name "*.rs" -type f | xargs grep -l "struct PigHitTester\|impl PigHitTester" | head -5Repository: killallgit/adhd-ranch
Length of output: 99
🏁 Script executed:
cat -n src-tauri/src/display/hit_test.rsRepository: killallgit/adhd-ranch
Length of output: 1094
🏁 Script executed:
find src-tauri -name "*.rs" -type f | xargs grep -l "PigRect" | head -10Repository: killallgit/adhd-ranch
Length of output: 159
🏁 Script executed:
cat -n src-tauri/src/display/mod.rsRepository: killallgit/adhd-ranch
Length of output: 6119
🏁 Script executed:
cat -n src-tauri/src/ui_bridge/mod.rsRepository: killallgit/adhd-ranch
Length of output: 5097
🏁 Script executed:
rg -A 3 "cursor_position\|outer_position" src-tauri/src/display/overlay.rs | head -20Repository: killallgit/adhd-ranch
Length of output: 47
🏁 Script executed:
rg "scale_factor" src-tauri/src --context 3Repository: killallgit/adhd-ranch
Length of output: 2727
🏁 Script executed:
cat -n src-tauri/src/display/monitor.rs | head -80Repository: killallgit/adhd-ranch
Length of output: 3015
Convert cursor and window position to logical coordinates before hit-testing.
cursor_position() and outer_position() return physical pixels, but PigHitTester receives rects from the frontend in logical/CSS coordinates. On scaled displays (DPI > 100%), this coordinate mismatch causes hit-test to fail. Convert both cursor and origin to logical space using window.scale_factor() before calculating local_x/local_y:
let scale_factor = win_clone.scale_factor().unwrap_or(1.0);
let origin = win_clone
.outer_position()
.map(|p| ((p.x as f64 / scale_factor), (p.y as f64 / scale_factor)))
.unwrap_or((0.0, 0.0));
let local_x = (cursor.x / scale_factor) - origin.0;
let local_y = (cursor.y / scale_factor) - origin.1;
tester_thread.is_hit(local_x, local_y)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/display/overlay.rs` around lines 111 - 118, cursor_position()
and outer_position() return physical pixels while PigHitTester expects
logical/CSS coordinates, so the hit test in tester_thread.is_hit is using
mismatched units; convert cursor and window origin to logical coordinates by
dividing both cursor coordinates and outer_position()'s x/y by the window's
scale factor (use win_clone.scale_factor().unwrap_or(1.0)) before computing
local_x and local_y and then call tester_thread.is_hit(local_x, local_y) with
those logical values.
Comment on lines
+27
to
+45
| const handleSetDragActive = useCallback((active: boolean) => { | ||
| invoke("set_pig_drag_active", { active }).catch(() => {}); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| const unlistenPromise = listen("gather-pigs", gather); | ||
| return () => { | ||
| unlistenPromise.then((unlisten) => unlisten()); | ||
| }; | ||
| }, [gather]); | ||
|
|
||
| useEffect(() => { | ||
| const unlistenPromise = listen<SpawnRegion>("display-region", (event) => { | ||
| setRegion(event.payload); | ||
| }); | ||
| return () => { | ||
| unlistenPromise.then((unlisten) => unlisten()); | ||
| }; | ||
| }, [setRegion]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Move the Tauri bridge out of App.
invoke(...) and these listen(...) subscriptions make src/components/App.tsx own platform I/O again. Please lift this into a hook/service outside src/components/ and pass the resulting callbacks/state in so App stays presentational.
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 27 - 45, The App component currently
performs platform I/O via handleSetDragActive (invoke("set_pig_drag_active",
...)) and two listen(...) subscriptions ("gather-pigs" and "display-region");
move that logic into a dedicated hook or service (e.g., useTauriBridge or
tauriBridge) outside src/components, expose functions like
setPigDragActive(active: boolean) and subscription APIs like onGather(callback)
and onDisplayRegion(callback) that return cleanup functions, and update App to
be purely presentational by receiving the callbacks/state (e.g.,
onSetDragActive, region, onGather) as props; ensure the new hook handles
invoking, listen generic typing (SpawnRegion), and unlisten cleanup so App no
longer imports invoke or listen or calls setRegion/gather directly.
Comment on lines
+249
to
+251
| const setRegion = useCallback((r: SpawnRegion) => { | ||
| regionRef.current = r; | ||
| }, []); |
There was a problem hiding this comment.
Initial pigs can still spawn outside the primary display.
regionRef starts as defaultRegion(), the focuses effect immediately seeds pigs with that full-window region, and setRegion() only mutates a ref. If the first "display-region" event arrives after mount, the existing pigs never get reinitialized into the primary region, so the off-screen spawn bug can still happen on startup.
Gate initial spawning on the first real region payload, or trigger a one-time respawn/reclamp when setRegion() receives the initial display region.
Also applies to: 331-341
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/usePigMovement.ts` around lines 249 - 251, The initial spawn uses
defaultRegion() and never updates existing pigs when the real display region
arrives because setRegion only mutates regionRef; fix by gating the initial
spawn on receiving the first real region payload (the "display-region" event) or
by making setRegion (the useCallback that updates regionRef.current) trigger a
one-time respawn/reclamp of existing pigs when it receives the first non-default
region; update the focuses effect (and the logic referenced around lines
331-341) to either wait for a boolean like hasReceivedRegion before seeding pigs
or call the respawn/reclamp routine from setRegion when first invoked to move
any off-screen pigs into the primary region.
Comment on lines
+279
to
+296
| const gather = useCallback(() => { | ||
| setPigs((prev) => { | ||
| const r = regionRef.current; | ||
| const margin = 20; | ||
| const rowHeight = PIG_SIZE + 24; | ||
| const colWidth = PIG_SIZE + 24; | ||
| const rows = Math.max(1, Math.floor((r.h - margin * 2) / rowHeight)); | ||
| const next = prev.map((p, i) => ({ | ||
| ...p, | ||
| x: r.x + r.w - margin - PIG_SIZE - Math.floor(i / rows) * colWidth, | ||
| y: r.y + margin + (i % rows) * rowHeight, | ||
| vx: 0, | ||
| vy: 0, | ||
| })); | ||
| pigsRef.current = next; | ||
| return next; | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
Sync hit rects when gather() teleports pigs.
This updates React state immediately, but the Rust hit tester keeps the old rectangles until the next periodic sync. That leaves a brief ghost-interaction window after the tray action.
Possible fix
const next = prev.map((p, i) => ({
...p,
x: r.x + r.w - margin - PIG_SIZE - Math.floor(i / rows) * colWidth,
y: r.y + margin + (i % rows) * rowHeight,
vx: 0,
vy: 0,
}));
pigsRef.current = next;
+ syncRects(next, selectedIdRef.current !== null || dragIdRef.current !== null);
return next;
});
}, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/usePigMovement.ts` around lines 279 - 296, The gather() callback
teleports pigs by updating React state and pigsRef.current but does not update
the Rust hit tester, causing stale hit rects; after computing next and assigning
pigsRef.current (inside gather), invoke the existing hit-rect synchronization
routine (the same function you use in the periodic movement loop—e.g.,
syncHitRects / updateRustHitTester / sendHitRectsToRust) to push the new
rectangles immediately to Rust so the hit tester matches the new positions; if
no single helper exists, reuse the movement-loop logic that serializes
pigsRef.current and sends it to Rust and call that from gather after
setPigs/pigsRef.current update.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 36-47: Add a short changelog bullet documenting the public API
change: state that usePigMovement now returns the additional properties gather
and setRegion (in addition to pigs, startDrag, moveDrag, endDrag), and describe
their purpose briefly (gather triggers programmatic pig gathering; setRegion
updates the region constraints). Reference usePigMovement, gather, and setRegion
so consumers know to update their imports/usage.
🪄 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: b5f45f62-64d8-47ea-bd5b-0aa391d1f296
📒 Files selected for processing (1)
CHANGELOG.md
Comment on lines
+36
to
+47
| ### Changed — 024 | ||
|
|
||
| - Window builder uses `.inner_size().position()` instead of post-build `set_size()` — macOS WKWebView was overriding `set_size()` and resetting window to 800×600 | ||
| - `from_tauri` divides monitor size (physical) and position (logical) each by `scale_factor` → uniform logical space for `compute_span` | ||
| - `compute_span` and `disambiguate_names` now in pure `display/monitor.rs` with tests | ||
| - `PIG_SPEED` raised 35 → 60 px/s; minimum velocity floor (35% of `PIG_SPEED`) so pigs never appear frozen | ||
| - `tickPig` uses `effectiveMaxY = primaryRegion.h` when pig is in primary display x-range — prevents pigs entering the dead zone below the main display on a multi-height span | ||
| - Drag: `startDrag` sends wide hit-rect immediately (was deferred up to 67ms); `endDrag` restores narrow rects immediately | ||
| - `PigSprite` adds `onPointerCancel` + `onLostPointerCapture` handlers to clean up stale drag state | ||
| - `gather()` places pigs relative to `primaryRegion` top-right instead of raw `screenW` | ||
| - New-focus window: dark opaque background (`rgba(22,22,26,0.97)`), larger padding, readable inputs | ||
| - Confirm-delete dialog removed from tray — deletes immediately; setting tracked in #027 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document the API change to usePigMovement return type.
The "Changed" section documents implementation details like gather() wrapping behavior (line 33) and drag timing changes, but doesn't explicitly document that usePigMovement now returns additional properties gather and setRegion. This is a public API change that consumers need to know about.
Consider adding an entry like:
- `usePigMovement` now returns `{ pigs, startDrag, moveDrag, endDrag, gather, setRegion }` — adds programmatic pig gathering and region constraint updates🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` around lines 36 - 47, Add a short changelog bullet documenting
the public API change: state that usePigMovement now returns the additional
properties gather and setRegion (in addition to pigs, startDrag, moveDrag,
endDrag), and describe their purpose briefly (gather triggers programmatic pig
gathering; setRegion updates the region constraints). Reference usePigMovement,
gather, and setRegion so consumers know to update their imports/usage.
This was referenced May 4, 2026
Merged
This was referenced May 5, 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.
Status
WIP / BROKEN — do not merge. Cross-monitor drag and boundary behaviour still has issues. Pigs visible on primary display; multi-monitor roaming partially functional.
What landed
Replaces
app/overlay_manager.rs+app/pig_hittest.rswith adisplay/module tree that fixes the root coordinate bug.Root causes found and fixed
set_size()afterbuild()overridden by macOS WKWebView init → window always 800×600WebviewWindowBuilder.inner_size().position()so dimensions are baked into window creationpositionin logical pts +sizein physical px) → wrong spanfrom_tauridivides both byscale_factor→ uniform logical space;compute_spanoperates entirely in logical coordsdisplay-regionevent with CSS offset+size of primary monitor; React confinesinitPigto that regiondrag_active: Arc<AtomicBool>in hit-test thread; JS sets it true onpointerdown→ thread forces interactive for entire dragtickPiguseseffectiveMaxY = primaryRegion.hwhen pig is in main-display x-rangeModule tree
Other changes
PIG_SPEEDraised to 60, minimum velocity floor prevents apparent freeze~/Library/Logs/com.adhd-ranch.app/Adhd Ranch.logCompletion promise
Not yet achieved. Cross-monitor drag still unreliable on the 270°-rotated portrait monitor.
Known remaining issues
Ref: issues/024-display-subsystem.md
Summary by CodeRabbit
New Features
Improvements