Editor: the toolkit is a rail tab on desktop, and the rail is wider #133

Merged
steve merged 2 commits from feat/toolkit-in-the-rail into main 2026-08-23 07:05:20 +00:00
4 changed files with 12 additions and 17 deletions
Showing only changes of commit 7a5b9d2ea1 - Show all commits
+1 -3
View File
@@ -177,9 +177,7 @@ export const Canvas = forwardRef<
st.setSel(m ? null : { type: 'object', id: o.id }) st.setSel(m ? null : { type: 'object', id: o.id })
st.setArmedKind(null) st.setArmedKind(null)
st.setGhost(null) st.setGhost(null)
// Focusing a bed is planting intent: the rail shows the plant palette // Focusing is planting intent: the rail shows the toolkit's plant palette.
Outdated
Review

3-line comment exceeds one-line CLAUDE.md convention; third line describes a different code path and can rot

maintainability · flagged by 1 model

  • 3-line comment in Canvas.tsx (web/src/editor/Canvas.tsx:180–182). CLAUDE.md permits one short line maximum, for non-obvious WHY only. The added block is three lines; the third line ("A single click still selects into the Plot tab") describes a different code path's behavior and will silently lie if the single-click handler changes. The first line's WHY is worth keeping; the rest is noise.

🪰 Gadfly · advisory

⚪ **3-line comment exceeds one-line CLAUDE.md convention; third line describes a different code path and can rot** _maintainability · flagged by 1 model_ - **3-line comment in Canvas.tsx** (`web/src/editor/Canvas.tsx:180–182`). CLAUDE.md permits one short line maximum, for non-obvious WHY only. The added block is three lines; the third line ("A single click still selects into the Plot tab") describes a different code path's behavior and will silently lie if the single-click handler changes. The first line's WHY is worth keeping; the rest is noise. <sub>🪰 Gadfly · advisory</sub>
// (the toolkit's focused mode), not the inspector. A single click still
// selects into the Plot tab.
st.setTab('toolkit') st.setTab('toolkit')
if (!el) return if (!el) return
const bw = o.rotationDeg % 180 ? o.heightCm : o.widthCm const bw = o.rotationDeg % 180 ? o.heightCm : o.widthCm
+10 -12
View File
@@ -14,18 +14,18 @@ import { useEditorStore } from './store'
import type { EditorObject } from './types' import type { EditorObject } from './types'
/** /**
* The desktop editor's toolkit. Out of focus: the seven object kinds as pill * The desktop editor's toolkit, the rail's first tab. Out of focus: the seven
* rows — click to arm (then click the plan), or drag one straight onto it. In a * object kinds as pill rows — click to arm (then click the plan), or drag one
* focused bed it swaps to the plant list with a search, same arm/drag behavior, * straight onto it. In a focused bed it swaps to the plant list with a search,
* plus the bed's bulk tools (fill, clear, scan a packet). * same arm/drag behavior, plus the bed's bulk tools (fill, clear, scan a
* packet).
* *
* It started life as the workspace's left card (the handoff's layout) and now * It started life as the workspace's left card (the handoff's layout) and
* lives as the first tab of the rail, so the plan and the rail get the width * moved into the rail so the plan and the rail get the width the card took;
* the card took. `embedded` is that mode: no card chrome of its own, and no * the rail's tab body provides the card chrome and the scrolling, and the tab
* "Toolkit" heading — the tab is the heading. * is the heading.
*/ */
export function Toolkit({ export function Toolkit({
Outdated
Review

🟠 embedded=false default keeps dead non-embedded rendering path with no callers

maintainability · flagged by 1 model

  • embedded = false default retains a dead rendering path (web/src/editor/Toolkit.tsx:28,65,68). There is exactly one <Toolkit call site (GardenEditorPage.tsx:587) and it always passes embedded. The embedded = false default makes the non-embedded branch — the panel overflow-y-auto p-3.5 wrapper and the <h6>Toolkit</h6> heading — unreachable dead code. CLAUDE.md is explicit: "Don't add features or introduce abstractions beyond what the task requires." Either make embedded a req…

🪰 Gadfly · advisory

🟠 **embedded=false default keeps dead non-embedded rendering path with no callers** _maintainability · flagged by 1 model_ - **`embedded = false` default retains a dead rendering path** (`web/src/editor/Toolkit.tsx:28,65,68`). There is exactly one `<Toolkit` call site (GardenEditorPage.tsx:587) and it always passes `embedded`. The `embedded = false` default makes the non-embedded branch — the `panel overflow-y-auto p-3.5` wrapper and the `<h6>Toolkit</h6>` heading — unreachable dead code. CLAUDE.md is explicit: "Don't add features or introduce abstractions beyond what the task requires." Either make `embedded` a req… <sub>🪰 Gadfly · advisory</sub>
embedded = false,
unit, unit,
plants, plants,
plantings, plantings,
@@ -40,7 +40,6 @@ export function Toolkit({
onClear, onClear,
onScan, onScan,
}: { }: {
embedded?: boolean
unit: UnitPref unit: UnitPref
plants: Plant[] plants: Plant[]
plantings: EditorPlanting[] plantings: EditorPlanting[]
@@ -62,10 +61,9 @@ export function Toolkit({
const [query, setQuery] = useState('') const [query, setQuery] = useState('')
return ( return (
<div className={cn('flex min-h-0 flex-col gap-2 overflow-x-hidden', embedded ? 'flex-1' : 'panel overflow-y-auto p-3.5')}> <div className="flex min-h-0 flex-1 flex-col gap-2 overflow-x-hidden">
{!focused ? ( {!focused ? (
Outdated
Review

🟡 embedded prop is always passed true; non-embedded left-card branch is now dead code

error-handling, maintainability · flagged by 2 models

  • web/src/editor/Toolkit.tsx:65,68 — the embedded prop is always passed true, so the non-embedded (left-card) branch is dead code. The only render site is web/src/pages/GardenEditorPage.tsx:587, which passes embedded; no test or phone-layout path renders Toolkit standalone. That leaves the panel overflow-y-auto p-3.5 styling and the {!embedded && <h6>Toolkit</h6>} heading unreachable. Since CLAUDE.md/DESIGN.md now state the left card is gone deliberately ("Don't bring the thi…

🪰 Gadfly · advisory

🟡 **`embedded` prop is always passed true; non-embedded left-card branch is now dead code** _error-handling, maintainability · flagged by 2 models_ - **`web/src/editor/Toolkit.tsx:65,68` — the `embedded` prop is always passed `true`, so the non-embedded (left-card) branch is dead code.** The only render site is `web/src/pages/GardenEditorPage.tsx:587`, which passes `embedded`; no test or phone-layout path renders `Toolkit` standalone. That leaves the `panel overflow-y-auto p-3.5` styling and the `{!embedded && <h6>Toolkit</h6>}` heading unreachable. Since CLAUDE.md/DESIGN.md now state the left card is gone deliberately ("Don't bring the thi… <sub>🪰 Gadfly · advisory</sub>
<> <>
{!embedded && <h6 className="mx-1 mb-1.5 mt-1">Toolkit</h6>}
{OBJECT_KINDS.map((k) => { {OBJECT_KINDS.map((k) => {
const armed = armedKind === k.kind const armed = armedKind === k.kind
return ( return (
+1 -1
View File
@@ -85,7 +85,7 @@ const TRANSIENT = {
armedLotId: null, armedLotId: null,
ghost: null, ghost: null,
seasonYear: null, seasonYear: null,
tab: 'plot' as RailTab, tab: 'plot' as RailTab, // the garden summary, not the first tab: a fresh editor shows what is there before how to add to it
mode: 'build' as PhoneMode, mode: 'build' as PhoneMode,
journalScope: null, journalScope: null,
liveObject: null, liveObject: null,
-1
View File
@@ -585,7 +585,6 @@ function Editor({
const plot = inspector ?? <GardenSummary objects={objects} plantings={plantings} plantsById={plantsById} canEdit={canEdit} /> const plot = inspector ?? <GardenSummary objects={objects} plantings={plantings} plantsById={plantsById} canEdit={canEdit} />
const toolkit = ( const toolkit = (
<Toolkit <Toolkit
embedded
unit={unit} unit={unit}
plants={plants} plants={plants}
plantings={plantings} plantings={plantings}