diff --git a/app/src/components/notes/NotesPanel.shared.test.tsx b/app/src/components/notes/NotesPanel.shared.test.tsx new file mode 100644 index 0000000..fc459ce --- /dev/null +++ b/app/src/components/notes/NotesPanel.shared.test.tsx @@ -0,0 +1,167 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, within, fireEvent, waitFor } from "@testing-library/react"; +import NotesPanel from "./NotesPanel"; +import { useAppState } from "../../store/appState"; +import type { Note } from "../../lib/types"; + +/** + * Two panels, one project — the configuration the app actually runs in. + * + * `NotesTab` and `NotesDock` both mount a `NotesPanel`, and the dock follows + * the active tab's project, so opening the dock over a Project Home tab mounts + * two panels for the *same* project. Every other notes test mounts exactly + * one, which is precisely the configuration in which a per-panel cache looks + * correct: it is only with two that an edit made in one is seen — or lost — by + * the other. `useNotes` is deliberately **not** mocked here; the cache is what + * is under test. + */ + +const files: Record = {}; + +vi.mock("../../lib/tauri-commands", () => ({ + listNotes: async (p: string) => [...(files[p] ?? [])], + saveNote: async (p: string, n: Note) => { + const list = files[p] ?? (files[p] = []); + const at = list.findIndex((x) => x.id === n.id); + if (at === -1) list.unshift(n); + else list[at] = n; + return n; + }, + deleteNote: async (p: string, id: string) => { + files[p] = (files[p] ?? []).filter((x) => x.id !== id); + }, +})); + +vi.mock("./SendToAgentButton", () => ({ + default: () => , +})); + +const note = (over: Partial = {}): Note => ({ + id: "n1", + title: "Deploy steps", + body: "one", + pinned: false, + created_at: "2026-09-01T00:00:00Z", + updated_at: "2026-09-01T00:00:00Z", + ...over, +}); + +/** The tab and the dock, mounted together the way `App` mounts them. */ +function renderBothSurfaces() { + render( + <> +
+ +
+
+ +
+ , + ); + return { + tab: () => within(screen.getByTestId("tab")), + dock: () => within(screen.getByTestId("dock")), + }; +} + +beforeEach(() => { + for (const key of Object.keys(files)) delete files[key]; + files.p1 = [note()]; + useAppState.setState({ notesByProject: {}, notesLoading: {}, toasts: [] }); +}); + +describe("NotesPanel with the tab and the dock both open", () => { + it("shows an edit made in one surface in the other", async () => { + const { tab, dock } = renderBothSurfaces(); + await waitFor(() => expect(tab().getByLabelText("Note body")).toHaveValue("one")); + + const dockTitle = dock().getByLabelText("Note title"); + fireEvent.change(dockTitle, { target: { value: "Deploy steps v2" } }); + fireEvent.blur(dockTitle); + + // The other surface's list *and* its editor, not just one of them. + await waitFor(() => + expect(tab().getByRole("button", { name: /deploy steps v2/i })).toBeInTheDocument(), + ); + expect(tab().getByLabelText("Note title")).toHaveValue("Deploy steps v2"); + }); + + it("does not write one surface's stale copy over the other's edit", async () => { + // The reported repro: edit in the dock, then go back to the tab and edit + // there. With a cache per panel, the tab committed `{...staleNote, ...}` + // and the dock's edit was gone from disk with no error and no indicator. + const { tab, dock } = renderBothSurfaces(); + await waitFor(() => expect(tab().getByLabelText("Note body")).toHaveValue("one")); + + const dockTitle = dock().getByLabelText("Note title"); + fireEvent.change(dockTitle, { target: { value: "Deploy steps v2" } }); + fireEvent.blur(dockTitle); + await waitFor(() => expect(files.p1[0].title).toBe("Deploy steps v2")); + + const tabBody = tab().getByLabelText("Note body"); + fireEvent.change(tabBody, { target: { value: "two" } }); + fireEvent.blur(tabBody); + + await waitFor(() => expect(files.p1[0].body).toBe("two")); + expect(files.p1).toHaveLength(1); + expect(files.p1[0].title).toBe("Deploy steps v2"); + }); + + it("reads the project once for both surfaces", async () => { + // Two panels are two `useNotes`, but the in-flight flag is per project, so + // mounting the dock over an open Notes tab does not re-read the file. + const listNotes = vi.spyOn( + await import("../../lib/tauri-commands"), + "listNotes", + ); + renderBothSurfaces(); + await waitFor(() => + expect(screen.getAllByLabelText("Note body")[0]).toHaveValue("one"), + ); + expect(listNotes).toHaveBeenCalledTimes(1); + listNotes.mockRestore(); + }); + + it("keeps text the user is part-way through typing when the other surface saves", async () => { + // Showing a remote edit must never mean discarding an unsaved local one. + const { tab, dock } = renderBothSurfaces(); + await waitFor(() => expect(tab().getByLabelText("Note body")).toHaveValue("one")); + + const tabBody = tab().getByLabelText("Note body"); + fireEvent.change(tabBody, { target: { value: "half-typed" } }); + + const dockBody = dock().getByLabelText("Note body"); + fireEvent.change(dockBody, { target: { value: "saved in the dock" } }); + fireEvent.blur(dockBody); + await waitFor(() => expect(files.p1[0].body).toBe("saved in the dock")); + + expect(tabBody).toHaveValue("half-typed"); + }); + + it("falls back to another note when the selected one is deleted", async () => { + // The claim a differently-named test in NotesPanel.test.tsx used to make + // and could not keep: `useNotes` is mocked there and its list never + // changes, so the fallback was invisible. Here the list is real. + files.p1 = [note(), note({ id: "n2", title: "Gotchas", body: "beware" })]; + const { tab } = renderBothSurfaces(); + await waitFor(() => expect(tab().getByLabelText("Note body")).toHaveValue("one")); + + fireEvent.click(tab().getByRole("button", { name: /delete note/i })); + + await waitFor(() => expect(tab().getByLabelText("Note body")).toHaveValue("beware")); + expect(tab().queryByRole("button", { name: /deploy steps/i })).not.toBeInTheDocument(); + expect(files.p1).toHaveLength(1); + }); + + it("shows a note created in one surface in the other", async () => { + const { tab, dock } = renderBothSurfaces(); + await waitFor(() => expect(tab().getByLabelText("Note body")).toHaveValue("one")); + + fireEvent.click(dock().getByRole("button", { name: /new note/i })); + + await waitFor(() => + expect(tab().getAllByRole("button", { name: /untitled note/i })).toHaveLength(1), + ); + expect(files.p1).toHaveLength(2); + }); +}); diff --git a/app/src/components/notes/NotesPanel.tsx b/app/src/components/notes/NotesPanel.tsx index d75ab42..f4953b8 100644 --- a/app/src/components/notes/NotesPanel.tsx +++ b/app/src/components/notes/NotesPanel.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useNotes } from "../../hooks/useNotes"; import NoteEditor from "./NoteEditor"; import Button from "../ui/Button"; @@ -30,22 +30,44 @@ export default function NotesPanel({ projectId }: Props) { [notes, selectedId], ); - // Load the selected note's stored text into the draft. Keyed on the id, not - // the note object, so a save round trip does not stomp what is being typed. + // What was last copied out of the store into the draft fields. The draft is + // "untouched" exactly while it still matches this, which is how an edit made + // somewhere else can be shown without ever discarding something half-typed. + const seeded = useRef<{ id: string | null; title: string; body: string }>({ + id: null, + title: "", + body: "", + }); + + // Load the selected note's stored text into the draft — on a change of note, + // and on a change to the *stored* text of the note already selected. The + // second case is the dock and the tab showing one project at once: an edit + // committed in one surface has to reach the other's editor, not just its + // list. It never overwrites text the user is part-way through typing; that + // blurs into a last-writer-wins save, as any blur-commit editor does. useEffect(() => { if (!selected) { + seeded.current = { id: null, title: "", body: "" }; setTitle(""); setBody(""); return; } - setTitle(selected.title); - setBody(selected.body); - }, [selected?.id]); // eslint-disable-line react-hooks/exhaustive-deps + const untouched = + title === seeded.current.title && body === seeded.current.body; + if (seeded.current.id !== selected.id || untouched) { + seeded.current = { id: selected.id, title: selected.title, body: selected.body }; + setTitle(selected.title); + setBody(selected.body); + } + }, [selected?.id, selected?.title, selected?.body]); // eslint-disable-line react-hooks/exhaustive-deps const commit = () => { if (!selected) return; // Reading is not editing: clicking through notes must not rewrite the file. if (title === selected.title && body === selected.body) return; + // Mark the draft as matching what was just committed, so the store update + // this save produces reads as "no change" rather than as a stale re-seed. + seeded.current = { id: selected.id, title, body }; void saveNote({ ...selected, title, body }); }; diff --git a/app/src/hooks/useNotes.test.ts b/app/src/hooks/useNotes.test.ts index ab9445c..a7a7182 100644 --- a/app/src/hooks/useNotes.test.ts +++ b/app/src/hooks/useNotes.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { renderHook, act, waitFor } from "@testing-library/react"; import { useNotes } from "./useNotes"; +import { useAppState } from "../store/appState"; import type { Note } from "../lib/types"; const listNotes = vi.fn(); @@ -13,14 +14,6 @@ vi.mock("../lib/tauri-commands", () => ({ deleteNote: (p: string, id: string) => deleteNote(p, id), })); -const pushToast = vi.fn(); -vi.mock("../store/appState", () => ({ - useAppState: Object.assign( - (selector: (s: unknown) => unknown) => selector({ pushToast }), - { getState: () => ({ pushToast }) }, - ), -})); - const note = (over: Partial = {}): Note => ({ id: "n1", title: "Deploy", @@ -31,8 +24,35 @@ const note = (over: Partial = {}): Note => ({ ...over, }); +/** The toasts the hook pushed. The store is real, so this is what a user sees. */ +const toasts = () => useAppState.getState().toasts; + +/** + * A stand-in for the Rust store: one list per project, upsert and delete + * applied to it, `list_notes` reading it back. Several of these tests are about + * what the *list* looks like after a sequence of writes, which a per-call + * `mockResolvedValueOnce` cannot express. + */ +function fakeBackend(initial: Record = {}) { + const files: Record = { ...initial }; + listNotes.mockImplementation(async (p: string) => [...(files[p] ?? [])]); + saveNote.mockImplementation(async (p: string, n: Note) => { + const list = files[p] ?? (files[p] = []); + const at = list.findIndex((x) => x.id === n.id); + if (at === -1) list.unshift(n); + else list[at] = n; + return n; + }); + deleteNote.mockImplementation(async (p: string, id: string) => { + files[p] = (files[p] ?? []).filter((x) => x.id !== id); + }); + return files; +} + beforeEach(() => { vi.clearAllMocks(); + // The cache is shared app state now, so it has to be reset like any other. + useAppState.setState({ notesByProject: {}, notesLoading: {}, toasts: [] }); listNotes.mockResolvedValue([note()]); saveNote.mockImplementation(async (_p: string, n: Note) => n); deleteNote.mockResolvedValue(undefined); @@ -60,7 +80,7 @@ describe("useNotes", () => { expect(ok).toBe(false); expect(result.current.saveState.status).toBe("failed"); - expect(pushToast).toHaveBeenCalled(); + expect(toasts()).toHaveLength(1); }); it("replaces the saved note in place rather than appending", async () => { @@ -128,7 +148,7 @@ describe("useNotes", () => { await waitFor(() => expect(result.current.loading).toBe(false)); expect(result.current.notes).toHaveLength(0); - expect(pushToast).toHaveBeenCalled(); + expect(toasts()).toHaveLength(1); }); it("ends with the list the backend returned when saving a new note", async () => { @@ -206,4 +226,147 @@ describe("useNotes", () => { expect(result.current.notes).toHaveLength(1); expect(result.current.notes[0].id).toBe("n2"); }); + + it("keeps the notes already on screen when a refresh fails", async () => { + // The second surface mounting for a project is a refresh behind a list the + // user is already reading. One shared cache means a failed refresh would + // otherwise blank both panels. + fakeBackend({ p1: [note()] }); + const tab = renderHook(() => useNotes("p1")); + await waitFor(() => expect(tab.result.current.loading).toBe(false)); + + listNotes.mockRejectedValueOnce(new Error("read failed")); + const dock = renderHook(() => useNotes("p1")); + await waitFor(() => expect(toasts()).toHaveLength(1)); + + expect(tab.result.current.notes).toHaveLength(1); + expect(dock.result.current.notes).toHaveLength(1); + }); + + it("does not report the old project's save on the new project's indicator", async () => { + // The indicator is per-panel and reads "Saved ✓". Firing it after a switch + // tells the user their *current* project was written when it was not. + const { result, rerender } = renderHook( + ({ projectId }: { projectId: string }) => useNotes(projectId), + { initialProps: { projectId: "p1" } }, + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + + let resolveSave: ((n: Note) => void) | undefined; + saveNote.mockImplementationOnce( + () => new Promise((resolve) => (resolveSave = resolve)), + ); + let savePromise: Promise | undefined; + await act(async () => { + savePromise = result.current.saveNote(note({ body: "edited" })); + }); + + rerender({ projectId: "p2" }); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { + resolveSave?.(note({ body: "edited" })); + await savePromise; + }); + + expect(result.current.saveState.status).toBe("idle"); + }); + + it("still reports a save on the indicator of the project it was made for", async () => { + const { result } = renderHook(() => useNotes("p1")); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { + await result.current.saveNote(note({ body: "edited" })); + }); + + expect(result.current.saveState.status).toBe("saved"); + }); + + it("serialises a project's writes so an edit cannot be re-inserted after its delete", async () => { + // Clicking Delete while the textarea has focus fires blur first, so a save + // and a delete go out back to back. The Rust write lock stops them + // interleaving but does not order them: a delete that wins the lock is + // undone by the upsert behind it, and the note comes back on next load. + const files = fakeBackend({ p1: [note()] }); + const { result } = renderHook(() => useNotes("p1")); + await waitFor(() => expect(result.current.loading).toBe(false)); + + const order: string[] = []; + saveNote.mockImplementationOnce(async (p: string, n: Note) => { + await new Promise((r) => setTimeout(r, 20)); + order.push("save"); + files[p] = [n]; + return n; + }); + deleteNote.mockImplementationOnce(async (p: string, id: string) => { + order.push("delete"); + files[p] = (files[p] ?? []).filter((x) => x.id !== id); + }); + + await act(async () => { + const save = result.current.saveNote(note({ body: "typo fixed" })); + const del = result.current.deleteNote("n1"); + await Promise.all([save, del]); + }); + + expect(order).toEqual(["save", "delete"]); + expect(files.p1).toHaveLength(0); + expect(result.current.notes).toHaveLength(0); + }); + + it("keeps a new note when another note is saved right after it", async () => { + // A purely local draft used to be wiped by the next re-read: two clicks of + // "New note", type in the second, blur, and the first row was gone. + const files = fakeBackend({ p1: [note()] }); + const { result } = renderHook(() => useNotes("p1")); + await waitFor(() => expect(result.current.loading).toBe(false)); + + let first: Note | null = null; + await act(async () => { + first = await result.current.createNote(); + await result.current.createNote(); + }); + expect(result.current.notes).toHaveLength(3); + + await act(async () => { + await result.current.saveNote(note({ body: "edited" })); + }); + + expect(result.current.notes).toHaveLength(3); + expect(result.current.notes.some((n) => n.id === first!.id)).toBe(true); + expect(files.p1).toHaveLength(3); + }); + + it("shares one cache between every hook watching the same project", async () => { + // The Project Home sub-tab and the dock both mount a panel for the same + // project. Two caches meant an edit in one was invisible to the other, and + // the other's next blur wrote its stale copy back over it. + fakeBackend({ p1: [note()] }); + const tab = renderHook(() => useNotes("p1")); + const dock = renderHook(() => useNotes("p1")); + await waitFor(() => expect(tab.result.current.loading).toBe(false)); + await waitFor(() => expect(dock.result.current.loading).toBe(false)); + + // One read for both — the in-flight flag is per project, not per hook. + expect(listNotes).toHaveBeenCalledTimes(1); + + await act(async () => { + await dock.result.current.saveNote(note({ body: "written in the dock" })); + }); + + expect(tab.result.current.notes[0].body).toBe("written in the dock"); + expect(tab.result.current.notes).toBe(dock.result.current.notes); + }); + + it("does not blank an already-loaded list when a second panel mounts", async () => { + fakeBackend({ p1: [note()] }); + const tab = renderHook(() => useNotes("p1")); + await waitFor(() => expect(tab.result.current.loading).toBe(false)); + + const dock = renderHook(() => useNotes("p1")); + // No "Loading notes…" flash on the second surface. + expect(dock.result.current.loading).toBe(false); + expect(dock.result.current.notes).toHaveLength(1); + }); }); diff --git a/app/src/hooks/useNotes.ts b/app/src/hooks/useNotes.ts index 4d99b21..07cac3c 100644 --- a/app/src/hooks/useNotes.ts +++ b/app/src/hooks/useNotes.ts @@ -8,8 +8,8 @@ import { useAppState } from "../store/appState"; function draft(): Note { const now = new Date().toISOString(); return { - // The backend owns the real id; this one only has to be unique enough to - // key the list until the first save returns. + // The backend keeps whatever id it is handed for a note it has not seen, + // so this one is the note's real id from the first save onward. id: crypto.randomUUID(), title: "", body: "", @@ -19,41 +19,108 @@ function draft(): Note { }; } +/** Stable empty list, so a project with nothing cached does not re-render on identity. */ +const NO_NOTES: Note[] = []; + +/** + * Per-project mutation chain. + * + * A project's writes are serialised so that two of them cannot be in flight at + * once. The Rust `write_lock` stops an upsert and a delete *interleaving*; it + * does not order them, and the order is the part that matters here. Clicking + * Delete while the textarea has focus fires `blur` first, so `save_note` and + * `delete_note` are issued back to back — and if the delete wins the lock, the + * upsert behind it re-inserts the note and it comes back on the next load. + * "Fix a typo, decide the note is useless, delete it" is an ordinary sequence. + * + * Module scope, not hook scope, for the reason `useTerminal`'s input queue is: + * several components call `useNotes` for the same project (the Project Home + * tab and the dock), and a per-hook chain would give each its own ordering and + * leave them racing each other — which is the bug, not the fix. + */ +const mutationChains = new Map>(); + +function enqueueMutation(projectId: string, run: () => Promise): Promise { + const previous = mutationChains.get(projectId) ?? Promise.resolve(); + // `run` on both arms: a failed mutation must not stall every later one. + const result = previous.then(run, run); + const tail = result.then( + () => {}, + () => {}, + ); + mutationChains.set(projectId, tail); + void tail.then(() => { + // Drop the entry once idle, so closed projects do not accumulate. + if (mutationChains.get(projectId) === tail) mutationChains.delete(projectId); + }); + return result; +} + +/** + * Re-read the canonical list into the shared cache. + * + * A successful save stamps a new `updated_at` and the backend sorts on it, so + * the record's position has changed and positional patching would disagree + * with what a reload would show. The backend owns the order; the webview never + * sorts. A failed re-read leaves the cache alone rather than clearing it. + */ +async function refresh(projectId: string): Promise { + try { + const reloaded = await commands.listNotes(projectId); + useAppState.getState().setProjectNotes(projectId, reloaded); + return true; + } catch { + return false; + } +} + /** * A project's notes, cached from the backend. * - * The backend is the source of truth and this is a cache — every mutation goes - * through a command and the returned record replaces the local one, so the - * list can never drift from the file. `saveState` mirrors `useProjectSave` so - * `ui/SaveIndicator` can report the outcome: a save that fails silently is a - * user staring at text they believe is stored. + * The backend is the source of truth and the zustand slice is the cache — + * every mutation goes through a command and the returned list replaces the + * cached one, so the list can never drift from the file. The cache lives in + * the store rather than in this hook because two surfaces show the same + * project's notes at once; see `notesByProject`. + * + * `saveState` is deliberately *not* shared: it is this panel's report of this + * panel's write, and `ui/SaveIndicator` is per-panel. A save that fails + * silently is a user staring at text they believe is stored. */ export function useNotes(projectId: string) { - const [notes, setNotes] = useState([]); - const [loading, setLoading] = useState(true); - const [saveState, setSaveState] = useState({ status: "idle", error: null }); + const cached = useAppState((s) => s.notesByProject[projectId]); const pushToast = useAppState((s) => s.pushToast); + const [saveState, setSaveState] = useState({ status: "idle", error: null }); const resetTimer = useRef | null>(null); const currentProjectId = useRef(projectId); currentProjectId.current = projectId; useEffect(() => { - if (!projectId) { - setNotes([]); - setLoading(false); - return; - } - let cancelled = false; - setLoading(true); - setNotes([]); + if (!projectId) return; + // Read through `getState` rather than through subscribed values: the + // effect must fire once per project, not again every time the flag it sets + // changes. Two panels mounting for the same project therefore make one + // read, and the second renders from the cache with no loading flash. + const store = useAppState.getState(); + if (store.notesLoading[projectId]) return; + store.setNotesLoading(projectId, true); commands .listNotes(projectId) .then((loaded) => { - if (!cancelled) setNotes(loaded); + useAppState.getState().setProjectNotes(projectId, loaded); }) .catch((e) => { - if (cancelled) return; - setNotes([]); + // A project that has never been read caches the empty list, so a panel + // does not sit on "Loading notes…" forever. One that *has* been read + // keeps what it has: this load is a refresh behind a list already on + // screen — the second surface mounting, say — and a failed refresh + // must not blank both of them. Same rule as `refresh()`. Neither + // branch has a stale-project hazard, because the write is keyed by the + // project it belongs to. + const store = useAppState.getState(); + if (store.notesByProject[projectId] === undefined) { + store.setProjectNotes(projectId, []); + } pushToast({ kind: "error", message: "Could not load notes for this project", @@ -61,11 +128,8 @@ export function useNotes(projectId: string) { }); }) .finally(() => { - if (!cancelled) setLoading(false); + useAppState.getState().setNotesLoading(projectId, false); }); - return () => { - cancelled = true; - }; }, [projectId, pushToast]); useEffect( @@ -75,6 +139,27 @@ export function useNotes(projectId: string) { [], ); + // The indicator belongs to whatever project this panel is showing *now*. + // Without this, switching project mid-save leaves the new project's + // SaveIndicator stuck on the old project's "Saving…" — the same wrong-project + // report as flashing its "Saved ✓", just in the other direction. + useEffect(() => { + if (resetTimer.current) clearTimeout(resetTimer.current); + setSaveState({ status: "idle", error: null }); + }, [projectId]); + + /** + * Whether this hook is still looking at the project a queued mutation was + * issued for. Only the *reporting* is gated on it — the cache write is not, + * because it is keyed by project and belongs to that project either way. + * Without this, the new project's SaveIndicator flashes "Saved ✓" for the + * old project's write. + */ + const isCurrent = useCallback( + () => currentProjectId.current === projectId, + [projectId], + ); + const succeeded = useCallback(() => { setSaveState({ status: "saved", error: null }); if (resetTimer.current) clearTimeout(resetTimer.current); @@ -85,68 +170,99 @@ export function useNotes(projectId: string) { }, []); const saveNote = useCallback( - async (note: Note) => { - if (resetTimer.current) clearTimeout(resetTimer.current); - setSaveState({ status: "saving", error: null }); - try { - await commands.saveNote(projectId, note); - // Re-read the canonical list from the backend. A successful save stamps a new - // `updated_at`, and the backend sorts unpinned notes by `updated_at` descending, - // so the record's position has changed and positional patching would disagree with - // what a reload would show. - try { - const reloaded = await commands.listNotes(projectId); - // Guard against a stale callback: if the project changed while the save was in - // flight, don't overwrite the new project's list with the old one. The save itself - // still succeeded, so succeeded() still fires; only the list replacement is skipped. - if (currentProjectId.current === projectId) { - setNotes(reloaded); - } - } catch { - // Keep the save reported as successful (it was) and leave the existing list alone - // rather than clearing it if the re-read fails. + (note: Note) => + enqueueMutation(projectId, async () => { + if (isCurrent()) { + if (resetTimer.current) clearTimeout(resetTimer.current); + setSaveState({ status: "saving", error: null }); } - succeeded(); - return true; - } catch (e) { - const message = String(e); - setSaveState({ status: "failed", error: message }); - pushToast({ kind: "error", message: "Could not save note", detail: message }); - return false; - } - }, - [projectId, pushToast, succeeded], + try { + await commands.saveNote(projectId, note); + await refresh(projectId); + if (isCurrent()) succeeded(); + return true; + } catch (e) { + const message = String(e); + if (isCurrent()) setSaveState({ status: "failed", error: message }); + // The toast is not project-scoped — it names the failure and stays + // readable after a switch — so it fires either way. + pushToast({ kind: "error", message: "Could not save note", detail: message }); + return false; + } + }), + [projectId, pushToast, succeeded, isCurrent], ); - const createNote = useCallback(async () => { - const note = draft(); - // Held locally first so the editor can focus it immediately; the save - // happens on blur like every other edit. The note does not exist backend-side, - // so no re-read can place it and no backend ordering applies to it yet. Prepending - // puts it at the top where the user can see it immediately, and on the first save - // its canonical position is established. - setNotes((current) => [note, ...current]); - return note; - }, []); - - const deleteNote = useCallback( - async (noteId: string) => { - try { - await commands.deleteNote(projectId, noteId); - // Guard against a stale callback: if the project changed while the delete was in - // flight, don't modify the list. Note ids are UUIDs so a stale filter cannot match - // another project's note, but applying the same guard for consistency. - if (currentProjectId.current === projectId) { - setNotes((current) => current.filter((n) => n.id !== noteId)); + /** + * Create a note by persisting it, rather than holding it locally until the + * first blur. + * + * The draft used to live only in the list, which meant any *other* note + * being saved replaced the list with the backend's and the unsaved draft + * silently vanished — click "New note" twice, type in the second, blur, and + * the first row is gone. Sharing one cache between two surfaces makes that + * worse rather than better: a local-only row would exist in whichever panel + * created it and nowhere else. Letting the backend own the row from the + * start removes the whole class: there is no such thing as a note in the + * list that the file does not have. + */ + const createNote = useCallback( + () => + enqueueMutation(projectId, async () => { + const note = draft(); + try { + const saved = await commands.saveNote(projectId, note); + if (!(await refresh(projectId))) { + // The note exists; only the re-read failed. Show it rather than + // leaving the user with a button that did nothing visible. + const store = useAppState.getState(); + store.setProjectNotes(projectId, [ + saved, + ...(store.notesByProject[projectId] ?? []), + ]); + } + return saved; + } catch (e) { + pushToast({ + kind: "error", + message: "Could not create note", + detail: String(e), + }); + return null; } - return true; - } catch (e) { - pushToast({ kind: "error", message: "Could not delete note", detail: String(e) }); - return false; - } - }, + }), [projectId, pushToast], ); - return { notes, loading, saveState, createNote, saveNote, deleteNote }; + const deleteNote = useCallback( + (noteId: string) => + enqueueMutation(projectId, async () => { + try { + await commands.deleteNote(projectId, noteId); + const store = useAppState.getState(); + store.setProjectNotes( + projectId, + (store.notesByProject[projectId] ?? []).filter((n) => n.id !== noteId), + ); + return true; + } catch (e) { + pushToast({ kind: "error", message: "Could not delete note", detail: String(e) }); + return false; + } + }), + [projectId, pushToast], + ); + + return { + notes: cached ?? NO_NOTES, + // Only "loading" before the project has ever been read — never on a + // refresh behind a list that is already on screen, and never on the second + // panel to mount for a project the first one already fetched. A failed + // load caches the empty list, so this cannot latch on. + loading: Boolean(projectId) && cached === undefined, + saveState, + createNote, + saveNote, + deleteNote, + }; } diff --git a/app/src/store/appState.ts b/app/src/store/appState.ts index ce0d749..d3771da 100644 --- a/app/src/store/appState.ts +++ b/app/src/store/appState.ts @@ -1,5 +1,12 @@ import { create } from "zustand"; -import type { Project, TerminalSession, AppSettings, UpdateInfo, ImageUpdateInfo } from "../lib/types"; +import type { + Project, + TerminalSession, + AppSettings, + UpdateInfo, + ImageUpdateInfo, + Note, +} from "../lib/types"; const SIDEBAR_COLLAPSED_KEY = "triple-c.sidebar.collapsed"; @@ -146,6 +153,29 @@ interface AppState { /** Nudge the active tab left/right — the keyboard route to the same thing. */ moveActiveTab: (delta: number) => void; + // Per-project notes, cached from the backend. + // + // Rust is the source of truth and this is a cache — but it has to be *one* + // cache. Notes are shown by two surfaces at once (the Project Home sub-tab + // and the dock, which resolves to the same project), and a hook-local + // `useState` in each gave them independent copies: an edit made in the dock + // was invisible to the tab, and the tab's next blur wrote its stale record + // back over it with no error and no indicator. Keyed by project id so a + // response that lands after the user has moved on updates the project it + // belongs to instead of whichever one is on screen. + // + // This is also the boundary a detached notes window would need: swap the + // transport for a `notes-changed` event and both windows feed the same slice. + notesByProject: Record; + /** + * Projects with a `list_notes` in flight, so two panels mounting for the + * same project make one read rather than two, and so a panel whose project + * has never been read can tell "loading" from "no notes". + */ + notesLoading: Record; + setProjectNotes: (projectId: string, notes: Note[]) => void; + setNotesLoading: (projectId: string, loading: boolean) => void; + // Inline container progress, replacing the blocking progress modal. containerProgress: Record; setContainerProgress: (projectId: string, message: string | null) => void; @@ -395,6 +425,22 @@ export const useAppState = create((set) => ({ return { tabOrder }; }), + // Notes + notesByProject: {}, + notesLoading: {}, + setProjectNotes: (projectId, notes) => + set((state) => ({ + notesByProject: { ...state.notesByProject, [projectId]: notes }, + })), + setNotesLoading: (projectId, loading) => + set((state) => { + if ((state.notesLoading[projectId] ?? false) === loading) return {}; + const next = { ...state.notesLoading }; + if (loading) next[projectId] = true; + else delete next[projectId]; + return { notesLoading: next }; + }), + // Container progress containerProgress: {}, setContainerProgress: (projectId, message) =>