feat: 019 — PigDetail card redesign - #21
Conversation
Add onAddTask prop; Enter appends task via focusWriter.appendTask and clears the field. Card background fully opaque, min-width 340px, padding 16px, task list scrolls at max-height 240px.
📝 WalkthroughWalkthroughThis PR adds an inline task-creation input to the PigDetail card component. It includes the new ChangesTask Input Implementation
Sequence DiagramsequenceDiagram
actor User
participant App
participant PigDetail
participant focusWriter
User->>App: Click "Customer X bug" pig
App->>PigDetail: Render with onAddTask callback
User->>PigDetail: Type "write tests" in Add task input
User->>PigDetail: Press Enter
PigDetail->>App: Call onAddTask("write tests")
App->>focusWriter: appendTask(selectedFocus.id, "write tests")
focusWriter-->>App: Task appended
App-->>PigDetail: Handler completes
PigDetail->>PigDetail: Clear input field
PigDetail-->>User: Input cleared, ready for next task
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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: 8/10 reviews remaining, refill in 11 minutes and 56 seconds. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/PigDetail.tsx (1)
40-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp card X position to a left boundary.
At Line 40,
Math.min(...)alone can yield a negative X on narrow viewports, making the card partially off-screen and hard to close/interact with.Proposed fix
- const x = Math.min(rawX, viewportW - CARD_W - 16); + const x = Math.max(16, Math.min(rawX, viewportW - CARD_W - 16));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PigDetail.tsx` at line 40, The computed card X uses Math.min only (const x = Math.min(rawX, viewportW - CARD_W - 16)) which can go negative; update the clamp in the PigDetail component so x is bounded on both sides by taking the max of the left padding and the current min value (use left boundary 16 and right boundary viewportW - CARD_W - 16 referencing rawX and CARD_W) to ensure the card never renders off-screen.
🤖 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 App component currently performs direct I/O in
handleAddTask by calling focusWriter.appendTask(selectedFocus.id, text); extract
that persistence logic into a non-UI layer (e.g., create a task service or hook
like useTaskService or TaskController with a method appendTask(focusId: string,
text: string) that handles errors/logging) and replace the in-component call
with a UI-safe callback (e.g., call addTask(text) or useAddTask(text) from the
service/hook). Ensure the new service API accepts a focusId (so App only passes
selectedFocus.id or a selectedFocusChanged callback) and remove any direct I/O
from App.handleAddTask and other occurrences (also update the similar call
around line 65) so App becomes view-only and consumes the service's callback.
In `@src/components/PigDetail.tsx`:
- Around line 29-30: In PigDetail, task draft text persists across focus
switches: add logic to reset taskInput by calling setTaskInput("") whenever the
selected focus changes (watch the prop/state that represents the current focus,
e.g., selectedFocus or selectedFocusId) — implement this with a useEffect inside
the PigDetail component that depends on that selected-focus identifier so
taskInput is cleared on focus switch (also ensure any submit/close handlers
around the existing task creation flow that reference taskInput/ setTaskInput
follow the same reset behavior).
---
Outside diff comments:
In `@src/components/PigDetail.tsx`:
- Line 40: The computed card X uses Math.min only (const x = Math.min(rawX,
viewportW - CARD_W - 16)) which can go negative; update the clamp in the
PigDetail component so x is bounded on both sides by taking the max of the left
padding and the current min value (use left boundary 16 and right boundary
viewportW - CARD_W - 16 referencing rawX and CARD_W) to ensure the card never
renders off-screen.
🪄 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: 3be9c248-62d4-41fb-8f8c-fb84386497cd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.jsonsrc/components/App.test.tsxsrc/components/App.tsxsrc/components/PigDetail.test.tsxsrc/components/PigDetail.tsxsrc/styles.css
| 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 add-task persistence out of the component layer.
At Line 37, App performs direct I/O via focusWriter.appendTask(...). This violates the component-layer boundary and should be delegated to a non-UI layer (e.g., hook/controller/service), with App consuming a UI-safe callback.
As per coding guidelines, src/components/**/*.{ts,tsx}: Frontend React components in src/components/ should be view-only with no fetch and no direct I/O.
Also applies to: 65-65
🤖 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 App component currently
performs direct I/O in handleAddTask by calling
focusWriter.appendTask(selectedFocus.id, text); extract that persistence logic
into a non-UI layer (e.g., create a task service or hook like useTaskService or
TaskController with a method appendTask(focusId: string, text: string) that
handles errors/logging) and replace the in-component call with a UI-safe
callback (e.g., call addTask(text) or useAddTask(text) from the service/hook).
Ensure the new service API accepts a focusId (so App only passes
selectedFocus.id or a selectedFocusChanged callback) and remove any direct I/O
from App.handleAddTask and other occurrences (also update the similar call
around line 65) so App becomes view-only and consumes the service's callback.
| const [taskInput, setTaskInput] = useState(""); | ||
|
|
There was a problem hiding this comment.
Reset draft input when the selected focus changes.
taskInput persists across focus switches because local state is reused. This leaks draft text between cards.
Proposed fix
const [taskInput, setTaskInput] = useState("");
+
+ useEffect(() => {
+ setTaskInput("");
+ }, [focus.id]);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` around lines 29 - 30, In PigDetail, task draft
text persists across focus switches: add logic to reset taskInput by calling
setTaskInput("") whenever the selected focus changes (watch the prop/state that
represents the current focus, e.g., selectedFocus or selectedFocusId) —
implement this with a useEffect inside the PigDetail component that depends on
that selected-focus identifier so taskInput is cleared on focus switch (also
ensure any submit/close handlers around the existing task creation flow that
reference taskInput/ setTaskInput follow the same reset behavior).
Summary
onAddTask(text)prop toPigDetail; Enter appends task viafocusWriter.appendTaskand clears fieldrgba(20,20,20,1))min-width: 340px,padding: 16px, task listmax-height: 240pxwith scrollCARD_Wconstant updated to 340 to keep card-position clamping in syncTest plan
PigDetail.test.tsx: renders input, Enter callsonAddTask, clears field, empty text blocked, Escape still closesApp.test.tsx: add-task routes tofocusWriter.appendTaskwith correct focus idtask checkgreenCloses #17
Summary by CodeRabbit
New Features
Tests
Chores