Touch ergonomics: bigger handles + on-screen nudge pad (#104) #117

Merged
steve merged 2 commits from feat/touch-ergonomics into main 2026-07-22 14:20:24 +00:00
2 changed files with 107 additions and 53 deletions
Showing only changes of commit 1323a03acd - Show all commits
+6 -1
View File
@@ -3,7 +3,12 @@
// one place instead of drifting between files. // one place instead of drifting between files.
export const SELECT_COLOR = '#2f7a3e' // selection stroke/handles export const SELECT_COLOR = '#2f7a3e' // selection stroke/handles
export const HANDLE_PX = 12 // on-screen size of a drag/resize handle // On-screen size of a drag/resize handle. Bigger on a coarse pointer (touch) so
// a fingertip can actually grab a resize corner or the rotate knob — 12px is fine
// for a mouse but frustrating for a thumb (#104). Read once at load; a device
// doesn't switch its primary pointer mid-session.
export const HANDLE_PX =
Review

🟠 matchMedia optional chain misses .matches guard, can throw on unsupported environments

error-handling, maintainability · flagged by 2 models

  • web/src/editor/shared.ts:11window.matchMedia?.('(pointer: coarse)').matches will throw a TypeError in environments without matchMedia (or if it returns undefined) because the optional chain only guards the call, not the .matches access on its result. The expression should be window.matchMedia?.('(pointer: coarse)')?.matches so the property read is also short-circuited. Fix: add a second ?. before .matches.

🪰 Gadfly · advisory

🟠 **matchMedia optional chain misses .matches guard, can throw on unsupported environments** _error-handling, maintainability · flagged by 2 models_ - **`web/src/editor/shared.ts:11`** — `window.matchMedia?.('(pointer: coarse)').matches` will throw a `TypeError` in environments without `matchMedia` (or if it returns `undefined`) because the optional chain only guards the call, not the `.matches` access on its result. The expression should be `window.matchMedia?.('(pointer: coarse)')?.matches` so the property read is also short-circuited. *Fix:* add a second `?.` before `.matches`. <sub>🪰 Gadfly · advisory</sub>
typeof window !== 'undefined' && window.matchMedia?.('(pointer: coarse)').matches ? 22 : 12
export const MIN_RADIUS_CM = 1 // smallest plop radius export const MIN_RADIUS_CM = 1 // smallest plop radius
export const DIMMED_OPACITY = 0.4 // non-focused objects/plops in focus mode export const DIMMED_OPACITY = 0.4 // non-focused objects/plops in focus mode
+75 -26
View File
@@ -283,20 +283,12 @@ export function GardenEditorPage() {
// eslint-disable-next-line react-hooks/exhaustive-deps // eslint-disable-next-line react-hooks/exhaustive-deps
}, []) }, [])
// Desktop keyboard nudging: arrows move the selected object/plop 1cm (Shift = // Move the selected object/plop by (dx, dy) cm: apply live geometry instantly,
// 10cm); the PATCH is debounced ~400ms on key-idle so a held key doesn't spam. // then commit ONE debounced PATCH so a burst of nudges (a held arrow key, or
// Plops nudge in their object's local frame and clamp to its (local) bounds. // repeated taps of the touch pad) doesn't spam the server. Plops clamp to their
// Mounted once, reading live values from nudgeCtx so a data refetch can't // object's local bounds. Reads live values from getState/nudgeCtx so it's
// re-subscribe and cancel a pending commit; the pending commit is flushed on // correct whichever surface calls it; a commit only fires if its live value is
// unmount, and a fire only commits if its live value is still present (a drag // still present (a drag that cleared it already committed its own PATCH).
// that cleared it already committed its own PATCH).
useEffect(() => {
const DIRS: Record<string, [number, number]> = {
ArrowUp: [0, -1],
ArrowDown: [0, 1],
ArrowLeft: [-1, 0],
ArrowRight: [1, 0],
}
const commitLater = (fire: () => void) => { const commitLater = (fire: () => void) => {
nudgeFire.current = fire nudgeFire.current = fire
if (nudgeTimer.current != null) window.clearTimeout(nudgeTimer.current) if (nudgeTimer.current != null) window.clearTimeout(nudgeTimer.current)
@@ -307,25 +299,20 @@ export function GardenEditorPage() {
fn?.() fn?.()
}, 400) }, 400)
} }
function onKey(e: KeyboardEvent) {
const nudgeSelected = (dx: number, dy: number) => {
Review

🟠 nudgeSelected captured by empty-deps effect; refs-only invariant is now implicit and unguarded

maintainability · flagged by 2 models

  • web/src/pages/GardenEditorPage.tsx:303nudgeSelected and commitLater are defined in the component body (recreated per render) but the keyboard useEffect (line 343, empty deps at line 373) captures the first render's copies. This is functionally correct today because every read goes through nudgeCtx.current / useEditorStore.getState(), and it matches the pre-existing mount-once philosophy. The new asymmetry introduced by lifting these out of the effect: the touch pad gets a fres…

🪰 Gadfly · advisory

🟠 **nudgeSelected captured by empty-deps effect; refs-only invariant is now implicit and unguarded** _maintainability · flagged by 2 models_ - `web/src/pages/GardenEditorPage.tsx:303` — `nudgeSelected` and `commitLater` are defined in the component body (recreated per render) but the keyboard `useEffect` (line 343, empty deps at line 373) captures the *first render's* copies. This is functionally correct today because every read goes through `nudgeCtx.current` / `useEditorStore.getState()`, and it matches the pre-existing mount-once philosophy. The new asymmetry introduced by lifting these out of the effect: the touch pad gets a fres… <sub>🪰 Gadfly · advisory</sub>
const { canEdit: canNudge, objects: objs, plantings: plops, updateObject: uo, updatePlanting: up } = const { canEdit: canNudge, objects: objs, plantings: plops, updateObject: uo, updatePlanting: up } =
nudgeCtx.current nudgeCtx.current
if (!canNudge) return if (!canNudge) return
const dir = DIRS[e.key]
if (!dir) return
const el = document.activeElement
if (el && (el.tagName === 'INPUT' || el.tagName === 'TEXTAREA' || el.tagName === 'SELECT')) return
const s = useEditorStore.getState() const s = useEditorStore.getState()
if (s.objectDragging) return // don't fight an active pointer drag if (s.objectDragging) return // don't fight an active pointer drag
const step = e.shiftKey ? 10 : 1
if (s.selectedId != null) { if (s.selectedId != null) {
const base = s.liveObject?.id === s.selectedId ? s.liveObject : objs.find((o) => o.id === s.selectedId) const base = s.liveObject?.id === s.selectedId ? s.liveObject : objs.find((o) => o.id === s.selectedId)
if (!base) return if (!base) return
e.preventDefault() s.setLiveObject({ ...base, xCm: base.xCm + dx, yCm: base.yCm + dy })
s.setLiveObject({ ...base, xCm: base.xCm + dir[0] * step, yCm: base.yCm + dir[1] * step })
commitLater(() => { commitLater(() => {
const live = useEditorStore.getState().liveObject const live = useEditorStore.getState().liveObject
if (live?.id !== s.selectedId) return // a drag cleared it and committed if (live?.id !== s.selectedId) return
uo.mutate({ id: live.id, version: live.version, xCm: live.xCm, yCm: live.yCm }) uo.mutate({ id: live.id, version: live.version, xCm: live.xCm, yCm: live.yCm })
useEditorStore.getState().setLiveObject(null) useEditorStore.getState().setLiveObject(null)
}) })
@@ -333,10 +320,9 @@ export function GardenEditorPage() {
const base = const base =
s.livePlanting?.id === s.selectedPlantingId ? s.livePlanting : plops.find((p) => p.id === s.selectedPlantingId) s.livePlanting?.id === s.selectedPlantingId ? s.livePlanting : plops.find((p) => p.id === s.selectedPlantingId)
if (!base) return if (!base) return
e.preventDefault()
const obj = objs.find((o) => o.id === base.objectId) const obj = objs.find((o) => o.id === base.objectId)
let nx = base.xCm + dir[0] * step let nx = base.xCm + dx
let ny = base.yCm + dir[1] * step let ny = base.yCm + dy
if (obj) { if (obj) {
nx = Math.max(-obj.widthCm / 2, Math.min(obj.widthCm / 2, nx)) nx = Math.max(-obj.widthCm / 2, Math.min(obj.widthCm / 2, nx))
ny = Math.max(-obj.heightCm / 2, Math.min(obj.heightCm / 2, ny)) ny = Math.max(-obj.heightCm / 2, Math.min(obj.heightCm / 2, ny))
@@ -350,6 +336,28 @@ export function GardenEditorPage() {
}) })
} }
} }
// Keyboard nudging (desktop): arrows move the selection 1cm, Shift = 10cm — the
// same nudgeSelected the touch pad uses. Mounted once; a pending commit is
// flushed on unmount so a nudge in flight isn't lost.
useEffect(() => {
const DIRS: Record<string, [number, number]> = {
ArrowUp: [0, -1],
ArrowDown: [0, 1],
ArrowLeft: [-1, 0],
ArrowRight: [1, 0],
}
function onKey(e: KeyboardEvent) {
const dir = DIRS[e.key]
if (!dir) return
const el = document.activeElement
if (el && (el.tagName === 'INPUT' || el.tagName === 'TEXTAREA' || el.tagName === 'SELECT')) return
const s = useEditorStore.getState()
if (s.selectedId == null && s.selectedPlantingId == null) return
e.preventDefault()
const step = e.shiftKey ? 10 : 1
nudgeSelected(dir[0] * step, dir[1] * step)
}
window.addEventListener('keydown', onKey) window.addEventListener('keydown', onKey)
return () => { return () => {
window.removeEventListener('keydown', onKey) window.removeEventListener('keydown', onKey)
@@ -627,6 +635,12 @@ export function GardenEditorPage() {
)} )}
<div className="relative min-h-0 flex-1"> <div className="relative min-h-0 flex-1">
<GardenCanvas garden={garden} objects={objects} plantings={plantings} plantsById={plantsById} canEdit={canEdit} /> <GardenCanvas garden={garden} objects={objects} plantings={plantings} plantsById={plantsById} canEdit={canEdit} />
{/* Touch fine-positioning: the keyboard's arrow-nudge has no equivalent
on a phone, and dragging can't hit single-cm precision. Shown while
something's selected on a touch layout (#104). */}
{canEdit && (selectedId != null || selectedPlantingId != null) && (
<NudgePad onNudge={nudgeSelected} />
)}
</div> </div>
{/* Empty-state hints (non-interactive overlays). */} {/* Empty-state hints (non-interactive overlays). */}
@@ -822,6 +836,41 @@ function FillControl({ onFill, busy }: { onFill: (layout: FillLayout) => void; b
) )
} }
// On-screen nudge pad (#104): 1cm arrows for the selected object/plop on touch,
// where the keyboard's arrow-nudge isn't reachable and a drag can't hit single-cm
// precision. md:hidden — desktop uses the keyboard. Wired to the same
// nudgeSelected, so it shares the live-then-debounced-PATCH behaviour.
function NudgePad({ onNudge }: { onNudge: (dx: number, dy: number) => void }) {
const btn =
'flex size-10 items-center justify-center rounded-md border border-border bg-surface/90 text-fg ' +
Review

size-10 (40px touch target) duplicated in btn class and center label span

maintainability · flagged by 1 model

  • web/src/pages/GardenEditorPage.tsx:845,861 — the 40px target size (size-10) lives in two places: the btn class string (line 845) and again on the center "1cm" label span (line 861). If the touch target ever needs tuning, both must move. A shared class or applying btn to the label too would keep them in sync. Trivial duplication.

🪰 Gadfly · advisory

⚪ **size-10 (40px touch target) duplicated in btn class and center label span** _maintainability · flagged by 1 model_ - `web/src/pages/GardenEditorPage.tsx:845,861` — the `40px` target size (`size-10`) lives in two places: the `btn` class string (line 845) and again on the center `"1cm"` label span (line 861). If the touch target ever needs tuning, both must move. A shared class or applying `btn` to the label too would keep them in sync. Trivial duplication. <sub>🪰 Gadfly · advisory</sub>
'shadow-sm outline-none backdrop-blur transition-colors active:bg-border/70 focus-visible:ring-2 focus-visible:ring-accent/40'
return (
<div
role="group"
aria-label="Nudge selection by 1cm"
className="absolute bottom-2 left-2 z-20 grid grid-cols-3 grid-rows-3 gap-0.5 md:hidden"
>
<span />
<button type="button" className={btn} aria-label="Nudge up" onClick={() => onNudge(0, -1)}>
</button>
<span />
<button type="button" className={btn} aria-label="Nudge left" onClick={() => onNudge(-1, 0)}>
</button>
<span className="flex size-10 items-center justify-center text-[0.6rem] font-medium text-muted">1cm</span>
<button type="button" className={btn} aria-label="Nudge right" onClick={() => onNudge(1, 0)}>
</button>
<span />
<button type="button" className={btn} aria-label="Nudge down" onClick={() => onNudge(0, 1)}>
</button>
<span />
</div>
)
}
// The mobile primary mode switch (#99): one always-there tab bar so "placing // The mobile primary mode switch (#99): one always-there tab bar so "placing
// beds", "planting", "journaling" and "assistant" stop competing for the same // beds", "planting", "journaling" and "assistant" stop competing for the same
// strip. Assistant is dropped when the instance has no model configured. // strip. Assistant is dropped when the instance has no model configured.