diff --git a/DESIGN.md b/DESIGN.md index cfd16aa..70508b5 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -105,7 +105,8 @@ React 19 + TypeScript + Vite + Tailwind 4 (`@tailwindcss/vite`), `@tanstack/reac - **Routes:** `/login`, `/register`, `/gardens` (list), `/gardens/:id` (editor, `?focus=objectId`), `/plants` (catalog). Auth guard on the router root via `/auth/me`. - **State:** TanStack Query for all server state (editor keyed on `gardens/:id/full`; optimistic mutations with version-conflict rollback). One small Zustand store for ephemeral editor state only: viewport, selection, focused object, active tool, in-flight drag. -- **Editor components (`web/src/editor/`):** `GardenCanvas` (svg root + viewport g), `useViewport` (use-gesture pan/zoom/pinch), `ObjectShape`, `PlopMarker` (semantic-zoom branching), `Palette` (drag-to-place object kinds), `Inspector` (side panel; bottom sheet on mobile), `PlantPicker`. Pure geometry helpers (local↔world transforms, unit formatting) in `web/src/lib/geometry.ts`, unit-tested. +- **Editor components (`web/src/editor/`):** `GardenCanvas` (svg root + viewport g), `useViewport` (use-gesture pan/zoom/pinch), `ObjectShape`, `PlopMarker` (semantic-zoom branching), `Palette` (drag-to-place object kinds), `EditorRail` (the one side panel), `Inspector`, `HistoryPanel`, `PlantPicker`. +- **One rail, tabs inside it.** The inspector, history, and later the journal and chat panel all want the same strip of screen; rather than each bolting on its own chrome they are tabs in `EditorRail` — so the canvas is one width instead of a different width per panel, and adding a panel is adding a tab. Selecting an object switches to the Inspector tab automatically, so the rail is never something you operate before you can edit; on a phone the same tabs render in the bottom sheet the inspector already used. Pure geometry helpers (local↔world transforms, unit formatting) in `web/src/lib/geometry.ts`, unit-tested. ## Roadmap diff --git a/web/src/editor/EditorRail.tsx b/web/src/editor/EditorRail.tsx new file mode 100644 index 0000000..2aa76b4 --- /dev/null +++ b/web/src/editor/EditorRail.tsx @@ -0,0 +1,88 @@ +import type { ReactNode } from 'react' +import { cn } from '@/lib/cn' + +/** + * The editor's side rail, and the answer to "four things want one rail". + * + * The inspector, history, and (later) the journal and chat panel all want the + * same strip of screen. Rather than each bolting on its own chrome, they are + * tabs in one rail — which keeps the canvas one width instead of a different + * width per panel, and means adding the journal or chat is adding a tab. + * + * Two constraints shaped it: + * + * - Selecting an object must land you in the inspector with no extra click. + * The editor watches the selection and switches to that tab itself, so the + * rail never becomes a thing you have to operate before you can edit. + * - The canvas has to stay worth watching while the agent edits it, so the + * rail is a fixed 20rem column and closes completely when nothing needs it. + * + * On a phone the same tabs render in a bottom sheet, which is where the + * inspector already lived. + */ + +export interface RailTab { + id: string + label: string + /** Rendered lazily: only the active tab's content is built. */ + render: () => ReactNode + /** Shown as a dot on the tab — a count, or true for a plain marker. */ + badge?: number | boolean +} + +export function EditorRail({ + tabs, + activeId, + onActivate, + onClose, +}: { + tabs: RailTab[] + activeId: string + onActivate: (id: string) => void + onClose: () => void +}) { + const active = tabs.find((t) => t.id === activeId) ?? tabs[0] + if (!active) return null + + return ( +
+
+ {tabs.map((tab) => ( + + ))} + +
+
{active.render()}
+
+ ) +} diff --git a/web/src/editor/HistoryPanel.tsx b/web/src/editor/HistoryPanel.tsx new file mode 100644 index 0000000..b3a43a3 --- /dev/null +++ b/web/src/editor/HistoryPanel.tsx @@ -0,0 +1,131 @@ +import { Alert } from '@/components/ui/Alert' +import { Button } from '@/components/ui/Button' +import { errorMessage } from '@/lib/api' +import { describeCounts, totalChanges, useGardenHistory, useUndo, type ChangeSet } from '@/lib/history' +import { cn } from '@/lib/cn' +import { UndoButton } from './UndoButton' + +/** + * The garden's change history, newest first, with an undo on each entry. + * + * A reverted entry stays in the list, marked — and the revert appears as its own + * entry, because that is what it is. Making the original disappear would be + * rewriting history rather than adding to it, and would leave no way to undo the + * undo. + */ +export function HistoryPanel({ gardenId, canEdit }: { gardenId: number; canEdit: boolean }) { + const history = useGardenHistory(gardenId) + const undo = useUndo(gardenId) + const sets = history.data?.pages.flatMap((p) => p.changeSets) ?? [] + + return ( +
+

History

+ + {history.isPending &&

Loading…

} + {history.isError && {errorMessage(history.error, 'Could not load history.')}} + + {history.isSuccess && sets.length === 0 && ( +

+ Nothing yet. Every change you make here shows up in this list, and can be undone from it. +

+ )} + +
    + {sets.map((cs) => ( + + ))} +
+ + {history.hasNextPage && ( + + )} + + {/* Said plainly rather than discovered: deleting a garden bypasses this + list entirely, because the delete cascades below the layer that records + changes. Better to state the gap than to imply cover we don't have. */} +

+ Deleting a whole garden isn't covered here and can't be undone. +

+
+ ) +} + +function HistoryEntry({ + changeSet, + canEdit, + undo, +}: { + changeSet: ChangeSet + canEdit: boolean + undo: ReturnType +}) { + const counts = describeCounts(changeSet) + const reverted = changeSet.revertedById != null + const isRevert = changeSet.revertsId != null + + return ( +
  • +
    +
    +

    {changeSet.summary}

    +

    + {changeSet.source === 'agent' && ( + + agent + + )} + {isRevert && undo} + {changeSet.actorName} + · + + {counts && ( + <> + · + {counts} + + )} +

    + {reverted &&

    Undone

    } +
    + + {canEdit && totalChanges(changeSet) > 0 && ( + + )} +
    +
    • + ) +} + +/** A compact "3m ago" / "yesterday" for a UTC timestamp. */ +export function relativeTime(iso: string): string { + const then = Date.parse(iso) + if (Number.isNaN(then)) return iso + const seconds = Math.round((Date.now() - then) / 1000) + if (seconds < 60) return 'just now' + const minutes = Math.round(seconds / 60) + if (minutes < 60) return `${minutes}m ago` + const hours = Math.round(minutes / 60) + if (hours < 24) return `${hours}h ago` + const days = Math.round(hours / 24) + if (days === 1) return 'yesterday' + if (days < 30) return `${days}d ago` + return new Date(then).toLocaleDateString() +} diff --git a/web/src/editor/UndoButton.tsx b/web/src/editor/UndoButton.tsx new file mode 100644 index 0000000..f97ad66 --- /dev/null +++ b/web/src/editor/UndoButton.tsx @@ -0,0 +1,51 @@ +import { Button } from '@/components/ui/Button' +import { cn } from '@/lib/cn' +import type { ChangeSet, UndoOutcome, useUndo } from '@/lib/history' + +/** + * Undo, plus what happened. Paired with useUndo so the history list and the + * agent turn's inline undo (#57) behave identically — including the part that + * actually matters, which is reporting a partial result honestly rather than as + * a success or a failure. + */ +export function UndoButton({ + changeSet, + undo, + className, + label = 'Undo', +}: { + changeSet: ChangeSet + undo: ReturnType + className?: string + label?: string +}) { + const outcome = undo.outcomeFor(changeSet.id) + return ( +
    + + {outcome && outcome.tone !== 'pending' && } +
    + ) +} + +const TONE_CLASS: Record, string> = { + ok: 'text-muted', + partial: 'text-amber-700 dark:text-amber-400', + error: 'text-red-700 dark:text-red-400', +} + +function OutcomeNote({ outcome }: { outcome: UndoOutcome }) { + if (outcome.tone === 'pending') return null + return ( +

    + {outcome.message} +

    + ) +} diff --git a/web/src/editor/store.ts b/web/src/editor/store.ts index dfe194c..ff39d5d 100644 --- a/web/src/editor/store.ts +++ b/web/src/editor/store.ts @@ -26,6 +26,12 @@ interface EditorState { focusedObjectId: number | null setFocusedObject: (id: number | null) => void + // Which rail tab is showing, or null when the rail is closed. Selecting an + // object switches this to 'inspector' (see GardenEditorPage), so editing never + // starts with a click on the rail itself. + railTab: string | null + setRailTab: (tab: string | null) => void + // The plant armed for placing plops (set after the PlantPicker choice); stays // armed for repeat-placement until cleared (Escape / done). null = not placing. armedPlant: Plant | null @@ -70,6 +76,9 @@ export const useEditorStore = create((set) => ({ focusedObjectId: null, setFocusedObject: (id) => set({ focusedObjectId: id }), + railTab: null, + setRailTab: (tab) => set({ railTab: tab }), + armedPlant: null, setArmedPlant: (p) => set({ armedPlant: p }), diff --git a/web/src/lib/history.test.ts b/web/src/lib/history.test.ts new file mode 100644 index 0000000..272fde6 --- /dev/null +++ b/web/src/lib/history.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it } from 'vitest' +import { describeConflict, describeCounts, describeUndo, totalChanges, type ChangeSet } from './history' + +function changeSet(over: Partial = {}): ChangeSet { + return { + id: 1, + gardenId: 1, + actorId: 1, + actorName: 'Steve', + source: 'ui', + summary: 'Did a thing', + counts: [], + createdAt: '2026-07-21T00:00:00Z', + ...over, + } +} + +describe('describeCounts', () => { + it('pluralizes per entity and names the operation', () => { + const cs = changeSet({ + counts: [ + { entityType: 'planting', op: 'create', n: 3 }, + { entityType: 'object', op: 'update', n: 1 }, + ], + }) + expect(describeCounts(cs)).toBe('3 plantings added, 1 bed changed') + }) + + it('says nothing rather than "0 changes"', () => { + expect(describeCounts(changeSet())).toBe('') + expect(describeCounts(changeSet({ counts: [{ entityType: 'object', op: 'create', n: 0 }] }))).toBe('') + }) +}) + +describe('describeConflict', () => { + it('names the entity when it has a name', () => { + expect(describeConflict({ entityType: 'object', entityId: 4, reason: 'changed', name: 'North Bed' })).toBe( + '“North Bed” was edited since, so it was left alone', + ) + }) + + it('falls back to the entity kind when it has no name', () => { + expect(describeConflict({ entityType: 'planting', entityId: 9, reason: 'missing' })).toBe( + 'that planting no longer exists', + ) + }) +}) + +describe('describeUndo', () => { + const target = changeSet({ counts: [{ entityType: 'object', op: 'update', n: 3 }] }) + + it('is a plain success when nothing conflicted', () => { + const out = describeUndo(target, { changeSet: changeSet({ id: 2 }), conflicts: [] }) + expect(out).toEqual({ tone: 'ok', message: 'Undone.' }) + }) + + // The case the issue was specific about: a partial revert really did change + // things, so reporting a bare failure would be a lie about what applied. + it('says how much applied and what was skipped', () => { + const out = describeUndo(target, { + changeSet: changeSet({ id: 2, counts: [{ entityType: 'object', op: 'update', n: 2 }] }), + conflicts: [{ entityType: 'object', entityId: 7, reason: 'changed', name: 'North Bed' }], + }) + expect(out.tone).toBe('partial') + expect(out.message).toBe('2 of 3 changes undone — “North Bed” was edited since, so it was left alone.') + }) + + it('is an error when nothing applied at all', () => { + const out = describeUndo(target, { + changeSet: null, + conflicts: [{ entityType: 'object', entityId: 7, reason: 'changed', name: 'North Bed' }], + }) + expect(out.tone).toBe('error') + expect(out.message).toBe('Nothing was undone — “North Bed” was edited since, so it was left alone.') + }) + + it('lists every conflict, not just the first', () => { + const out = describeUndo(target, { + changeSet: changeSet({ id: 2, counts: [{ entityType: 'object', op: 'update', n: 1 }] }), + conflicts: [ + { entityType: 'object', entityId: 7, reason: 'changed', name: 'North Bed' }, + { entityType: 'planting', entityId: 8, reason: 'missing' }, + ], + }) + expect(out.message).toContain('“North Bed”') + expect(out.message).toContain('that planting no longer exists') + }) +}) + +describe('totalChanges', () => { + it('sums every tally', () => { + expect( + totalChanges( + changeSet({ + counts: [ + { entityType: 'planting', op: 'create', n: 12 }, + { entityType: 'object', op: 'update', n: 1 }, + ], + }), + ), + ).toBe(13) + }) +}) diff --git a/web/src/lib/history.ts b/web/src/lib/history.ts new file mode 100644 index 0000000..a02420b --- /dev/null +++ b/web/src/lib/history.ts @@ -0,0 +1,218 @@ +// Change history and undo (#49): zod-validated shapes for /gardens/:id/history +// and /change-sets/:id/revert, plus the hooks the editor's History tab uses. +// +// A revert is a real write that can partly succeed, so its mutation is unusual: +// a 409 is not a plain failure. It carries the change set that DID apply +// alongside the entities it deliberately left alone, and the caller is expected +// to report both. + +import { useState } from 'react' +import { useInfiniteQuery, useMutation, useQueryClient } from '@tanstack/react-query' +import { z } from 'zod' +import { ApiError, api, errorMessage } from './api' +import { gardenFullKey } from './objects' + +export const changeSourceSchema = z.enum(['ui', 'agent', 'api']) +export type ChangeSource = z.infer + +export const changeCountSchema = z.object({ + entityType: z.enum(['garden', 'object', 'planting']), + op: z.enum(['create', 'update', 'delete']), + n: z.number(), +}) +export type ChangeCount = z.infer + +export const changeSetSchema = z.object({ + id: z.number(), + gardenId: z.number(), + actorId: z.number(), + actorName: z.string().default(''), + source: changeSourceSchema, + summary: z.string(), + agentRunId: z.string().optional(), + // Set when this change set reverts another. + revertsId: z.number().optional(), + // Set when another change set reverted this one. + revertedById: z.number().optional(), + counts: z.array(changeCountSchema).default([]), + createdAt: z.string(), +}) +export type ChangeSet = z.infer + +export const revertConflictSchema = z.object({ + entityType: z.enum(['garden', 'object', 'planting']), + entityId: z.number(), + reason: z.enum(['changed', 'missing', 'exists', 'unsupported']), + name: z.string().optional(), +}) +export type RevertConflict = z.infer + +const historyPageSchema = z.object({ + changeSets: z.array(changeSetSchema), + hasMore: z.boolean(), +}) + +const revertResultSchema = z.object({ + changeSet: changeSetSchema.nullable(), + conflicts: z.array(revertConflictSchema).default([]), +}) +export type RevertResult = z.infer + +const PAGE_SIZE = 30 + +export function historyKey(gardenId: number) { + return ['gardens', gardenId, 'history'] as const +} + +/** A garden's change sets, newest first, paged on demand. */ +export function useGardenHistory(gardenId: number, enabled = true) { + return useInfiniteQuery({ + queryKey: historyKey(gardenId), + enabled, + initialPageParam: 0, + queryFn: async ({ pageParam }) => + historyPageSchema.parse( + await api.get(`/gardens/${gardenId}/history`, { params: { limit: PAGE_SIZE, offset: pageParam } }), + ), + // Offset paging, not cursor: history is append-only at the FRONT, so a new + // entry arriving mid-scroll shifts the window by one. That's a duplicate row + // at worst, and the alternative (a cursor on created_at) buys little for a + // list you scroll a page or two of. + getNextPageParam: (last, pages) => (last.hasMore ? pages.length * PAGE_SIZE : undefined), + }) +} + +/** The conflicts carried by a 409 from a revert, or null if this isn't one. */ +export function revertConflicts(err: unknown): RevertResult | null { + if (!(err instanceof ApiError) || err.status !== 409) return null + const parsed = revertResultSchema.safeParse(err.body) + return parsed.success ? parsed.data : null +} + +/** + * Undo a change set. Invalidates the editor payload so the canvas redraws, and + * the history list so the new entry (and the "reverted" mark on its target) + * appear — on failure too, because a 409 means part of it applied. + */ +export function useRevertChangeSet(gardenId: number) { + const qc = useQueryClient() + const refresh = () => { + void qc.invalidateQueries({ queryKey: gardenFullKey(gardenId) }) + void qc.invalidateQueries({ queryKey: historyKey(gardenId) }) + } + return useMutation({ + mutationFn: async (changeSetId: number): Promise => + revertResultSchema.parse(await api.post(`/change-sets/${changeSetId}/revert`, undefined)), + onSuccess: refresh, + onError: (err) => { + if (revertConflicts(err)) refresh() // a partial revert still changed things + }, + }) +} + +/** How many rows a change set touched, for "3 changes" in the list. */ +export function totalChanges(cs: ChangeSet): number { + return cs.counts.reduce((sum, c) => sum + c.n, 0) +} + +const ENTITY_NOUNS: Record = { + garden: ['garden setting', 'garden settings'], + object: ['bed', 'beds'], + planting: ['planting', 'plantings'], +} + +const OP_VERBS: Record = { + create: 'added', + update: 'changed', + delete: 'removed', +} + +/** + * The change set's counts as a human phrase: "3 plantings added, 1 bed changed". + * Returns '' when there's nothing to say, so callers can skip the line entirely + * rather than render "0 changes". + */ +export function describeCounts(cs: ChangeSet): string { + return cs.counts + .filter((c) => c.n > 0) + .map((c) => { + const [one, many] = ENTITY_NOUNS[c.entityType] + return `${c.n} ${c.n === 1 ? one : many} ${OP_VERBS[c.op]}` + }) + .join(', ') +} + +/** What a revert left alone, phrased for the person who asked for the undo. */ +export function describeConflict(c: RevertConflict): string { + const noun = c.name ? `“${c.name}”` : `that ${ENTITY_NOUNS[c.entityType][0]}` + switch (c.reason) { + case 'changed': + return `${noun} was edited since, so it was left alone` + case 'missing': + return `${noun} no longer exists` + case 'exists': + return `${noun} exists again, so it wasn't restored over the top` + case 'unsupported': + return `${noun} can't be undone automatically` + } +} + +/** + * The one undo implementation, shared by the history list and (per #49) the + * agent turn's inline Undo in #57 — so an undo behaves identically wherever it + * is offered, including how it explains a partial result. + * + * Outcomes are keyed by change set id, so a single instance serves a whole list. + */ +export function useUndo(gardenId: number) { + const revert = useRevertChangeSet(gardenId) + const [outcomes, setOutcomes] = useState>({}) + + const undo = (cs: ChangeSet) => { + setOutcomes((prev) => ({ ...prev, [cs.id]: { tone: 'pending', message: 'Undoing…' } })) + revert.mutate(cs.id, { + onSuccess: (result) => { + setOutcomes((prev) => ({ ...prev, [cs.id]: describeUndo(cs, result) })) + }, + onError: (err) => { + const result = revertConflicts(err) + setOutcomes((prev) => ({ + ...prev, + [cs.id]: result + ? describeUndo(cs, result) + : { tone: 'error', message: errorMessage(err, "That couldn't be undone.") }, + })) + }, + }) + } + + return { + undo, + outcomeFor: (id: number): UndoOutcome | undefined => outcomes[id], + isPending: revert.isPending, + } +} + +export interface UndoOutcome { + tone: 'pending' | 'ok' | 'partial' | 'error' + message: string +} + +/** + * What actually happened, said in full. A revert can partly succeed, and "2 of 3 + * changes undone — “north bed” was edited since and was left alone" is the only + * useful thing to report: a bare failure would be a lie about the two that did + * apply, and a bare success would hide the one that didn't. + */ +export function describeUndo(target: ChangeSet, result: RevertResult): UndoOutcome { + const skipped = result.conflicts.map(describeConflict).join('; ') + if (result.conflicts.length === 0) { + return { tone: 'ok', message: 'Undone.' } + } + const applied = result.changeSet ? totalChanges(result.changeSet) : 0 + if (applied === 0) { + return { tone: 'error', message: `Nothing was undone — ${skipped}.` } + } + const total = totalChanges(target) + return { tone: 'partial', message: `${applied} of ${total} changes undone — ${skipped}.` } +} diff --git a/web/src/lib/objects.ts b/web/src/lib/objects.ts index 3077f0a..322414c 100644 --- a/web/src/lib/objects.ts +++ b/web/src/lib/objects.ts @@ -69,7 +69,11 @@ export function toEditorObject(o: ServerObject): EditorObject { } } -const fullKey = (gardenId: number) => ['garden-full', gardenId] as const +// Exported so anything that changes a garden's contents from outside this file +// — undo, in particular — can invalidate the editor payload and make the canvas +// redraw. +export const gardenFullKey = (gardenId: number) => ['garden-full', gardenId] as const +const fullKey = gardenFullKey export function gardenFullQueryOptions(gardenId: number) { return queryOptions({ diff --git a/web/src/pages/GardenEditorPage.tsx b/web/src/pages/GardenEditorPage.tsx index 2a979d4..52123da 100644 --- a/web/src/pages/GardenEditorPage.tsx +++ b/web/src/pages/GardenEditorPage.tsx @@ -3,6 +3,8 @@ import { getRouteApi } from '@tanstack/react-router' import { Alert } from '@/components/ui/Alert' import { Button } from '@/components/ui/Button' import { GardenCanvas } from '@/editor/GardenCanvas' +import { EditorRail, type RailTab } from '@/editor/EditorRail' +import { HistoryPanel } from '@/editor/HistoryPanel' import { Inspector } from '@/editor/Inspector' import { PlopInspector } from '@/editor/PlopInspector' import { PlantPicker } from '@/editor/PlantPicker' @@ -38,6 +40,8 @@ export function GardenEditorPage() { const selectPlanting = useEditorStore((s) => s.selectPlanting) const focusedObjectId = useEditorStore((s) => s.focusedObjectId) const setFocusedObject = useEditorStore((s) => s.setFocusedObject) + const railTab = useEditorStore((s) => s.railTab) + const setRailTab = useEditorStore((s) => s.setRailTab) const armedPlant = useEditorStore((s) => s.armedPlant) const setArmedPlant = useEditorStore((s) => s.setArmedPlant) const setArmedKind = useEditorStore((s) => s.setArmedKind) @@ -86,6 +90,7 @@ export function GardenEditorPage() { setArmedPlant(null) setLiveObject(null) setLivePlanting(null) + setRailTab(null) } clear() setFocusedObject(focus ?? null) @@ -110,6 +115,16 @@ export function GardenEditorPage() { } }, [focusedObjectId, objects, full.data, setFocusedObject]) + // Selecting anything lands you in the inspector without a click — the rail + // must never be something you operate before you can edit. Deselecting drops + // back out of the inspector, but leaves History/Chat open if that's where you + // were, since those aren't about the selection. + const hasSelection = selectedId != null || selectedPlantingId != null + useEffect(() => { + if (hasSelection) setRailTab('inspector') + else if (useEditorStore.getState().railTab === 'inspector') setRailTab(null) + }, [hasSelection, setRailTab]) + const exitFocus = () => { setFocusedObject(null) setArmedPlant(null) @@ -270,6 +285,48 @@ export function GardenEditorPage() { setPicker(null) } + // One rail, tabs inside it — the layout decision #49 settles for the journal + // (#53) and chat (#57) panels too: they add a tab here rather than each + // claiming their own strip of screen. + const railTabs: RailTab[] = [ + { + id: 'inspector', + label: 'Inspector', + render: () => + selectedObject ? ( + { + setFocusedObject(selectedObject.id) + select(null) + }} + /> + ) : selectedPlop ? ( + setPicker('change')} + onClose={() => selectPlanting(null)} + /> + ) : ( +

    Select a bed or a planting to edit it.

    + ), + }, + { + id: 'history', + label: 'History', + render: () => , + }, + ] + return (
    @@ -285,6 +342,13 @@ export function GardenEditorPage() {

    👁 View only

    )} {focusedObjectId == null && canEdit && } +
    @@ -335,34 +399,17 @@ export function GardenEditorPage() { )}
    - {(selectedObject || selectedPlop) && ( -
    - {selectedObject && ( - { - setFocusedObject(selectedObject.id) - select(null) - }} - /> - )} - {selectedPlop && ( - setPicker('change')} - onClose={() => selectPlanting(null)} - /> - )} -
    + {railTab && ( + { + setRailTab(null) + select(null) + selectPlanting(null) + }} + /> )} {picker && (