steve/pansy
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

Canvas: make double-click-to-plant work on desktop #135

Merged
steve merged 3 commits from fix/double-click-focus into main 2026-08-23 07:26:50 +00:00
Conversation 3 Commits 3 Files Changed 2
+21 -1
1 changed files with 21 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 916b2989f5 - Show all commits
web/src/editor/Canvas.tsx
+21 -1
View File
@@ -329,6 +329,16 @@ export const Canvas = forwardRef<
if (!drag.current) drag.current = { t: 'pan', sx: e.clientX, sy: e.clientY, tx: st.vp.tx, ty: st.vp.ty, moved: false, th: thresh(e) } if (!drag.current) drag.current = { t: 'pan', sx: e.clientX, sy: e.clientY, tx: st.vp.tx, ty: st.vp.ty, moved: false, th: thresh(e) }
} }
// The last press on an object, for double-click detection. Detected here
// rather than with onDoubleClick on the object's <g>: track() captures the
// pointer on the SVG root, and capture retargets the compatibility click
// and dblclick events to the root, so a dblclick handler on the object
// never fires — which is why "double-click a bed to plant it" did nothing
// from the Organic rebuild until this was noticed (2026-08-23).
const lastPress = useRef<{ id: number; at: number; x: number; y: number } | null>(null)
gitea-actions commented 2026-08-23 07:21:35 +00:00
Outdated
Review
Copy Link
Copy Source

🟡 lastPress useRef declared far from the component's other refs (lines 87-97), breaking the established grouping pattern

maintainability · flagged by 3 models

  • lastPress ref declared out-of-group (web/src/editor/Canvas.tsx:338): All other useRef hooks are declared together at lines 87–97. lastPress is inserted ~250 lines later, next to the function that uses it. A reader scanning for "which refs does this component own?" will miss it. It should sit with its peers at the top of the hook block — the proximity to objDown doesn't add anything because refs don't have render-time side-effects.

🪰 Gadfly · advisory

🟡 **lastPress useRef declared far from the component's other refs (lines 87-97), breaking the established grouping pattern** _maintainability · flagged by 3 models_ - **`lastPress` ref declared out-of-group** (`web/src/editor/Canvas.tsx:338`): All other `useRef` hooks are declared together at lines 87–97. `lastPress` is inserted ~250 lines later, next to the function that uses it. A reader scanning for "which refs does this component own?" will miss it. It should sit with its peers at the top of the hook block — the proximity to `objDown` doesn't add anything because refs don't have render-time side-effects. <sub>🪰 Gadfly · advisory</sub>
const DOUBLE_PRESS_MS = 400
const DOUBLE_PRESS_PX = 12
const objDown = (o: EditorObject) => (e: ReactPointerEvent) => { const objDown = (o: EditorObject) => (e: ReactPointerEvent) => {
e.stopPropagation() e.stopPropagation()
track(e) track(e)
@@ -342,6 +352,17 @@ export const Canvas = forwardRef<
} }
// Dimmed siblings stay inert inside a focused bed. // Dimmed siblings stay inert inside a focused bed.
if (st.focusId != null && o.id !== st.focusId) return if (st.focusId != null && o.id !== st.focusId) return
const prev = lastPress.current
gitea-actions commented 2026-08-23 07:21:35 +00:00
Outdated
Review
Copy Link
Copy Source

🟠 Double-click detection accepts non-primary pointer buttons, unlike native dblclick

correctness, error-handling · flagged by 2 models

  • web/src/editor/Canvas.tsx:357 — lastPress is updated for every pointer button, so non-primary clicks (right-click, middle-click) can participate in or complete a double-click. Verified by reading objDown: the ref is written unconditionally before the e.button check, and the if (prev && …) block that fires focusObject does not inspect e.button. Native dblclick fires only for the primary button; right-clicking a bed twice within 400 ms should not focus it. Fix: gate the `last…

🪰 Gadfly · advisory

🟠 **Double-click detection accepts non-primary pointer buttons, unlike native dblclick** _correctness, error-handling · flagged by 2 models_ - **`web/src/editor/Canvas.tsx:357` — `lastPress` is updated for every pointer button, so non-primary clicks (right-click, middle-click) can participate in or complete a double-click.** Verified by reading `objDown`: the ref is written unconditionally before the `e.button` check, and the `if (prev && …)` block that fires `focusObject` does not inspect `e.button`. Native `dblclick` fires only for the primary button; right-clicking a bed twice within 400 ms should not focus it. Fix: gate the `last… <sub>🪰 Gadfly · advisory</sub>
const now = performance.now()
lastPress.current = { id: o.id, at: now, x: e.clientX, y: e.clientY }
if (prev && prev.id === o.id && now - prev.at < DOUBLE_PRESS_MS && Math.hypot(e.clientX - prev.x, e.clientY - prev.y) < DOUBLE_PRESS_PX) {
// Second press of a double-click: focus the bed to plant it. No drag
// starts, so the matching pointerup has nothing to select into Plot.
lastPress.current = null
drag.current = null
if (o.plantable) focusObject(o)
return
}
if (!latest.current.canEdit) { if (!latest.current.canEdit) {
st.setSel({ type: 'object', id: o.id }) st.setSel({ type: 'object', id: o.id })
st.setTab('plot') st.setTab('plot')
@@ -562,7 +583,6 @@ export const Canvas = forwardRef<
opacity={dim ? DIM_OBJECT : 1} opacity={dim ? DIM_OBJECT : 1}
style={{ cursor: dim ? 'default' : canEdit ? 'grab' : 'pointer' }} style={{ cursor: dim ? 'default' : canEdit ? 'grab' : 'pointer' }}
onPointerDown={objDown(o)} onPointerDown={objDown(o)}
onDoubleClick={o.plantable && !dim ? () => focusObject(o) : undefined}
> >
{o.shape === 'circle' ? ( {o.shape === 'circle' ? (
<ellipse rx={o.widthCm / 2} ry={o.heightCm / 2} {...common} /> <ellipse rx={o.widthCm / 2} ry={o.heightCm / 2} {...common} />