Editor mode model: mobile Fixtures/Plants/Journal/Assistant bar (#99) #110

Merged
steve merged 2 commits from feat/editor-modes into main 2026-07-22 05:32:47 +00:00
2 changed files with 142 additions and 64 deletions
Showing only changes of commit 2d25b7e28e - Show all commits
+5 -2
View File
@@ -18,6 +18,9 @@ export const MAX_SCALE = 20 // px per cm — fully zoomed in
// side-column layout and treats this as a lighter hint.
export type EditorMode = 'fixtures' | 'plants' | 'journal' | 'assistant'
// Where the editor starts, and where it returns on a garden switch.
export const DEFAULT_MODE: EditorMode = 'fixtures'
interface EditorState {
viewport: Viewport
setViewport: (next: Viewport | ((prev: Viewport) => Viewport)) => void
@@ -94,7 +97,7 @@ export const useEditorStore = create<EditorState>((set) => ({
viewport: { tx: 0, ty: 0, scale: 1 },
setViewport: (next) => set((s) => ({ viewport: typeof next === 'function' ? next(s.viewport) : next })),
mode: 'fixtures',
mode: DEFAULT_MODE,
setMode: (mode) => set({ mode }),
selectedId: null,
@@ -144,6 +147,6 @@ export const useEditorStore = create<EditorState>((set) => ({
railTab: null,
Outdated
Review

Default mode 'fixtures' hardcoded twice (initial state and resetTransient)

maintainability · flagged by 1 model

🪰 Gadfly · advisory

⚪ **Default mode 'fixtures' hardcoded twice (initial state and resetTransient)** _maintainability · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
seasonYear: null,
journalObjectId: null,
mode: 'fixtures',
mode: DEFAULT_MODE,
}),
}))
+137 -62
View File
@@ -172,11 +172,18 @@ export function GardenEditorPage() {
}
}, [focusedObjectId, objects, full.data, setFocusedObject])
// Focusing a bed IS "placing plants", so the mobile mode bar follows into Plants
// — otherwise the bottom strip would show the object palette while you're inside
// a bed. (Inert on desktop, where the mode bar isn't shown.)
// The canvas mode follows focus: inside a bed you're placing Plants, out of it
Outdated
Review

🟡 Misleading comment claims focus effect is inert on desktop

error-handling, maintainability · flagged by 2 models

  • web/src/pages/GardenEditorPage.tsx:175-177 — The comment claims the focus→plants effect is "Inert on desktop". It is not inert: it actively writes to the store on every focus change. The value happens to be unused on desktop, which is different.

🪰 Gadfly · advisory

🟡 **Misleading comment claims focus effect is inert on desktop** _error-handling, maintainability · flagged by 2 models_ - **`web/src/pages/GardenEditorPage.tsx:175-177`** — The comment claims the focus→plants effect is "Inert on desktop". It is not inert: it actively writes to the store on every focus change. The value happens to be unused on desktop, which is different. <sub>🪰 Gadfly · advisory</sub>
// you're arranging Fixtures — so the bottom strip can't show the object palette
// while you're in a bed, or the seed tray while you're not. A panel mode
// (journal/assistant) is set explicitly and left alone here. (Inert on desktop,
// where the mode bar isn't shown.)
useEffect(() => {
if (focusedObjectId != null) setMode('plants')
const cur = useEditorStore.getState().mode
if (focusedObjectId != null) {
if (cur !== 'journal' && cur !== 'assistant') setMode('plants')
} else if (cur === 'plants') {
setMode('fixtures')
}
}, [focusedObjectId, setMode])
// Selecting anything lands you in the inspector without a click — the rail
@@ -184,11 +191,20 @@ export function GardenEditorPage() {
// selected ids rather than a "something is selected" boolean, so selecting a
// DIFFERENT object while History is open still brings the inspector forward.
// 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.
// that's where you were, since those aren't about the selection. Selecting is a
// canvas interaction, so it also leaves a panel mode — otherwise closing the
// inspector would strand the mode bar on Journal/Assistant with nothing open.
useEffect(() => {
if (selectedId != null || selectedPlantingId != null) setRailTab('inspector')
else if (useEditorStore.getState().railTab === 'inspector') setRailTab(null)
}, [selectedId, selectedPlantingId, setRailTab])
if (selectedId != null || selectedPlantingId != null) {
setRailTab('inspector')
const s = useEditorStore.getState()
if (s.mode === 'journal' || s.mode === 'assistant') {
setMode(s.focusedObjectId != null ? 'plants' : 'fixtures')
}
} else if (useEditorStore.getState().railTab === 'inspector') {
setRailTab(null)
}
}, [selectedId, selectedPlantingId, setRailTab, setMode])
const exitFocus = () => {
setFocusedObject(null)
1
@@ -199,7 +215,8 @@ export function GardenEditorPage() {
// The mobile mode bar. Journal/Assistant are panel modes, so they open the
// rail sheet; Fixtures/Plants are canvas modes, so they close a panel rail (but
// leave an inspector, which is about the selection, alone).
// leave an inspector, which is about the selection, alone). Tapping Fixtures
// means going back to arranging objects, so it steps out of a focused bed.
const selectMode = (m: EditorMode) => {
setMode(m)
if (m === 'journal') {
@@ -208,11 +225,21 @@ export function GardenEditorPage() {
} else if (m === 'assistant') {
setRailTab('chat')
} else {
const rt = useEditorStore.getState().railTab
if (rt === 'journal' || rt === 'chat') setRailTab(null)
if (railTab === 'journal' || railTab === 'chat') setRailTab(null)
if (m === 'fixtures' && focusedObjectId != null) exitFocus()
}
}
// Safety for a live capability flip: if the admin turns the assistant off while
// it's the active mode, drop back to a canvas mode so the bar isn't stuck on a
// tab that no longer exists (and close the now-orphaned chat rail).
useEffect(() => {
if (!capabilities.data?.agent && useEditorStore.getState().mode === 'assistant') {
setMode('fixtures')
if (useEditorStore.getState().railTab === 'chat') setRailTab(null)
}
}, [capabilities.data?.agent, setMode, setRailTab])
// Escape peels back one layer: stop placing → deselect plop → exit focus → deselect.
useEffect(() => {
function onKey(e: KeyboardEvent) {
1
@@ -544,29 +571,16 @@ export function GardenEditorPage() {
<span className="max-w-[8rem] truncate text-muted">{objectDisplayName(focusedObject)}</span>
{canEdit &&
(focusedObject.plantable ? (
<>
<SeedTray
trayPlants={trayPlants}
armedPlantId={armedPlant?.id ?? null}
onArm={armPlant}
onRemove={removeFromTrayAndDisarm}
onOpenPicker={() => setPicker('place')}
/>
{armedPlant && (
<Button variant="ghost" className="px-2 py-1 text-xs" onClick={() => setArmedPlant(null)}>
Done
</Button>
)}
{focusedPlops.length > 0 && (
<Button
variant="ghost"
className="px-2 py-1 text-xs text-red-600 dark:text-red-400"
onClick={() => setClearing(true)}
>
Clear ({focusedPlops.length})
</Button>
)}
</>
<PlantPlacementTools
trayPlants={trayPlants}
armedPlant={armedPlant}
onArm={armPlant}
onRemove={removeFromTrayAndDisarm}
onOpenPicker={() => setPicker('place')}
onDisarm={() => setArmedPlant(null)}
focusedPlopCount={focusedPlops.length}
onClear={() => setClearing(true)}
/>
) : (
<span className="text-xs text-muted">Not plantable</span>
))}
2
@@ -593,39 +607,41 @@ export function GardenEditorPage() {
<div className="mb-2 min-h-[2.25rem]">
{mode === 'fixtures' && <Palette />}
{mode === 'plants' &&
(focusedObject?.plantable ? (
<div className="flex flex-wrap items-center gap-2">
<SeedTray
trayPlants={trayPlants}
armedPlantId={armedPlant?.id ?? null}
onArm={armPlant}
onRemove={removeFromTrayAndDisarm}
onOpenPicker={() => setPicker('place')}
/>
{armedPlant && (
<Button variant="ghost" className="px-2 py-1 text-xs" onClick={() => setArmedPlant(null)}>
(focusedObject ? (
focusedObject.plantable ? (
<div className="flex flex-wrap items-center gap-2">
<PlantPlacementTools
trayPlants={trayPlants}
armedPlant={armedPlant}
onArm={armPlant}
onRemove={removeFromTrayAndDisarm}
onOpenPicker={() => setPicker('place')}
onDisarm={() => setArmedPlant(null)}
focusedPlopCount={focusedPlops.length}
onClear={() => setClearing(true)}
/>
<Button variant="ghost" className="ml-auto px-2 py-1 text-xs" onClick={exitFocus}>
Done planting
Outdated
Review

🟡 Mobile Plants-mode hint says 'tap a bed' even when a non-plantable object is already focused (via deep-linked ?focus=), unlike the desktop toolbar's explicit 'Not plantable' state

correctness · flagged by 2 models

  • web/src/pages/GardenEditorPage.tsx:624 — Plants mode on a focused non-plantable object shows a misleading hint and no exit. Verified: focusing any object triggers the effect at :178mode='plants'. The Inspector only exposes the Focus button for object.plantable (Inspector.tsx:139), so in practice users can't focus a non-plantable object from the inspector — but a ?focus=<id> deep link (adopted at :135) to a non-plantable object (e.g. a shed) sets focusedObjectId directly,…

🪰 Gadfly · advisory

🟡 **Mobile Plants-mode hint says 'tap a bed' even when a non-plantable object is already focused (via deep-linked ?focus=), unlike the desktop toolbar's explicit 'Not plantable' state** _correctness · flagged by 2 models_ - **`web/src/pages/GardenEditorPage.tsx:624` — Plants mode on a focused non-plantable object shows a misleading hint and no exit.** Verified: focusing any object triggers the effect at `:178` → `mode='plants'`. The Inspector only exposes the Focus button for `object.plantable` (`Inspector.tsx:139`), so in practice users can't focus a non-plantable object from the inspector — but a `?focus=<id>` deep link (adopted at `:135`) to a non-plantable object (e.g. a shed) sets `focusedObjectId` directly,… <sub>🪰 Gadfly · advisory</sub>
</Button>
</div>
) : (
// Reachable via a ?focus= deep link onto a non-plantable object:
Outdated
Review

🟡 mode can stay stuck on 'assistant' with no active tab/panel if the agent capability flips off live

error-handling · flagged by 1 model

🪰 Gadfly · advisory

🟡 **mode can stay stuck on 'assistant' with no active tab/panel if the agent capability flips off live** _error-handling · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
// say so, and give a way back out (there's no focus toolbar here).
<div className="flex items-center gap-2">
<span className="px-1 text-xs text-muted">
{objectDisplayName(focusedObject)} isnt plantable.
</span>
<Button variant="ghost" className="ml-auto px-2 py-1 text-xs" onClick={exitFocus}>
Done
</Button>
)}
{focusedPlops.length > 0 && (
<Button
variant="ghost"
className="px-2 py-1 text-xs text-red-600 dark:text-red-400"
onClick={() => setClearing(true)}
>
Clear ({focusedPlops.length})
</Button>
)}
<Button variant="ghost" className="ml-auto px-2 py-1 text-xs" onClick={exitFocus}>
Done planting
</Button>
</div>
</div>
)
) : (
<p className="px-1 text-xs text-muted">Tap a bed, then 🌱 Plant here to start planting.</p>
))}
</div>
)}
<ModeBar mode={mode} onSelect={selectMode} hasAssistant={!!capabilities.data?.agent} />
<ModeBar mode={mode} onSelect={selectMode} hasAssistant={!!capabilities.data?.agent} canEdit={canEdit} />
</div>
Review

🔴 Closing Journal/Assistant/History rail forces mode back to 'fixtures' even while a bed is still focused, contradicting the focus→plants invariant and hiding the seed tray behind the object palette

correctness, error-handling · flagged by 5 models

  • web/src/pages/GardenEditorPage.tsx:645 — Closing a panel rail (journal/history/chat) unconditionally calls setMode('fixtures'), which can violate the "focusing a bed puts you in Plants mode" invariant established at line 178-180. Reachable path: focus a bed (mode→plants via the effect) → tap Journal mode (selectMode('journal') opens the rail, mode=journal) → close the rail. The close handler sets mode back to fixtures while focusedObjectId is still set, so the mobile bottom s…

🪰 Gadfly · advisory

🔴 **Closing Journal/Assistant/History rail forces mode back to 'fixtures' even while a bed is still focused, contradicting the focus→plants invariant and hiding the seed tray behind the object palette** _correctness, error-handling · flagged by 5 models_ - `web/src/pages/GardenEditorPage.tsx:645` — Closing a panel rail (journal/history/chat) unconditionally calls `setMode('fixtures')`, which can violate the "focusing a bed puts you in Plants mode" invariant established at line 178-180. Reachable path: focus a bed (mode→`plants` via the effect) → tap **Journal** mode (`selectMode('journal')` opens the rail, mode=`journal`) → close the rail. The close handler sets mode back to `fixtures` while `focusedObjectId` is still set, so the mobile bottom s… <sub>🪰 Gadfly · advisory</sub>
{railTab && (
@@ -642,7 +658,10 @@ export function GardenEditorPage() {
select(null)
selectPlanting(null)
} else {
setMode('fixtures')
// Back to a canvas mode so the mode bar reappears — Plants if you're
// still inside a bed, else Fixtures. (Hardcoding Fixtures here docked
// the object palette inside a focused bed.)
setMode(focusedObjectId != null ? 'plants' : 'fixtures')
}
setRailTab(null)
}}
1
@@ -672,6 +691,54 @@ export function GardenEditorPage() {
)
}
// The plant-placement cluster (seed tray + Done + Clear), shared by the desktop
Outdated
Review

🟡 ModeBar can have no active tab when assistant capability is removed

correctness · flagged by 1 model

  • web/src/pages/GardenEditorPage.tsx:694 — When hasAssistant flips from true to false (e.g. capabilities query refreshes after an admin disables the agent), the ModeBar filters out the assistant tab, but mode may still be 'assistant'. No tab will be highlighted as active, leaving the mode bar in an inconsistent state. Fix: either normalize mode to 'fixtures' when the current mode is no longer in the filtered list, or keep the assistant tab disabled rather than removing it.

🪰 Gadfly · advisory

🟡 **ModeBar can have no active tab when assistant capability is removed** _correctness · flagged by 1 model_ * `web/src/pages/GardenEditorPage.tsx:694` — When `hasAssistant` flips from `true` to `false` (e.g. capabilities query refreshes after an admin disables the agent), the `ModeBar` filters out the assistant tab, but `mode` may still be `'assistant'`. No tab will be highlighted as active, leaving the mode bar in an inconsistent state. **Fix:** either normalize `mode` to `'fixtures'` when the current mode is no longer in the filtered list, or keep the assistant tab disabled rather than removing it. <sub>🪰 Gadfly · advisory</sub>
// focus toolbar and the mobile Plants strip so the two can't drift apart.
function PlantPlacementTools({
trayPlants,
armedPlant,
onArm,
onRemove,
onOpenPicker,
onDisarm,
focusedPlopCount,
onClear,
}: {
trayPlants: Plant[]
armedPlant: Plant | null
onArm: (p: Plant, lot?: SeedLot) => void
onRemove: (id: number) => void
onOpenPicker: () => void
onDisarm: () => void
focusedPlopCount: number
onClear: () => void
}) {
return (
<>
<SeedTray
trayPlants={trayPlants}
armedPlantId={armedPlant?.id ?? null}
onArm={onArm}
onRemove={onRemove}
onOpenPicker={onOpenPicker}
/>
{armedPlant && (
<Button variant="ghost" className="px-2 py-1 text-xs" onClick={onDisarm}>
Done
</Button>
)}
{focusedPlopCount > 0 && (
<Button
variant="ghost"
className="px-2 py-1 text-xs text-red-600 dark:text-red-400"
onClick={onClear}
>
Clear ({focusedPlopCount})
</Button>
)}
</>
)
}
// The mobile primary mode switch (#99): one always-there tab bar so "placing
// beds", "planting", "journaling" and "assistant" stop competing for the same
// strip. Assistant is dropped when the instance has no model configured.
@@ -686,12 +753,20 @@ function ModeBar({
mode,
onSelect,
hasAssistant,
canEdit,
}: {
mode: EditorMode
onSelect: (m: EditorMode) => void
hasAssistant: boolean
canEdit: boolean
}) {
const modes = hasAssistant ? MODES : MODES.filter((m) => m.id !== 'assistant')
const modes = MODES.filter((m) => {
if (m.id === 'assistant') return hasAssistant
// Fixtures/Plants are edit actions — a viewer can't place anything, so the
// bar offers only what they can do (read the journal, ask the assistant).
if (m.id === 'fixtures' || m.id === 'plants') return canEdit
return true
})
return (
<div
role="tablist"