feat(pig-detail): inline edit + delete in popover#44
Merged
archae0pteryx merged 4 commits intomainfrom May 5, 2026
Merged
Conversation
Adds end-to-end rename_focus / update_task / toggle_task across storage, commands, Tauri bridge, HTTP PATCH routes, and frontend FocusWriter. PigDetail popover now renders title and task text as always-editable inputs (commit on blur/Enter, revert on Escape), task checkboxes that flip [ ]/[x] in markdown, and a trash button gated by the widget.confirm_delete setting (inline confirm overlay vs immediate). Domain Task gains a `done: bool` field, parsed from the markdown checkbox state.
|
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 (2)
📝 WalkthroughWalkthroughAdds task completion tracking and three mutation operations: rename focus, update task text, and toggle task done. Changes span domain model and parsing, storage trait and Markdown implementation, command validation and delegation, HTTP/Tauri APIs, frontend wiring/components/styles, and tests/fixtures. ChangesFocus & Task Mutation
Sequence DiagramsequenceDiagram
actor User
participant React as "React UI"
participant Tauri as "Tauri Bridge"
participant Commands as "Commands"
participant Storage as "FocusStore"
participant FS as "Filesystem"
User->>React: Toggle task checkbox
React->>Tauri: invoke('toggle_task', { focusId, index, done })
Tauri->>Commands: commands.toggle_task(focusId, index, done)
Commands->>Storage: store.toggle_task(focusId, index, done)
Storage->>FS: read focus.md
Storage->>Storage: find Nth checkbox, rewrite line
Storage->>FS: write updated focus.md atomically
FS-->>Storage: write complete
Storage-->>Commands: Ok(())
Commands-->>Tauri: Ok(())
Tauri-->>React: Ok(())
React-->>User: UI updated (done state)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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)
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/commands/src/focus.rs`:
- Around line 239-253: The test toggle_task_round_trip currently only ensures no
panic; update it to assert the final task done state by loading the focus after
the toggles and checking the task's done field is false. In the
toggle_task_round_trip test, after the two commands.toggle_task calls (and using
the created.id from commands.create_focus/CreateFocusInput and the appended task
at index 0), call the appropriate read/get method on commands (e.g.,
commands.get_focus or commands.read_focus) to retrieve the focus, access its
tasks[0].done, and assert it equals false to validate the round-trip.
In `@crates/http-api/src/router/tests.rs`:
- Around line 168-186: Extend the patch_task_toggles_done test to also verify
reverse toggling: after the existing PATCH that sets {"done": true} and
asserting "- [x] one", send a second PATCH via the same helper patch_json to
"/focuses/f1/tasks/0" with {"done": false}, assert StatusCode::NO_CONTENT again,
read the file at h.focuses_root.join("f1/focus.md") and assert the content
contains "- [ ] one"; update the test function patch_task_toggles_done and reuse
make_app, write_focus, focus_md, and patch_json helpers so both directions
(true→false and false→true) are covered.
In `@src/components/App.tsx`:
- Around line 26-30: The App component currently performs I/O by calling
getSettings inside its useEffect and setting local state via setConfirmDelete;
move this logic out of src/components by creating a non-component adapter or
hook (e.g., useSettingsService or settingsService.getConfirmDelete) outside the
components folder, have that module call getSettings and expose confirmDelete
(and a subscription or promise) and then inject the boolean into the App
component as a prop (confirmDelete) or via a lightweight view-only hook imported
from outside src/components; update App to remove
useEffect/getSettings/setConfirmDelete and rely only on the injected
confirmDelete value.
🪄 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: 1fbe4235-4d6c-4d44-a2cd-df2c211718b5
📒 Files selected for processing (22)
crates/commands/src/caps.rscrates/commands/src/focus.rscrates/domain/src/caps.rscrates/domain/src/focus.rscrates/domain/src/parse.rscrates/http-api/src/router/focuses.rscrates/http-api/src/router/tests.rscrates/storage/src/focus_store.rssrc-tauri/src/app/mod.rssrc-tauri/src/ui_bridge/mod.rssrc/api/fixtureFocusReader.test.tssrc/api/focusWriter.tssrc/api/tauriFocusReader.tssrc/components/App.test.tsxsrc/components/App.tsxsrc/components/FocusCard.test.tsxsrc/components/FocusList.test.tsxsrc/components/PigDetail.test.tsxsrc/components/PigDetail.tsxsrc/lib/capState.test.tssrc/styles.csssrc/types/focus.ts
macOS demotes NSWindow level across key/resign-key transitions when the window is not an NSPanel. Popover inputs make the overlay key on click; without re-application the overlay drops below other windows.
- Assert final done state in toggle_task round-trip test - Add reverse-toggle (true→false) coverage in PATCH task test - Extract getSettings IPC out of App.tsx into useConfirmDelete hook to keep components view-only per project conventions
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/display/overlay.rs`:
- Around line 70-78: The on_window_event handler in overlay::ensure_shown is
being re-registered on every call, causing duplicate handlers; change
ensure_shown (or overlay initialization) to register the window event listener
exactly once — e.g. add a boolean/once flag (or store the handler id) on the
overlay/display manager state and check it before calling
window.on_window_event, or move the registration into the overlay/module init
path so DisplayManager::apply and overlay::ensure_shown no longer re-register;
keep the handler body calling crate::app::window_always_on_top::apply(&win_evt,
true) unchanged.
In `@src/hooks/useConfirmDelete.ts`:
- Around line 4-11: The fetched settings currently fall back to true and ignore
the hook's initial parameter; in useConfirmDelete replace the hardcoded fallback
true with the initial argument so the setConfirmDelete call uses
s?.widget?.confirm_delete ?? initial; keep the initial default parameter and
retain the existing error handling and useEffect shape.
🪄 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: 99475939-4353-417d-8cbc-443078a2d0a4
📒 Files selected for processing (5)
crates/commands/src/focus.rscrates/http-api/src/router/tests.rssrc-tauri/src/display/overlay.rssrc/components/App.tsxsrc/hooks/useConfirmDelete.ts
- Move overlay window-event registration into the new-window branch so repeated ensure_shown calls on display config changes don't accumulate duplicate Focused listeners - useConfirmDelete: fall back to initial param (not hardcoded true) when fetched settings are missing, and include initial in effect deps
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
- [ ] ↔ - [x].settings.widget.confirm_delete: shows inlineDelete "…"? [Delete] [Cancel]overlay when true, deletes immediately when false.rename_focus/update_task/toggle_taskacross storage → commands → Tauri bridge → HTTPPATCH /focuses/:idandPATCH /focuses/:id/tasks/:idx→ frontendFocusWriter.Taskgains adone: boolfield, parsed from checkbox state. Slug/dir name remains stable on rename — id never changes.Layer counts
storage::focus_storecommands::focushttp-apiPigDetail.test.tsxtask checkgreen: rust tests 51 + 68 + 24 + 20, vitest 71 passed.Test plan
task dev(port 1420 currently held — kill old node before relaunch so new Tauri commands register).title:updates; pig sprite name updates.[ ]/[x]state.[ ] ↔ [x]; UI shows strikethrough on done.confirm_delete = true. Click trash → inlineDelete "…"?overlay; Cancel hides; Delete removes focus dir + closes popover.confirm_delete = false. Click trash → immediate delete + close.Summary by CodeRabbit
New Features
Style
Tests
Chores