Touch ergonomics: bigger handles + on-screen nudge pad (#104) #117
@@ -3,7 +3,17 @@
|
||||
// one place instead of drifting between files.
|
||||
|
||||
export const SELECT_COLOR = '#2f7a3e' // selection stroke/handles
|
||||
export const HANDLE_PX = 12 // on-screen size of a drag/resize handle
|
||||
// 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.
|
||||
|
|
||||
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
|
||||
|
||||
|
||||
@@ -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'
|
||||
@@ -283,13 +284,67 @@ export function GardenEditorPage() {
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [])
|
||||
|
||||
// Desktop keyboard nudging: arrows move the selected object/plop 1cm (Shift =
|
||||
// 10cm); the PATCH is debounced ~400ms on key-idle so a held key doesn't spam.
|
||||
// Plops nudge in their object's local frame and clamp to its (local) bounds.
|
||||
// Mounted once, reading live values from nudgeCtx so a data refetch can't
|
||||
// re-subscribe and cancel a pending commit; the pending commit is flushed on
|
||||
// unmount, and a fire only commits if its live value is still present (a drag
|
||||
// that cleared it already committed its own PATCH).
|
||||
// Move the selected object/plop by (dx, dy) cm: apply live geometry instantly,
|
||||
// then commit ONE debounced PATCH so a burst of nudges (a held arrow key, or
|
||||
// repeated taps of the touch pad) doesn't spam the server. Plops clamp to their
|
||||
// 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).
|
||||
// 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(() => {
|
||||
nudgeTimer.current = null
|
||||
const fn = nudgeFire.current
|
||||
nudgeFire.current = null
|
||||
|
gitea-actions
commented
🟠 nudgeSelected captured by empty-deps effect; refs-only invariant is now implicit and unguarded maintainability · flagged by 2 models
🪰 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 = useCallback((dx: number, dy: number) => {
|
||||
const { canEdit: canNudge, objects: objs, plantings: plops, updateObject: uo, updatePlanting: up } =
|
||||
nudgeCtx.current
|
||||
if (!canNudge) return
|
||||
const s = useEditorStore.getState()
|
||||
if (s.objectDragging) return // don't fight an active pointer drag
|
||||
if (s.selectedId != null) {
|
||||
const base = s.liveObject?.id === s.selectedId ? s.liveObject : objs.find((o) => o.id === s.selectedId)
|
||||
if (!base) return
|
||||
s.setLiveObject({ ...base, xCm: base.xCm + dx, yCm: base.yCm + dy })
|
||||
commitLater(() => {
|
||||
const live = useEditorStore.getState().liveObject
|
||||
if (live?.id !== s.selectedId) return
|
||||
uo.mutate({ id: live.id, version: live.version, xCm: live.xCm, yCm: live.yCm })
|
||||
useEditorStore.getState().setLiveObject(null)
|
||||
})
|
||||
} else if (s.selectedPlantingId != null) {
|
||||
const base =
|
||||
s.livePlanting?.id === s.selectedPlantingId ? s.livePlanting : plops.find((p) => p.id === s.selectedPlantingId)
|
||||
if (!base) return
|
||||
const obj = objs.find((o) => o.id === base.objectId)
|
||||
let nx = base.xCm + dx
|
||||
let ny = base.yCm + dy
|
||||
if (obj) {
|
||||
nx = Math.max(-obj.widthCm / 2, Math.min(obj.widthCm / 2, nx))
|
||||
ny = Math.max(-obj.heightCm / 2, Math.min(obj.heightCm / 2, ny))
|
||||
}
|
||||
s.setLivePlanting({ ...base, xCm: nx, yCm: ny })
|
||||
commitLater(() => {
|
||||
const live = useEditorStore.getState().livePlanting
|
||||
if (live?.id !== s.selectedPlantingId) return
|
||||
up.mutate({ id: live.id, version: live.version, xCm: live.xCm, yCm: live.yCm })
|
||||
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
|
||||
// flushed on unmount so a nudge in flight isn't lost.
|
||||
useEffect(() => {
|
||||
const DIRS: Record<string, [number, number]> = {
|
||||
ArrowUp: [0, -1],
|
||||
@@ -297,58 +352,16 @@ export function GardenEditorPage() {
|
||||
ArrowLeft: [-1, 0],
|
||||
ArrowRight: [1, 0],
|
||||
}
|
||||
const commitLater = (fire: () => void) => {
|
||||
nudgeFire.current = fire
|
||||
if (nudgeTimer.current != null) window.clearTimeout(nudgeTimer.current)
|
||||
nudgeTimer.current = window.setTimeout(() => {
|
||||
nudgeTimer.current = null
|
||||
const fn = nudgeFire.current
|
||||
nudgeFire.current = null
|
||||
fn?.()
|
||||
}, 400)
|
||||
}
|
||||
function onKey(e: KeyboardEvent) {
|
||||
const { canEdit: canNudge, objects: objs, plantings: plops, updateObject: uo, updatePlanting: up } =
|
||||
nudgeCtx.current
|
||||
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()
|
||||
if (s.objectDragging) return // don't fight an active pointer drag
|
||||
if (s.selectedId == null && s.selectedPlantingId == null) return
|
||||
e.preventDefault()
|
||||
const step = e.shiftKey ? 10 : 1
|
||||
if (s.selectedId != null) {
|
||||
const base = s.liveObject?.id === s.selectedId ? s.liveObject : objs.find((o) => o.id === s.selectedId)
|
||||
if (!base) return
|
||||
e.preventDefault()
|
||||
s.setLiveObject({ ...base, xCm: base.xCm + dir[0] * step, yCm: base.yCm + dir[1] * step })
|
||||
commitLater(() => {
|
||||
const live = useEditorStore.getState().liveObject
|
||||
if (live?.id !== s.selectedId) return // a drag cleared it and committed
|
||||
uo.mutate({ id: live.id, version: live.version, xCm: live.xCm, yCm: live.yCm })
|
||||
useEditorStore.getState().setLiveObject(null)
|
||||
})
|
||||
} else if (s.selectedPlantingId != null) {
|
||||
const base =
|
||||
s.livePlanting?.id === s.selectedPlantingId ? s.livePlanting : plops.find((p) => p.id === s.selectedPlantingId)
|
||||
if (!base) return
|
||||
e.preventDefault()
|
||||
const obj = objs.find((o) => o.id === base.objectId)
|
||||
let nx = base.xCm + dir[0] * step
|
||||
let ny = base.yCm + dir[1] * step
|
||||
if (obj) {
|
||||
nx = Math.max(-obj.widthCm / 2, Math.min(obj.widthCm / 2, nx))
|
||||
ny = Math.max(-obj.heightCm / 2, Math.min(obj.heightCm / 2, ny))
|
||||
}
|
||||
s.setLivePlanting({ ...base, xCm: nx, yCm: ny })
|
||||
commitLater(() => {
|
||||
const live = useEditorStore.getState().livePlanting
|
||||
if (live?.id !== s.selectedPlantingId) return
|
||||
up.mutate({ id: live.id, version: live.version, xCm: live.xCm, yCm: live.yCm })
|
||||
useEditorStore.getState().setLivePlanting(null)
|
||||
})
|
||||
}
|
||||
nudgeSelected(dir[0] * step, dir[1] * step)
|
||||
}
|
||||
window.addEventListener('keydown', onKey)
|
||||
return () => {
|
||||
@@ -627,6 +640,13 @@ 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 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>
|
||||
|
||||
{/* Empty-state hints (non-interactive overlays). */}
|
||||
@@ -822,6 +842,41 @@ function FillControl({ onFill, busy }: { onFill: (layout: FillLayout) => void; b
|
||||
)
|
||||
}
|
||||
|
||||
// On-screen nudge pad (#104): 1cm arrows for the selected object/plop on a touch
|
||||
|
gitea-actions
commented
⚪ size-10 (40px touch target) duplicated in btn class and center label span maintainability · flagged by 1 model
🪰 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 ' +
|
||||
'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"
|
||||
>
|
||||
<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
|
||||
// beds", "planting", "journaling" and "assistant" stop competing for the same
|
||||
// strip. Assistant is dropped when the instance has no model configured.
|
||||
|
||||
Reference in New Issue
Block a user
🟠 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)').matcheswill throw aTypeErrorin environments withoutmatchMedia(or if it returnsundefined) because the optional chain only guards the call, not the.matchesaccess on its result. The expression should bewindow.matchMedia?.('(pointer: coarse)')?.matchesso the property read is also short-circuited. Fix: add a second?.before.matches.🪰 Gadfly · advisory