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 30 additions and 19 deletions
Showing only changes of commit b0e11bce17 - Show all commits
+11 -6
View File
@@ -3,12 +3,17 @@
// one place instead of drifting between files.
export const SELECT_COLOR = '#2f7a3e' // selection stroke/handles
// 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 =
typeof window !== 'undefined' && window.matchMedia?.('(pointer: coarse)').matches ? 22 : 12
// Whether the primary pointer is a fingertip rather than a mouse — the one signal
// the touch affordances key off (bigger handles here, the on-screen nudge pad in
// the editor), so they can't disagree about what "touch" means. Read once at
// load; a device doesn't switch its primary pointer mid-session, and the optional
// chain keeps it false (mouse defaults) under test / SSR where matchMedia is absent.
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>
export const isCoarsePointer =
typeof window !== 'undefined' && !!window.matchMedia?.('(pointer: coarse)').matches
// On-screen size of a drag/resize handle. Bigger on 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).
export const HANDLE_PX = isCoarsePointer ? 22 : 12
export const MIN_RADIUS_CM = 1 // smallest plop radius
export const DIMMED_OPACITY = 0.4 // non-focused objects/plops in focus mode
+19 -13
View File
@@ -1,4 +1,4 @@
import { useEffect, useMemo, useRef, useState } from 'react'
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { getRouteApi } from '@tanstack/react-router'
import { Alert } from '@/components/ui/Alert'
import { Button } from '@/components/ui/Button'
@@ -18,6 +18,7 @@ import { EditorHint } from '@/editor/EditorHint'
import { SeasonBanner, SeasonPicker } from '@/editor/SeasonPicker'
import { objectDisplayName } from '@/editor/kinds'
import { useEditorStore, type EditorMode } from '@/editor/store'
import { isCoarsePointer } from '@/editor/shared'
import { cn } from '@/lib/cn'
import type { EditorGarden } from '@/editor/types'
import { ShareGardenModal } from '@/components/gardens/ShareGardenModal'
@@ -289,7 +290,11 @@ export function GardenEditorPage() {
// object's local bounds. Reads live values from getState/nudgeCtx so it's
// correct whichever surface calls it; a commit only fires if its live value is
// still present (a drag that cleared it already committed its own PATCH).
const commitLater = (fire: () => void) => {
// Stable across renders (empty deps): both read live values through refs
// (nudgeCtx) / getState, never through closed-over props, so a mount-once
// consumer (the keydown effect) and a memo-friendly one (NudgePad) both get a
// function that stays current without a new identity each render.
const commitLater = useCallback((fire: () => void) => {
nudgeFire.current = fire
if (nudgeTimer.current != null) window.clearTimeout(nudgeTimer.current)
nudgeTimer.current = window.setTimeout(() => {
@@ -298,9 +303,9 @@ export function GardenEditorPage() {
nudgeFire.current = null
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>
fn?.()
}, 400)
}
}, [])
const nudgeSelected = (dx: number, dy: number) => {
const nudgeSelected = useCallback((dx: number, dy: number) => {
const { canEdit: canNudge, objects: objs, plantings: plops, updateObject: uo, updatePlanting: up } =
nudgeCtx.current
if (!canNudge) return
@@ -335,7 +340,7 @@ export function GardenEditorPage() {
useEditorStore.getState().setLivePlanting(null)
})
}
}
}, [commitLater])
// Keyboard nudging (desktop): arrows move the selection 1cm, Shift = 10cm — the
// same nudgeSelected the touch pad uses. Mounted once; a pending commit is
@@ -636,9 +641,10 @@ export function GardenEditorPage() {
<div className="relative min-h-0 flex-1">
<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) && (
on a touch device, and dragging can't hit single-cm precision. Shown
on a coarse pointer (same signal as the bigger handles) while
something's selected (#104). */}
{canEdit && isCoarsePointer && (selectedId != null || selectedPlantingId != null) && (
<NudgePad onNudge={nudgeSelected} />
)}
</div>
@@ -836,10 +842,10 @@ 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.
// On-screen nudge pad (#104): 1cm arrows for the selected object/plop on a touch
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>
// device, where the keyboard's arrow-nudge isn't reachable and a drag can't hit
// single-cm precision. Rendered only on a coarse pointer (the caller gates it).
// Wired to the same nudgeSelected, so it shares the live-then-debounced-PATCH.
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 ' +
@@ -848,7 +854,7 @@ function NudgePad({ onNudge }: { onNudge: (dx: number, dy: number) => void }) {
<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"
className="absolute bottom-2 left-2 z-20 grid grid-cols-3 grid-rows-3 gap-0.5"
>
<span />
<button type="button" className={btn} aria-label="Nudge up" onClick={() => onNudge(0, -1)}>