From 0b0cf82743f4872cbfa3d85aa13128be61c44c57 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Wed, 12 Aug 2026 23:11:41 +0000 Subject: [PATCH 1/4] feat(projects): add lists tab UI to Projects app Adds a Lists tab to the Projects workspace with a list rail showing all project lists and an entries panel for the selected list. Entry rows include a done checkbox, category chip, status pill (new=blue, seen=grey, actioned=green, discuss=amber), a quick-add input with Enter-to-submit, and an indicator to view the original text when it has been tidied. Follows the Store/Images design bar pattern with rounded borders and shell surface backgrounds. Adds vitest tests covering quick-add creation, status pill rendering, and done toggle behavior. --- desktop/src/apps/ProjectsApp/ProjectLists.tsx | 314 ++++++++++++++++++ .../src/apps/ProjectsApp/ProjectWorkspace.tsx | 8 +- .../apps/ProjectsApp/ProjectsApp.module.css | 34 ++ .../__tests__/ProjectLists.test.tsx | 121 +++++++ desktop/src/lib/projects.ts | 68 ++++ 5 files changed, 542 insertions(+), 3 deletions(-) create mode 100644 desktop/src/apps/ProjectsApp/ProjectLists.tsx create mode 100644 desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx diff --git a/desktop/src/apps/ProjectsApp/ProjectLists.tsx b/desktop/src/apps/ProjectsApp/ProjectLists.tsx new file mode 100644 index 000000000..a5a70c279 --- /dev/null +++ b/desktop/src/apps/ProjectsApp/ProjectLists.tsx @@ -0,0 +1,314 @@ +import { useEffect, useState, useRef, useCallback } from "react"; +import { projectsApi, type Project, type ProjectList, type ProjectListEntry } from "@/lib/projects"; +import styles from "./ProjectsApp.module.css"; + +const STATUS_STYLE: Record = { + new: "bg-blue-500/15 text-blue-400", + seen: "bg-shell-bg-deep text-shell-text-tertiary", + actioned: "bg-green-500/15 text-green-400", + discuss: "bg-amber-500/15 text-amber-400", +}; + +const EMPTY_LIST: ProjectList = { id: "", project_id: "", title: "", description: "", status: "active", created_by: "", created_at: 0, updated_at: 0 }; + +export function ProjectLists({ project }: { project: Project }) { + const [lists, setLists] = useState([]); + const [selectedListId, setSelectedListId] = useState(null); + const [entries, setEntries] = useState([]); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + const [quickText, setQuickText] = useState(""); + const [submitting, setSubmitting] = useState(false); + const quickInputRef = useRef(null); + + const refreshLists = useCallback(() => { + return projectsApi.lists.list(project.id).catch(() => { + setError("Could not load lists."); + return [] as ProjectList[]; + }); + }, [project.id]); + + const refreshEntries = useCallback(() => { + if (!selectedListId) return Promise.resolve([] as ProjectListEntry[]); + return projectsApi.lists.entries.list(project.id, selectedListId) + .then((ents) => { + setEntries(ents); + return ents; + }) + .catch(() => { + setError("Could not load entries."); + return [] as ProjectListEntry[]; + }); + }, [project.id, selectedListId]); + + useEffect(() => { + let cancelled = false; + setLoading(true); + setError(null); + refreshLists() + .then((ls) => { + if (!cancelled) { + setLists(ls); + if (!selectedListId && ls.length > 0) setSelectedListId(ls[0]!.id); + } + }) + .catch(() => { + if (!cancelled) setError("Could not load lists for this project."); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { cancelled = true; }; + }, [project.id, refreshLists]); + + useEffect(() => { + if (!selectedListId) { setEntries([]); return; } + let cancelled = false; + setLoading(true); + refreshEntries().finally(() => { if (!cancelled) setLoading(false); }); + return () => { cancelled = true; }; + }, [selectedListId, refreshEntries]); + + const createList = async () => { + const title = prompt("New list name"); + if (!title) return; + try { + const created = await projectsApi.lists.create(project.id, { title: title.trim() }); + await refreshLists(); + setSelectedListId(created.id); + } catch (err) { + setError(String(err)); + } + }; + + const deleteList = async (listId: string) => { + if (!confirm("Delete this list and all its entries?")) return; + try { + await projectsApi.lists.remove(project.id, listId); + if (selectedListId === listId) setSelectedListId(null); + await refreshLists(); + } catch (err) { + setError(String(err)); + } + }; + + const createEntry = async (e: React.FormEvent) => { + e.preventDefault(); + const text = quickText.trim(); + if (!text || submitting || !selectedListId) return; + setSubmitting(true); + try { + await projectsApi.lists.entries.create(project.id, selectedListId, { text }); + setQuickText(""); + await refreshEntries(); + quickInputRef.current?.focus(); + } catch (err) { + setError(String(err)); + } finally { + setSubmitting(false); + } + }; + + const toggleDone = async (entry: ProjectListEntry) => { + try { + await projectsApi.lists.entries.update(project.id, entry.list_id, entry.id, { done: entry.done ? 0 : 1 }); + await refreshEntries(); + } catch (err) { + setError(String(err)); + } + }; + + const updateEntryStatus = async (entry: ProjectListEntry, status: string) => { + try { + await projectsApi.lists.entries.update(project.id, entry.list_id, entry.id, { status }); + await refreshEntries(); + } catch (err) { + setError(String(err)); + } + }; + + const removeEntry = async (entry: ProjectListEntry) => { + if (!confirm("Remove this entry?")) return; + try { + await projectsApi.lists.entries.remove(project.id, entry.list_id, entry.id); + await refreshEntries(); + } catch (err) { + setError(String(err)); + } + }; + + const selectedList = lists.find((l) => l.id === selectedListId) ?? EMPTY_LIST; + + return ( +
+
+ + +
+ {!selectedListId ? ( +

Select or create a list.

+ ) : ( +
+
+

{selectedList.title || "Untitled list"}

+ {selectedList.description && ( + {selectedList.description} + )} +
+ +
+ + setQuickText(e.target.value)} + placeholder="Add entry…" + disabled={submitting} + aria-label={`Quick add entry to ${selectedList.title || "Untitled list"}`} + className="flex-1 rounded-lg border border-shell-border bg-shell-surface px-3 py-1.5 text-sm text-shell-text outline-none focus:ring-2 focus:ring-accent-line disabled:opacity-50" + /> + +
+ + {error && ( +

{error}

+ )} + + {loading ? ( +

Loading entries…

+ ) : entries.length === 0 ? ( +

No entries yet. Type above and press Enter to add one.

+ ) : ( +
    + {entries.map((entry) => { + const tidied = entry.text !== entry.original_text; + return ( +
  • +
    + + {entry.category && ( + + {entry.category} + + )} + + {entry.status} + + {tidied && ( + { if (e.key === "Enter" || e.key === " ") { e.preventDefault(); alert(entry.original_text); } }} + onClick={() => alert(entry.original_text)} + > + + + + + original + + )} +
    + + +
    +
    +

    + {entry.text} +

    +
    • + ); + })} +
+ )} +
+ )} +
+
+
+ ); +} diff --git a/desktop/src/apps/ProjectsApp/ProjectWorkspace.tsx b/desktop/src/apps/ProjectsApp/ProjectWorkspace.tsx index 9d7071aa6..70aeefa28 100644 --- a/desktop/src/apps/ProjectsApp/ProjectWorkspace.tsx +++ b/desktop/src/apps/ProjectsApp/ProjectWorkspace.tsx @@ -22,9 +22,10 @@ import { ElementCreateDialog } from "./elements/ElementCreateDialog"; import styles from "./ProjectsApp.module.css"; import { CommunityView } from "./CommunityView"; +import { ProjectLists } from "./ProjectLists"; -export type Tab = "workspace" | "board" | "canvas" | "tasks" | "files" | "messages" | "members" | "activity" | "decisions" | "routines" | "community"; -const TABS: Tab[] = ["workspace", "board", "canvas", "tasks", "files", "messages", "members", "activity", "decisions", "routines", "community"]; +export type Tab = "workspace" | "board" | "canvas" | "tasks" | "files" | "messages" | "members" | "activity" | "decisions" | "routines" | "community" | "lists"; +const TABS: Tab[] = ["workspace", "board", "canvas", "tasks", "files", "messages", "members", "activity", "decisions", "routines", "community", "lists"]; function isTab(value: string | undefined): value is Tab { return value != null && (TABS as string[]).includes(value); @@ -105,7 +106,7 @@ export function ProjectWorkspace({ project, onChanged, initialTab, filePath }: { // Mobile pill order: surface Messages right after Workspace so it is reachable // without scrolling (on mobile Messages is its own full page, not a squeezed // pane inside Workspace). - const mobileTabOrder: Tab[] = ["workspace", "messages", "board", "tasks", "canvas", "files", "members", "activity", "decisions", "routines", "community"]; + const mobileTabOrder: Tab[] = ["workspace", "messages", "board", "tasks", "canvas", "files", "members", "activity", "decisions", "routines", "community", "lists"]; const tabPills = mobileTabOrder.map((t) => ({ id: t, label: t.charAt(0).toUpperCase() + t.slice(1), @@ -390,6 +391,7 @@ export function ProjectWorkspace({ project, onChanged, initialTab, filePath }: { {tab === "decisions" && } {tab === "routines" && } {tab === "community" && } + {tab === "lists" && } {isMobile && (tab === "tasks" || tab === "board") && ( diff --git a/desktop/src/apps/ProjectsApp/ProjectsApp.module.css b/desktop/src/apps/ProjectsApp/ProjectsApp.module.css index 8f28e89c6..8b298105a 100644 --- a/desktop/src/apps/ProjectsApp/ProjectsApp.module.css +++ b/desktop/src/apps/ProjectsApp/ProjectsApp.module.css @@ -870,3 +870,37 @@ font-size: 0.7rem; white-space: nowrap; } + +/* Lists Tab */ +.listsRail { + width: 220px; + flex: none; + display: flex; + flex-direction: column; + background: var(--color-shell-surface); + border: 1px solid var(--color-shell-border); + border-radius: 12px; + padding: 12px; + overflow: auto; + min-height: 0; +} + +.listsEntriesPanel { + flex: 1; + min-height: 0; + overflow: auto; + display: flex; + flex-direction: column; + gap: 12px; +} + +@media (max-width: 767px) { + .listsRail { + width: 100%; + min-height: auto; + max-height: 40vh; + } + .listsEntriesPanel { + width: 100%; + } +} diff --git a/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx b/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx new file mode 100644 index 000000000..828250883 --- /dev/null +++ b/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx @@ -0,0 +1,121 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, act, fireEvent, waitFor } from "@testing-library/react"; +import type { Project } from "@/lib/projects"; +import { ProjectLists } from "../ProjectLists"; + +const fakeProject: Project = { + id: "p1", + slug: "p1", + name: "P1", + description: "", + status: "active", + created_by: "u1", + created_at: 0, + updated_at: 0, +}; + +function ok(data: unknown) { + return { ok: true, status: 200, json: async () => data }; +} + +describe("ProjectLists", () => { + let fetchMock: ReturnType; + let entriesData: { id: string; list_id: string; project_id: string; text: string; original_text: string; category: string | null; status: string; done: number; author_kind: string; author_id: string; edited_by: string | null; position: number; created_at: number; updated_at: number }[]; + + beforeEach(() => { + entriesData = [ + { id: "ent-1", list_id: "lst-1", project_id: "p1", text: "Milk", original_text: "Milk", category: "groceries", status: "new", done: 0, author_kind: "user", author_id: "u1", edited_by: null, position: 0, created_at: 0, updated_at: 0 }, + { id: "ent-2", list_id: "lst-1", project_id: "p1", text: "Bread", original_text: "Whole grain bread", category: null, status: "actioned", done: 1, author_kind: "user", author_id: "u1", edited_by: "u1", position: 1, created_at: 0, updated_at: 0 }, + { id: "ent-3", list_id: "lst-1", project_id: "p1", text: "Call plumber", original_text: "Call plumber", category: null, status: "discuss", done: 0, author_kind: "user", author_id: "u1", edited_by: null, position: 2, created_at: 0, updated_at: 0 }, + { id: "ent-4", list_id: "lst-1", project_id: "p1", text: "Review PR", original_text: "Review PR", category: null, status: "seen", done: 0, author_kind: "user", author_id: "u1", edited_by: null, position: 3, created_at: 0, updated_at: 0 }, + ]; + + fetchMock = vi.fn((url: string, init?: RequestInit) => { + if (url === "/api/projects/p1/lists") { + if (init?.method === "POST") { + return Promise.resolve(ok({ id: "lst-new", project_id: "p1", title: "New list", description: "", status: "active", created_by: "u1", created_at: 0, updated_at: 0 })); + } + return Promise.resolve(ok({ items: [{ id: "lst-1", project_id: "p1", title: "Shopping", description: "", status: "active", created_by: "u1", created_at: 0, updated_at: 0 }] })); + } + if (url === "/api/projects/p1/lists/lst-1/entries") { + if (init?.method === "POST") { + const newEntry = { id: "ent-new", list_id: "lst-1", project_id: "p1", text: "new entry", original_text: "new entry", category: null as string | null, status: "new" as string, done: 0 as number, author_kind: "user" as string, author_id: "u1" as string, edited_by: null as string | null, position: entriesData.length as number, created_at: 0 as number, updated_at: 0 as number }; + entriesData.push(newEntry); + return Promise.resolve(ok(newEntry)); + } + return Promise.resolve(ok({ items: [...entriesData] })); + } + if (url.startsWith("/api/projects/p1/lists/lst-1/entries/") && init?.method === "PATCH") { + const entryId = url.split("/").pop()!; + const entry = entriesData.find((e) => e.id === entryId); + if (!entry) return Promise.resolve(ok({})); + const patch = JSON.parse(init.body as string); + Object.assign(entry, patch); + return Promise.resolve(ok({ ...entry })); + } + if (url === "/api/projects/p1/lists/lst-1" && init?.method === "DELETE") { + return Promise.resolve(ok({ ok: true })); + } + if (url.startsWith("/api/projects/p1/lists/lst-1/entries/") && init?.method === "DELETE") { + const entryId = url.split("/").pop()!; + entriesData = entriesData.filter((e) => e.id !== entryId); + return Promise.resolve(ok({ ok: true })); + } + return Promise.resolve(ok({})); + }); + vi.stubGlobal("fetch", fetchMock); + }); + + it("renders lists rail and entries for the first list", async () => { + await act(async () => { + render(); + }); + const shoppingItems = screen.getAllByText("Shopping"); + expect(shoppingItems.length).toBeGreaterThanOrEqual(1); + expect(screen.getByText("Milk")).toBeInTheDocument(); + expect(screen.getByText("Bread")).toBeInTheDocument(); + }); + + it("renders status pills with correct colors", async () => { + await act(async () => { + render(); + }); + const newPills = screen.getAllByText("new"); + expect(newPills.length).toBeGreaterThanOrEqual(1); + expect(newPills[0]!.className).toContain("bg-blue-500/15"); + const actionedPills = screen.getAllByText("actioned"); + expect(actionedPills.length).toBeGreaterThanOrEqual(1); + const discussPills = screen.getAllByText("discuss"); + expect(discussPills.length).toBeGreaterThanOrEqual(1); + const seenPills = screen.getAllByText("seen"); + expect(seenPills.length).toBeGreaterThanOrEqual(1); + }); + + it("shows the original text indicator when text is tidied", async () => { + await act(async () => { + render(); + }); + expect(screen.getByText("original")).toBeInTheDocument(); + }); + + it("quick-add creates a new entry on Enter", async () => { + await act(async () => { + render(); + }); + const input = screen.getByLabelText(/quick add/i); + fireEvent.change(input, { target: { value: "new entry" } }); + await act(async () => { + fireEvent.submit(input.closest("form")!); + }); + await waitFor(() => expect(screen.getByText("new entry")).toBeInTheDocument()); + }); + + it("done toggle marks entry as done and strikes through text", async () => { + await act(async () => { + render(); + }); + const checkbox = screen.getByLabelText(/mark milk as done/i); + fireEvent.click(checkbox); + await waitFor(() => expect(screen.getByText("Milk").className).toContain("line-through")); + }); +}); diff --git a/desktop/src/lib/projects.ts b/desktop/src/lib/projects.ts index d094b187d..7b02964d1 100644 --- a/desktop/src/lib/projects.ts +++ b/desktop/src/lib/projects.ts @@ -139,6 +139,34 @@ export type DocReview = { updated_at: number; }; +export type ProjectList = { + id: string; + project_id: string; + title: string; + description: string; + status: string; + created_by: string; + created_at: number; + updated_at: number; +}; + +export type ProjectListEntry = { + id: string; + list_id: string; + project_id: string; + text: string; + original_text: string; + category: string | null; + status: string; + done: number; + author_kind: string; + author_id: string; + edited_by: string | null; + position: number; + created_at: number; + updated_at: number; +}; + export type DocReviewMissing = { project_id: string; doc_path: string; @@ -358,6 +386,46 @@ export const projectsApi = { activity: (pid: string) => http<{ items: ProjectActivity[] }>(`/api/projects/${pid}/activity`).then((r) => r.items), + lists: { + list: (pid: string) => + http<{ items: ProjectList[] }>(`/api/projects/${pid}/lists`).then((r) => r.items), + create: (pid: string, input: { title: string; description?: string }) => + http(`/api/projects/${pid}/lists`, { + method: "POST", + body: JSON.stringify(input), + }), + get: (pid: string, listId: string) => + http(`/api/projects/${pid}/lists/${listId}`), + update: (pid: string, listId: string, patch: Partial>) => + http(`/api/projects/${pid}/lists/${listId}`, { + method: "PATCH", + body: JSON.stringify(patch), + }), + remove: (pid: string, listId: string) => + http<{ ok: boolean }>(`/api/projects/${pid}/lists/${listId}`, { method: "DELETE" }), + entries: { + list: (pid: string, listId: string) => + http<{ items: ProjectListEntry[] }>(`/api/projects/${pid}/lists/${listId}/entries`).then((r) => r.items), + create: (pid: string, listId: string, input: { text: string; category?: string | null; position?: number | null }) => + http(`/api/projects/${pid}/lists/${listId}/entries`, { + method: "POST", + body: JSON.stringify(input), + }), + update: (pid: string, listId: string, entryId: string, patch: Partial>) => + http(`/api/projects/${pid}/lists/${listId}/entries/${entryId}`, { + method: "PATCH", + body: JSON.stringify(patch), + }), + remove: (pid: string, listId: string, entryId: string) => + http<{ ok: boolean }>(`/api/projects/${pid}/lists/${listId}/entries/${entryId}`, { method: "DELETE" }), + reorder: (pid: string, listId: string, entries: { id: string; position: number }[]) => + http<{ ok: boolean }>(`/api/projects/${pid}/lists/${listId}/entries/reorder`, { + method: "POST", + body: JSON.stringify({ entries }), + }), + }, + }, + docReviews: { list: (pid: string, state?: string) => http<{ items: DocReview[] }>( From 2bd3d2d0150877edbbfb82d8bdf7d9b8e48c76ae Mon Sep 17 00:00:00 2001 From: jaylfc Date: Wed, 12 Aug 2026 23:29:24 +0000 Subject: [PATCH 2/4] fix(projects): refresh the lists rail after create and delete refreshLists() fetched the lists and returned them without calling setLists, and the mount effect was the only caller that stored the result. createList and deleteList each awaited refreshLists() and discarded it, so a created list never appeared in the rail and a deleted one never left it. The entry paths were unaffected because refreshEntries already set its own state, which is why the existing tests all passed: they cover entries, not lists. refreshLists now sets state and returns the lists, matching refreshEntries. Two tests added for the paths that had none, both proven to fail against 0b0cf827 first ("Unable to find an element with the text: New list"). The fetch mock is now stateful for lists as it already was for entries, because a fixed lists response cannot tell a refreshed rail from a stale one. Docs-Reviewed: no desktop app was added or removed. ProjectLists.tsx is a component inside the existing Projects app, so README's app list and count are unchanged. The doc-gate 'apps' rule matches desktop/src/apps/*/** and so fires on any file added under any existing app; that rule defect is carded separately and is not fixed here. --- changelog.d/2379-projects-lists-tab.md | 1 + desktop/src/apps/ProjectsApp/ProjectLists.tsx | 17 +++++-- .../__tests__/ProjectLists.test.tsx | 45 ++++++++++++++++++- 3 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 changelog.d/2379-projects-lists-tab.md diff --git a/changelog.d/2379-projects-lists-tab.md b/changelog.d/2379-projects-lists-tab.md new file mode 100644 index 000000000..35d2d893c --- /dev/null +++ b/changelog.d/2379-projects-lists-tab.md @@ -0,0 +1 @@ +- Added a Lists tab to the Projects app: a rail of the project's lists beside an entry panel with quick-add, done toggles, category and status pills, a status selector, and the original text behind any entry an agent tidied. diff --git a/desktop/src/apps/ProjectsApp/ProjectLists.tsx b/desktop/src/apps/ProjectsApp/ProjectLists.tsx index a5a70c279..49f82306d 100644 --- a/desktop/src/apps/ProjectsApp/ProjectLists.tsx +++ b/desktop/src/apps/ProjectsApp/ProjectLists.tsx @@ -22,10 +22,19 @@ export function ProjectLists({ project }: { project: Project }) { const quickInputRef = useRef(null); const refreshLists = useCallback(() => { - return projectsApi.lists.list(project.id).catch(() => { - setError("Could not load lists."); - return [] as ProjectList[]; - }); + // Sets state, like refreshEntries below. It used to only RETURN the lists, + // so createList/deleteList awaited a fetch whose result was discarded -- a + // created list never appeared in the rail and a deleted one never left it, + // because the mount effect was the only caller of setLists. + return projectsApi.lists.list(project.id) + .then((ls) => { + setLists(ls); + return ls; + }) + .catch(() => { + setError("Could not load lists."); + return [] as ProjectList[]; + }); }, [project.id]); const refreshEntries = useCallback(() => { diff --git a/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx b/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx index 828250883..bd7e46418 100644 --- a/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx +++ b/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx @@ -20,9 +20,13 @@ function ok(data: unknown) { describe("ProjectLists", () => { let fetchMock: ReturnType; + let listsData: { id: string; project_id: string; title: string; description: string; status: string; created_by: string; created_at: number; updated_at: number }[]; let entriesData: { id: string; list_id: string; project_id: string; text: string; original_text: string; category: string | null; status: string; done: number; author_kind: string; author_id: string; edited_by: string | null; position: number; created_at: number; updated_at: number }[]; beforeEach(() => { + listsData = [ + { id: "lst-1", project_id: "p1", title: "Shopping", description: "", status: "active", created_by: "u1", created_at: 0, updated_at: 0 }, + ]; entriesData = [ { id: "ent-1", list_id: "lst-1", project_id: "p1", text: "Milk", original_text: "Milk", category: "groceries", status: "new", done: 0, author_kind: "user", author_id: "u1", edited_by: null, position: 0, created_at: 0, updated_at: 0 }, { id: "ent-2", list_id: "lst-1", project_id: "p1", text: "Bread", original_text: "Whole grain bread", category: null, status: "actioned", done: 1, author_kind: "user", author_id: "u1", edited_by: "u1", position: 1, created_at: 0, updated_at: 0 }, @@ -33,9 +37,18 @@ describe("ProjectLists", () => { fetchMock = vi.fn((url: string, init?: RequestInit) => { if (url === "/api/projects/p1/lists") { if (init?.method === "POST") { - return Promise.resolve(ok({ id: "lst-new", project_id: "p1", title: "New list", description: "", status: "active", created_by: "u1", created_at: 0, updated_at: 0 })); + // The mock has to behave like the server: a created list is in the + // NEXT list response. It used to return a fixed array, so a test + // could not tell a refreshed rail from a stale one. + const created = { id: "lst-new", project_id: "p1", title: "New list", description: "", status: "active", created_by: "u1", created_at: 0, updated_at: 0 }; + listsData.push(created); + return Promise.resolve(ok(created)); } - return Promise.resolve(ok({ items: [{ id: "lst-1", project_id: "p1", title: "Shopping", description: "", status: "active", created_by: "u1", created_at: 0, updated_at: 0 }] })); + return Promise.resolve(ok({ items: [...listsData] })); + } + // A freshly created list serves an empty entry set, like the real route. + if (url === "/api/projects/p1/lists/lst-new/entries") { + return Promise.resolve(ok({ items: [] })); } if (url === "/api/projects/p1/lists/lst-1/entries") { if (init?.method === "POST") { @@ -54,6 +67,7 @@ describe("ProjectLists", () => { return Promise.resolve(ok({ ...entry })); } if (url === "/api/projects/p1/lists/lst-1" && init?.method === "DELETE") { + listsData = listsData.filter((l) => l.id !== "lst-1"); return Promise.resolve(ok({ ok: true })); } if (url.startsWith("/api/projects/p1/lists/lst-1/entries/") && init?.method === "DELETE") { @@ -118,4 +132,31 @@ describe("ProjectLists", () => { fireEvent.click(checkbox); await waitFor(() => expect(screen.getByText("Milk").className).toContain("line-through")); }); + + // The rail is what the user reads to know their list exists. Both of these + // failed before the fix: refreshLists() fetched and threw the result away + // (only the mount effect ever called setLists), so a created list never + // appeared and a deleted one never left. + it("a created list appears in the rail", async () => { + vi.stubGlobal("prompt", vi.fn(() => "New list")); + await act(async () => { + render(); + }); + await act(async () => { + fireEvent.click(screen.getByLabelText("Create new list")); + }); + await waitFor(() => expect(screen.getAllByText("New list").length).toBeGreaterThanOrEqual(1)); + }); + + it("a deleted list disappears from the rail", async () => { + vi.stubGlobal("confirm", vi.fn(() => true)); + await act(async () => { + render(); + }); + expect(screen.getAllByText("Shopping").length).toBeGreaterThanOrEqual(1); + await act(async () => { + fireEvent.click(screen.getByLabelText("Delete Shopping")); + }); + await waitFor(() => expect(screen.queryByText("Shopping")).not.toBeInTheDocument()); + }); }); From f8cf50e3d374391eba35c299f8398d04ae54434b Mon Sep 17 00:00:00 2001 From: jaylfc Date: Wed, 12 Aug 2026 23:51:52 +0000 Subject: [PATCH 3/4] fix(projects): drop the previous project's list selection on switch Two review findings, both reachable and both proven red first. ProjectWorkspace renders with no key and is itself rendered unkeyed, so switching project reuses this component instead of remounting it. selectedListId survived the switch and the entries effect then fetched /api/projects//lists//entries, showing the previous project's entries against the new project. Resetting in an effect is not enough: the entries effect re-runs in the same commit because its deps include project.id, so it fires with the stale id before any effect-based reset lands. The selection is now dropped during render (React's adjust-state-on-prop-change pattern), which runs before effects. The lists effect additionally keeps a selection only if it belongs to the lists just fetched. createList trimmed the title only after the empty check, so a whitespace-only answer to the prompt reached the API as an empty title. Docs-Reviewed: no user-visible behaviour beyond the Lists tab already described in changelog.d/2379-projects-lists-tab.md, and no desktop app was added or removed. --- desktop/src/apps/ProjectsApp/ProjectLists.tsx | 29 +++++++++++++-- .../__tests__/ProjectLists.test.tsx | 35 +++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/desktop/src/apps/ProjectsApp/ProjectLists.tsx b/desktop/src/apps/ProjectsApp/ProjectLists.tsx index 49f82306d..529948aee 100644 --- a/desktop/src/apps/ProjectsApp/ProjectLists.tsx +++ b/desktop/src/apps/ProjectsApp/ProjectLists.tsx @@ -21,6 +21,20 @@ export function ProjectLists({ project }: { project: Project }) { const [submitting, setSubmitting] = useState(false); const quickInputRef = useRef(null); + // Drop the previous project's selection DURING RENDER, not in an effect. + // ProjectWorkspace renders unkeyed and is + // itself unkeyed, so a project switch reuses this component. Resetting in an + // effect is too late: the entries effect re-runs in the same commit (its deps + // include project.id) and would fetch + // /api/projects//lists//entries first. This is React's + // adjust-state-on-prop-change pattern and it runs before any effect. + const [prevProjectId, setPrevProjectId] = useState(project.id); + if (prevProjectId !== project.id) { + setPrevProjectId(project.id); + setSelectedListId(null); + setEntries([]); + } + const refreshLists = useCallback(() => { // Sets state, like refreshEntries below. It used to only RETURN the lists, // so createList/deleteList awaited a fetch whose result was discarded -- a @@ -58,7 +72,14 @@ export function ProjectLists({ project }: { project: Project }) { .then((ls) => { if (!cancelled) { setLists(ls); - if (!selectedListId && ls.length > 0) setSelectedListId(ls[0]!.id); + // Keep the selection only if it belongs to THIS project's lists. + // ProjectWorkspace renders with no + // key and is itself unkeyed, so switching project does not remount: + // the previous project's selectedListId survived and the entries + // effect then fetched /api/projects//lists//entries. + // The functional form also avoids depending on a stale closure value. + setSelectedListId((prev) => + prev && ls.some((l) => l.id === prev) ? prev : (ls[0]?.id ?? null)); } }) .catch(() => { @@ -79,10 +100,12 @@ export function ProjectLists({ project }: { project: Project }) { }, [selectedListId, refreshEntries]); const createList = async () => { - const title = prompt("New list name"); + // Trim BEFORE the empty check: " " is truthy, so a whitespace-only + // answer used to reach the API as an empty title. + const title = prompt("New list name")?.trim(); if (!title) return; try { - const created = await projectsApi.lists.create(project.id, { title: title.trim() }); + const created = await projectsApi.lists.create(project.id, { title }); await refreshLists(); setSelectedListId(created.id); } catch (err) { diff --git a/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx b/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx index bd7e46418..506bd9272 100644 --- a/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx +++ b/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx @@ -35,6 +35,12 @@ describe("ProjectLists", () => { ]; fetchMock = vi.fn((url: string, init?: RequestInit) => { + if (url === "/api/projects/p2/lists") { + return Promise.resolve(ok({ items: [{ id: "lst-p2", project_id: "p2", title: "Other project list", description: "", status: "active", created_by: "u1", created_at: 0, updated_at: 0 }] })); + } + if (url === "/api/projects/p2/lists/lst-p2/entries") { + return Promise.resolve(ok({ items: [] })); + } if (url === "/api/projects/p1/lists") { if (init?.method === "POST") { // The mock has to behave like the server: a created list is in the @@ -148,6 +154,35 @@ describe("ProjectLists", () => { await waitFor(() => expect(screen.getAllByText("New list").length).toBeGreaterThanOrEqual(1)); }); + it("switching project does not keep the previous project's selected list", async () => { + // Neither ProjectWorkspace nor ProjectLists is keyed by project, so the + // component is reused across a switch and the old selection survived. + let view: ReturnType; + await act(async () => { + view = render(); + }); + await act(async () => { + view!.rerender(); + }); + await waitFor(() => expect(screen.getAllByText("Other project list").length).toBeGreaterThanOrEqual(1)); + // the old project's entries must not still be on screen + expect(screen.queryByText("Milk")).not.toBeInTheDocument(); + const badFetch = fetchMock.mock.calls.find(([u]) => String(u) === "/api/projects/p2/lists/lst-1/entries"); + expect(badFetch).toBeUndefined(); + }); + + it("a whitespace-only list name creates nothing", async () => { + vi.stubGlobal("prompt", vi.fn(() => " ")); + await act(async () => { + render(); + }); + await act(async () => { + fireEvent.click(screen.getByLabelText("Create new list")); + }); + const posted = fetchMock.mock.calls.find(([u, i]) => String(u) === "/api/projects/p1/lists" && (i as RequestInit | undefined)?.method === "POST"); + expect(posted).toBeUndefined(); + }); + it("a deleted list disappears from the rail", async () => { vi.stubGlobal("confirm", vi.fn(() => true)); await act(async () => { From 95465c0538ea981c9e43a3e1a831f20c26dfebdf Mon Sep 17 00:00:00 2001 From: jaylfc Date: Wed, 12 Aug 2026 23:54:45 +0000 Subject: [PATCH 4/4] fix(projects): ignore an entries response for a deselected list Both bots flagged this independently. refreshEntries wrote whatever came back, so two quick list clicks raced and the slower response landed last, showing one list's entries under another's heading. The response is now dropped unless its list is still the selected one, compared against a ref because the closure's own copy is stale by definition. Covered by a deterministic race test: lst-1's fetch is held open, the user selects lst-2, then lst-1's response is released. It fails without the guard with "STALE MILK" on screen. The created-list assertion is now scoped to the rail. It searched the whole document, and the entries header renders the selected list's title too, so it could have passed while the rail stayed stale. Docs-Reviewed: behaviour is already covered by changelog.d/2379-projects-lists-tab.md and no desktop app was added or removed. --- desktop/src/apps/ProjectsApp/ProjectLists.tsx | 12 ++++- .../__tests__/ProjectLists.test.tsx | 47 ++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/desktop/src/apps/ProjectsApp/ProjectLists.tsx b/desktop/src/apps/ProjectsApp/ProjectLists.tsx index 529948aee..20265f5f6 100644 --- a/desktop/src/apps/ProjectsApp/ProjectLists.tsx +++ b/desktop/src/apps/ProjectsApp/ProjectLists.tsx @@ -20,6 +20,10 @@ export function ProjectLists({ project }: { project: Project }) { const [quickText, setQuickText] = useState(""); const [submitting, setSubmitting] = useState(false); const quickInputRef = useRef(null); + // Current selection, readable from inside an async closure that captured an + // older one. Assigned during render so it is correct before any effect runs. + const selectedListIdRef = useRef(selectedListId); + selectedListIdRef.current = selectedListId; // Drop the previous project's selection DURING RENDER, not in an effect. // ProjectWorkspace renders unkeyed and is @@ -53,8 +57,14 @@ export function ProjectLists({ project }: { project: Project }) { const refreshEntries = useCallback(() => { if (!selectedListId) return Promise.resolve([] as ProjectListEntry[]); - return projectsApi.lists.entries.list(project.id, selectedListId) + const forListId = selectedListId; + return projectsApi.lists.entries.list(project.id, forListId) .then((ents) => { + // Drop a response whose list is no longer selected. Two quick clicks + // race, and the slower fetch used to land last and show one list's + // entries under another's heading. The ref is the CURRENT selection; + // this closure's own copy is stale by definition. + if (forListId !== selectedListIdRef.current) return ents; setEntries(ents); return ents; }) diff --git a/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx b/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx index 506bd9272..256448047 100644 --- a/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx +++ b/desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, act, fireEvent, waitFor } from "@testing-library/react"; +import { render, screen, act, fireEvent, waitFor, within } from "@testing-library/react"; import type { Project } from "@/lib/projects"; import { ProjectLists } from "../ProjectLists"; @@ -151,7 +151,12 @@ describe("ProjectLists", () => { await act(async () => { fireEvent.click(screen.getByLabelText("Create new list")); }); - await waitFor(() => expect(screen.getAllByText("New list").length).toBeGreaterThanOrEqual(1)); + // Scope to the rail: the entries header renders the selected list's title + // too, so a whole-document match could pass while the rail stayed stale. + await waitFor(() => { + const rail = screen.getByLabelText("Project lists"); + expect(within(rail).getByText("New list")).toBeInTheDocument(); + }); }); it("switching project does not keep the previous project's selected list", async () => { @@ -183,6 +188,44 @@ describe("ProjectLists", () => { expect(posted).toBeUndefined(); }); + it("a slow entries response for a deselected list does not overwrite the current one", async () => { + // Deterministic race: lst-1's fetch is held open, the user picks lst-2, + // then lst-1's response lands last. Without the guard it wins and shows + // one list's entries under the other's heading. + let releaseSlow: (() => void) | null = null; + const slow = new Promise((res) => { releaseSlow = () => res(); }); + const raceFetch = vi.fn((url: string) => { + if (url === "/api/projects/p1/lists") { + return Promise.resolve(ok({ items: [ + { id: "lst-1", project_id: "p1", title: "Shopping", description: "", status: "active", created_by: "u1", created_at: 0, updated_at: 0 }, + { id: "lst-2", project_id: "p1", title: "Chores", description: "", status: "active", created_by: "u1", created_at: 0, updated_at: 0 }, + ] })); + } + if (url === "/api/projects/p1/lists/lst-1/entries") { + return slow.then(() => ok({ items: [{ id: "e-slow", list_id: "lst-1", project_id: "p1", text: "STALE MILK", original_text: "STALE MILK", category: null, status: "new", done: 0, author_kind: "user", author_id: "u1", edited_by: null, position: 0, created_at: 0, updated_at: 0 }] })); + } + if (url === "/api/projects/p1/lists/lst-2/entries") { + return Promise.resolve(ok({ items: [{ id: "e-fast", list_id: "lst-2", project_id: "p1", text: "Sweep floor", original_text: "Sweep floor", category: null, status: "new", done: 0, author_kind: "user", author_id: "u1", edited_by: null, position: 0, created_at: 0, updated_at: 0 }] })); + } + return Promise.resolve(ok({ items: [] })); + }); + vi.stubGlobal("fetch", raceFetch); + + await act(async () => { + render(); + }); + await act(async () => { + fireEvent.click(screen.getByText("Chores")); + }); + await waitFor(() => expect(screen.getByText("Sweep floor")).toBeInTheDocument()); + await act(async () => { + releaseSlow!(); + await slow; + }); + expect(screen.queryByText("STALE MILK")).not.toBeInTheDocument(); + expect(screen.getByText("Sweep floor")).toBeInTheDocument(); + }); + it("a deleted list disappears from the rail", async () => { vi.stubGlobal("confirm", vi.fn(() => true)); await act(async () => {