From 941aea30110464203a9369f9bc94e01666c6623a Mon Sep 17 00:00:00 2001 From: Quang Tran <16215255+trmquang93@users.noreply.github.com> Date: Tue, 17 Mar 2026 23:10:55 +0700 Subject: [PATCH] fix: prevent hotspot move on double-click and open edit modal Add drag threshold to hotspot reposition to prevent accidental moves during double-click. Introduce "reposition-pending" intermediate state that requires 5px mouse movement before committing to a drag. Double-clicking a hotspot now opens the HotspotModal for editing. Add ESC key support to close HotspotModal. --- src/Drawd.jsx | 12 +++++++++- src/components/HotspotModal.jsx | 10 +++++++- src/components/ScreenNode.jsx | 6 ++++- src/hooks/useCanvasMouseHandlers.js | 37 ++++++++++++++++++++++++----- src/hooks/useHotspotInteraction.js | 5 ++-- 5 files changed, 58 insertions(+), 12 deletions(-) diff --git a/src/Drawd.jsx b/src/Drawd.jsx index c872ce2..53e5dce 100644 --- a/src/Drawd.jsx +++ b/src/Drawd.jsx @@ -322,7 +322,7 @@ export default function Drawd() { // ── Canvas event handlers ────────────────────────────────────────────────────────── const { onCanvasMouseDown, onCanvasMouseMove, onCanvasMouseUp, onCanvasMouseLeave, canvasCursor } = useCanvasMouseHandlers({ - hotspotInteraction, setHotspotInteraction, commitDragSnapshot, + hotspotInteraction, setHotspotInteraction, captureDragSnapshot, commitDragSnapshot, screens, moveHotspot, moveHotspotToScreen, resizeHotspot, updateConnection, connecting, setConnecting, cancelConnecting, selectedConnection, setSelectedConnection, @@ -393,6 +393,15 @@ export default function Drawd() { setHotspotModal({ screen, hotspot: null }); }, [screens]); + const onHotspotDoubleClick = useCallback((_e, screenId, hotspotId) => { + const screen = screens.find((s) => s.id === screenId); + if (!screen) return; + const hotspot = screen.hotspots.find((h) => h.id === hotspotId); + if (!hotspot) return; + setHotspotInteraction(null); + setHotspotModal({ screen, hotspot }); + }, [screens, setHotspotInteraction]); + const addHotspotViaConnect = useCallback((screenId) => { onStartConnect(screenId); }, [onStartConnect]); @@ -613,6 +622,7 @@ export default function Drawd() { ? new Set(selectedHotspots.map((h) => h.hotspotId)) : null} onHotspotMouseDown={onHotspotMouseDown} + onHotspotDoubleClick={onHotspotDoubleClick} onImageAreaMouseDown={onImageAreaMouseDown} onHotspotDragHandleMouseDown={onHotspotDragHandleMouseDown} onResizeHandleMouseDown={onResizeHandleMouseDown} diff --git a/src/components/HotspotModal.jsx b/src/components/HotspotModal.jsx index c47c4dd..4265e19 100644 --- a/src/components/HotspotModal.jsx +++ b/src/components/HotspotModal.jsx @@ -1,4 +1,4 @@ -import { useState } from "react"; +import { useState, useEffect } from "react"; import { COLORS, FONTS, styles } from "../styles/theme"; import { generateId } from "../utils/generateId"; @@ -129,6 +129,14 @@ export function HotspotModal({ screen, hotspot, connection, screens, documents = const [validationPattern, setValidationPattern] = useState(hotspot?.validation?.pattern || ""); const [validationErrorMessage, setValidationErrorMessage] = useState(hotspot?.validation?.errorMessage || ""); + useEffect(() => { + const handleKeyDown = (e) => { + if (e.key === "Escape") onClose(); + }; + window.addEventListener("keydown", handleKeyDown); + return () => window.removeEventListener("keydown", handleKeyDown); + }, [onClose]); + const otherScreens = screens.filter((s) => s.id !== screen.id); const selectedDoc = documents.find((d) => d.id === documentId) || null; diff --git a/src/components/ScreenNode.jsx b/src/components/ScreenNode.jsx index 703a487..72f39af 100644 --- a/src/components/ScreenNode.jsx +++ b/src/components/ScreenNode.jsx @@ -5,7 +5,7 @@ import { DEFAULT_SCREEN_WIDTH, DEFAULT_IMAGE_HEIGHT, HEADER_HEIGHT, DESCRIPTION_ export function ScreenNode({ screen, selected, onSelect, onDragStart, onAddHotspot, onRemoveScreen, onDotDragStart, onConnectTarget, onHoverTarget, isConnectHoverTarget, isConnecting, - selectedHotspotId, selectedHotspotIds, onHotspotMouseDown, onImageAreaMouseDown, + selectedHotspotId, selectedHotspotIds, onHotspotMouseDown, onHotspotDoubleClick, onImageAreaMouseDown, onHotspotDragHandleMouseDown, onResizeHandleMouseDown, onScreenDimensions, drawRect, isHotspotDragging, onUpdateDescription, isSpaceHeld, onAddState, onDropImage, activeTool, @@ -361,6 +361,10 @@ export function ScreenNode({ e.stopPropagation(); if (onHotspotMouseDown) onHotspotMouseDown(e, screen.id, hs.id); }} + onDoubleClick={(e) => { + e.stopPropagation(); + if (onHotspotDoubleClick) onHotspotDoubleClick(e, screen.id, hs.id); + }} style={{ position: "absolute", left: `${hs.x}%`, diff --git a/src/hooks/useCanvasMouseHandlers.js b/src/hooks/useCanvasMouseHandlers.js index 71db838..3bde23f 100644 --- a/src/hooks/useCanvasMouseHandlers.js +++ b/src/hooks/useCanvasMouseHandlers.js @@ -10,6 +10,7 @@ export function useCanvasMouseHandlers({ // hotspot interaction hotspotInteraction, setHotspotInteraction, + captureDragSnapshot, commitDragSnapshot, screens, moveHotspot, @@ -66,7 +67,7 @@ export function useCanvasMouseHandlers({ if (selectedHotspots.length > 0) setSelectedHotspots([]); setSelectedStickyNote?.(null); setSelectedScreenGroup?.(null); - if (hotspotInteraction && hotspotInteraction.mode !== "draw" && hotspotInteraction.mode !== "reposition" && hotspotInteraction.mode !== "hotspot-drag" && hotspotInteraction.mode !== "resize" && hotspotInteraction.mode !== "conn-endpoint-drag") { + if (hotspotInteraction && hotspotInteraction.mode !== "draw" && hotspotInteraction.mode !== "reposition" && hotspotInteraction.mode !== "reposition-pending" && hotspotInteraction.mode !== "hotspot-drag" && hotspotInteraction.mode !== "resize" && hotspotInteraction.mode !== "conn-endpoint-drag") { setHotspotInteraction(null); } handleCanvasMouseDown(e); @@ -76,7 +77,7 @@ export function useCanvasMouseHandlers({ // Space+click: always pan, skip all interaction guards if (isSpaceHeld.current) { if (selectedConnection) setSelectedConnection(null); - if (hotspotInteraction && hotspotInteraction.mode !== "draw" && hotspotInteraction.mode !== "reposition" && hotspotInteraction.mode !== "hotspot-drag" && hotspotInteraction.mode !== "resize" && hotspotInteraction.mode !== "conn-endpoint-drag") { + if (hotspotInteraction && hotspotInteraction.mode !== "draw" && hotspotInteraction.mode !== "reposition" && hotspotInteraction.mode !== "reposition-pending" && hotspotInteraction.mode !== "hotspot-drag" && hotspotInteraction.mode !== "resize" && hotspotInteraction.mode !== "conn-endpoint-drag") { setHotspotInteraction(null); } handleCanvasMouseDown(e); @@ -97,14 +98,14 @@ export function useCanvasMouseHandlers({ setSelectedStickyNote?.(null); setSelectedScreenGroup?.(null); // Cancel hotspot interaction on canvas click - if (hotspotInteraction && hotspotInteraction.mode !== "draw" && hotspotInteraction.mode !== "reposition" && hotspotInteraction.mode !== "hotspot-drag" && hotspotInteraction.mode !== "resize" && hotspotInteraction.mode !== "conn-endpoint-drag") { + if (hotspotInteraction && hotspotInteraction.mode !== "draw" && hotspotInteraction.mode !== "reposition" && hotspotInteraction.mode !== "reposition-pending" && hotspotInteraction.mode !== "hotspot-drag" && hotspotInteraction.mode !== "resize" && hotspotInteraction.mode !== "conn-endpoint-drag") { setHotspotInteraction(null); } if (connecting) { if (connecting.mode === "click") cancelConnecting(); return; } - if (hotspotInteraction?.mode === "draw" || hotspotInteraction?.mode === "reposition" || hotspotInteraction?.mode === "hotspot-drag" || hotspotInteraction?.mode === "resize" || hotspotInteraction?.mode === "conn-endpoint-drag") { + if (hotspotInteraction?.mode === "draw" || hotspotInteraction?.mode === "reposition" || hotspotInteraction?.mode === "reposition-pending" || hotspotInteraction?.mode === "hotspot-drag" || hotspotInteraction?.mode === "resize" || hotspotInteraction?.mode === "conn-endpoint-drag") { return; } const result = handleCanvasMouseDown(e); @@ -139,6 +140,17 @@ export function useCanvasMouseHandlers({ return; } + if (hotspotInteraction?.mode === "reposition-pending") { + // Only transition to full reposition after exceeding drag threshold (5px) + const dx = e.clientX - hotspotInteraction.startClientX; + const dy = e.clientY - hotspotInteraction.startClientY; + if (Math.abs(dx) >= 5 || Math.abs(dy) >= 5) { + captureDragSnapshot(); + setHotspotInteraction((prev) => ({ ...prev, mode: "reposition" })); + } + return; + } + if (hotspotInteraction?.mode === "reposition") { // Track world cursor position — hotspot stays at original position until mouseup const rect = canvasRef.current.getBoundingClientRect(); @@ -209,7 +221,7 @@ export function useCanvasMouseHandlers({ if (screenMoves.length > 0) moveScreens(screenMoves); stickyMoves.forEach((item) => updateStickyNote(item.id, { x: item.x, y: item.y })); } - }, [handleMouseMove, moveScreen, moveScreens, updateStickyNote, connecting, setConnecting, canvasRef, pan, zoom, hotspotInteraction, setHotspotInteraction, screens, resizeHotspot, rubberBand, updateRubberBand]); + }, [handleMouseMove, moveScreen, moveScreens, updateStickyNote, connecting, setConnecting, canvasRef, pan, zoom, hotspotInteraction, setHotspotInteraction, screens, resizeHotspot, rubberBand, updateRubberBand, captureDragSnapshot]); const onCanvasMouseUp = useCallback((e) => { // Handle connection endpoint drag completion @@ -243,6 +255,12 @@ export function useCanvasMouseHandlers({ return; } + // Handle reposition-pending mouseup: threshold not reached, revert to selected + if (hotspotInteraction?.mode === "reposition-pending") { + setHotspotInteraction({ mode: "selected", screenId: hotspotInteraction.screenId, hotspotId: hotspotInteraction.hotspotId }); + return; + } + // Handle reposition completion (may transfer hotspot to another screen) if (hotspotInteraction?.mode === "reposition") { const rect = canvasRef.current.getBoundingClientRect(); @@ -330,6 +348,11 @@ export function useCanvasMouseHandlers({ setHotspotInteraction(null); return; } + if (hotspotInteraction?.mode === "reposition-pending") { + // Threshold not reached — revert to selected, no snapshot to commit + setHotspotInteraction({ mode: "selected", screenId: hotspotInteraction.screenId, hotspotId: hotspotInteraction.hotspotId }); + return; + } if (hotspotInteraction?.mode === "reposition") { // Reposition doesn't move the hotspot during drag, so just cancel — no snapshot to commit setHotspotInteraction({ mode: "selected", screenId: hotspotInteraction.screenId, hotspotId: hotspotInteraction.hotspotId }); @@ -367,7 +390,9 @@ export function useCanvasMouseHandlers({ ? (resizeCursors[hotspotInteraction.handle] || "default") : hotspotInteraction?.mode === "reposition" ? "grabbing" - : (spaceHeld && isPanning) ? "grabbing" + : hotspotInteraction?.mode === "reposition-pending" + ? "grab" + : (spaceHeld && isPanning) ? "grabbing" : spaceHeld ? "grab" : isPanning ? "grabbing" : "default"; diff --git a/src/hooks/useHotspotInteraction.js b/src/hooks/useHotspotInteraction.js index d38fb66..5951bc0 100644 --- a/src/hooks/useHotspotInteraction.js +++ b/src/hooks/useHotspotInteraction.js @@ -52,13 +52,12 @@ export function useHotspotInteraction({ if (selectedHotspots.length > 0) setSelectedHotspots([]); if (hotspotInteraction?.mode === "selected" && hotspotInteraction.hotspotId === hotspotId) { - // Same hotspot selected again -> begin reposition + // Same hotspot selected again -> enter pending reposition (drag threshold required) const screen = screens.find((s) => s.id === screenId); const hs = screen?.hotspots.find((h) => h.id === hotspotId); if (!screen || !hs) return; - captureDragSnapshot(); setHotspotInteraction({ - mode: "reposition", + mode: "reposition-pending", screenId, hotspotId, offsetPct: { dx: 0, dy: 0 },