Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"@tauri-apps/cli": "^2.1.0",
"@testing-library/jest-dom": "^6.6.3",
"@testing-library/react": "^16.1.0",
"@testing-library/user-event": "^14.6.1",
"@types/node": "^25.6.0",
"@types/react": "^18.3.12",
"@types/react-dom": "^18.3.1",
Expand Down
18 changes: 18 additions & 0 deletions src/components/App.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { describe, expect, it, vi } from "vitest";
import { createFixtureFocusReader } from "../api/fixtureFocusReader";
import type { FocusWriter } from "../api/focusWriter";
Expand Down Expand Up @@ -56,4 +57,21 @@ describe("App overlay", () => {
expect(screen.getByText("API refactor")).toBeInTheDocument();
});
});

it("add-task input calls focusWriter.appendTask with selected focus id", async () => {
const writer = noopFocusWriter();
render(<App focusReader={createFixtureFocusReader(sample)} focusWriter={writer} />);

await waitFor(() => {
expect(screen.getByText("Customer X bug")).toBeInTheDocument();
});

// Click the pig to open PigDetail
await userEvent.click(screen.getByText("Customer X bug"));

const input = screen.getByPlaceholderText("Add task…");
await userEvent.type(input, "write tests{Enter}");

expect(writer.appendTask).toHaveBeenCalledWith("a", "write tests");
});
});
10 changes: 10 additions & 0 deletions src/components/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ export function App({ focusReader, focusWriter }: AppProps) {
}
}

async function handleAddTask(text: string) {
if (!selectedFocus) return;
try {
await focusWriter.appendTask(selectedFocus.id, text);
} catch {
// focusWriter already logs the typed error
}
}
Comment on lines +34 to +41

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.


return (
<div className="overlay-root">
{pigs.map((pig) => (
Expand All @@ -53,6 +62,7 @@ export function App({ focusReader, focusWriter }: AppProps) {
viewportH={screenH}
onClose={() => setSelectedId(null)}
onClearTask={handleClearTask}
onAddTask={handleAddTask}
/>
)}
</div>
Expand Down
62 changes: 62 additions & 0 deletions src/components/PigDetail.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { describe, expect, it, vi } from "vitest";
import type { Focus } from "../types/focus";
import { PigDetail } from "./PigDetail";

const baseFocus: Focus = {
id: "pig-a",
title: "Ship it",
description: "",
tasks: [],
};

function renderDetail(overrides?: Partial<React.ComponentProps<typeof PigDetail>>) {
const props = {
focus: baseFocus,
pigX: 100,
pigY: 100,
viewportW: 1920,
viewportH: 1080,
onClose: vi.fn(),
onClearTask: vi.fn(),
onAddTask: vi.fn(),
...overrides,
};
render(<PigDetail {...props} />);
return props;
}

describe("PigDetail add-task input", () => {
it("renders an add task input with placeholder", () => {
renderDetail();
expect(screen.getByPlaceholderText("Add task…")).toBeInTheDocument();
});

it("Enter calls onAddTask with trimmed text", async () => {
const { onAddTask } = renderDetail();
const input = screen.getByPlaceholderText("Add task…");
await userEvent.type(input, "fix the thing{Enter}");
expect(onAddTask).toHaveBeenCalledWith("fix the thing");
});

it("Enter clears the input field", async () => {
renderDetail();
const input = screen.getByPlaceholderText("Add task…");
await userEvent.type(input, "some task{Enter}");
expect(input).toHaveValue("");
});

it("Enter with empty text does not call onAddTask", async () => {
const { onAddTask } = renderDetail();
const input = screen.getByPlaceholderText("Add task…");
await userEvent.type(input, " {Enter}");
expect(onAddTask).not.toHaveBeenCalled();
});

it("Escape closes the card", async () => {
const { onClose } = renderDetail();
await userEvent.keyboard("{Escape}");
expect(onClose).toHaveBeenCalled();
});
});
20 changes: 18 additions & 2 deletions src/components/PigDetail.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect } from "react";
import { useEffect, useState } from "react";
import { PIG_SIZE } from "../hooks/usePigMovement";
import type { Focus } from "../types/focus";

Expand All @@ -10,9 +10,10 @@ export interface PigDetailProps {
readonly viewportH: number;
readonly onClose: () => void;
readonly onClearTask: (index: number) => void;
readonly onAddTask: (text: string) => void;
}

const CARD_W = 210;
const CARD_W = 340;
const CARD_OFFSET_X = PIG_SIZE + 8;

export function PigDetail({
Expand All @@ -23,7 +24,10 @@ export function PigDetail({
viewportH,
onClose,
onClearTask,
onAddTask,
}: PigDetailProps) {
const [taskInput, setTaskInput] = useState("");

Comment on lines +29 to +30

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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).

useEffect(() => {
const handler = (e: KeyboardEvent) => {
if (e.key === "Escape") onClose();
Expand Down Expand Up @@ -67,6 +71,18 @@ export function PigDetail({
))}
</ul>
)}
<input
className="pig-detail-add-task"
placeholder="Add task…"
value={taskInput}
onChange={(e) => setTaskInput(e.target.value)}
onKeyDown={(e) => {
if (e.key === "Enter" && taskInput.trim()) {
onAddTask(taskInput.trim());
setTaskInput("");
}
}}
/>
</div>
</>
);
Expand Down
30 changes: 27 additions & 3 deletions src/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -429,10 +429,11 @@ body,
.pig-detail {
position: absolute;
pointer-events: auto;
background: rgba(28, 28, 30, 0.96);
background: rgba(20, 20, 20, 1);
border: 1px solid rgba(255, 255, 255, 0.1);
border-radius: 10px;
padding: 10px 12px;
padding: 16px;
min-width: 340px;
color: #f5f5f7;
font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
box-shadow: 0 4px 24px rgba(0, 0, 0, 0.5);
Expand All @@ -453,11 +454,13 @@ body,

.pig-detail-tasks {
list-style: none;
margin: 0;
margin: 0 0 10px;
padding: 0;
display: flex;
flex-direction: column;
gap: 4px;
max-height: 240px;
overflow-y: auto;
}

.pig-detail-task {
Expand Down Expand Up @@ -487,3 +490,24 @@ body,
.pig-detail-task-clear:hover {
opacity: 1;
}

.pig-detail-add-task {
width: 100%;
margin-top: 10px;
background: rgba(255, 255, 255, 0.06);
border: 1px solid rgba(255, 255, 255, 0.12);
border-radius: 6px;
color: #f5f5f7;
font-size: 12px;
padding: 5px 8px;
outline: none;
box-sizing: border-box;
}

.pig-detail-add-task::placeholder {
opacity: 0.45;
}

.pig-detail-add-task:focus {
border-color: rgba(255, 255, 255, 0.3);
}
Loading