From ecd4408d8af79e703c1ace520cc11152729664d4 Mon Sep 17 00:00:00 2001 From: Hiren Kangad Date: Thu, 12 Mar 2026 20:08:25 +0530 Subject: [PATCH] =?UTF-8?q?feat(review):=20group-aware=20comment=20targeti?= =?UTF-8?q?ng=20=E2=80=94=20resolve=20clicks=20to=20stable=20children?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New reviewTargeting.ts provides resolveReviewTargetAtPoint() which: - descends into groups to find the deepest commentable child under the click - walks children in z-order (front-to-back) for correct visual stacking - handles nested groups recursively - returns null if no valid child exists under the point Editor.tsx review click handler now delegates to the resolver instead of raw queryRegion + topmost-z pick. Defensive guard rejects group objectIds before draft creation. Existing group-anchored threads continue to render and open normally. --- frontend/src/canvas/reviewTargeting.ts | 143 +++++++++++++++++++++++++ frontend/src/pages/Editor.tsx | 27 ++--- 2 files changed, 157 insertions(+), 13 deletions(-) create mode 100644 frontend/src/canvas/reviewTargeting.ts diff --git a/frontend/src/canvas/reviewTargeting.ts b/frontend/src/canvas/reviewTargeting.ts new file mode 100644 index 0000000..8903bd4 --- /dev/null +++ b/frontend/src/canvas/reviewTargeting.ts @@ -0,0 +1,143 @@ +import type { SceneManager, SceneItem } from './SceneManager'; +import { getItemWorldBounds } from './SceneManager'; +import type { GroupObject } from './scene-format'; + +export interface ReviewTarget { + objectId: string; + objectType: string; + worldX: number; + worldY: number; + pinX: number; + pinY: number; +} + +/** + * Returns true if the item is a valid target for comment attachment. + * Groups are NOT commentable — comments should attach to stable children. + */ +export function isCommentableObject(item: SceneItem): boolean { + if (item.data.type === 'group') return false; + if (!item.data.visible) return false; + return true; +} + +/** + * Point-in-bounds test using world-space coordinates. + */ +function pointInBounds( + wx: number, + wy: number, + b: { x: number; y: number; w: number; h: number }, +): boolean { + return wx >= b.x && wx <= b.x + b.w && wy >= b.y && wy <= b.y + b.h; +} + +/** + * Resolve the deepest commentable child inside a group under the given world point. + * Walks children in front-to-back order (highest z first) and recurses into nested groups. + */ +function resolveGroupChild( + scene: SceneManager, + group: SceneItem, + worldX: number, + worldY: number, +): SceneItem | null { + const groupData = group.data as GroupObject; + + // Collect children that exist in the scene, sorted front-to-back (highest z first) + const children: SceneItem[] = []; + for (const childId of groupData.children) { + const child = scene.getById(childId); + if (child) children.push(child); + } + children.sort((a, b) => (b.data.z || 0) - (a.data.z || 0)); + + for (const child of children) { + if (!child.data.visible) continue; + const bounds = getItemWorldBounds(child); + if (!pointInBounds(worldX, worldY, bounds)) continue; + + // Nested group — recurse + if (child.data.type === 'group') { + const nested = resolveGroupChild(scene, child, worldX, worldY); + if (nested) return nested; + // No valid child inside nested group under this point — skip + continue; + } + + // Found a commentable child + if (isCommentableObject(child)) return child; + } + + return null; +} + +/** + * Resolve a world-space click into a valid review anchor target. + * + * - If the topmost hit is a commentable non-group object, returns it. + * - If the topmost hit is a group, descends into children to find the deepest + * valid child under the click point. + * - Returns null if no valid target exists (empty space, group with no child + * under the cursor, etc.) + * + * @param clickRadius - World-space radius for forgiving hit detection + */ +export function resolveReviewTargetAtPoint(params: { + scene: SceneManager; + worldX: number; + worldY: number; + clickRadius: number; +}): ReviewTarget | null { + const { scene, worldX, worldY, clickRadius } = params; + + // Query spatial grid with a small radius for forgiving click detection + const hitItems = scene.queryRegion( + worldX - clickRadius, + worldY - clickRadius, + clickRadius * 2, + clickRadius * 2, + ); + + if (hitItems.length === 0) return null; + + // Sort topmost first (highest z) + hitItems.sort((a, b) => (b.data.z || 0) - (a.data.z || 0)); + + for (const item of hitItems) { + if (!item.data.visible) continue; + + // If it's a group, try to resolve to a child + if (item.data.type === 'group') { + const child = resolveGroupChild(scene, item, worldX, worldY); + if (child) return buildTarget(child, worldX, worldY); + // No valid child under point — continue checking other hits + continue; + } + + // Non-group: check if commentable + if (isCommentableObject(item)) { + return buildTarget(item, worldX, worldY); + } + } + + return null; +} + +/** + * Build a ReviewTarget from a resolved SceneItem and the click point. + */ +function buildTarget(item: SceneItem, worldX: number, worldY: number): ReviewTarget { + const bounds = getItemWorldBounds(item); + const pinX = bounds.w > 0 ? Math.max(0, Math.min(1, (worldX - bounds.x) / bounds.w)) : 0.5; + const pinY = bounds.h > 0 ? Math.max(0, Math.min(1, (worldY - bounds.y) / bounds.h)) : 0.5; + + return { + objectId: item.id, + objectType: item.data.type, + worldX, + worldY, + pinX, + pinY, + }; +} diff --git a/frontend/src/pages/Editor.tsx b/frontend/src/pages/Editor.tsx index 1d92865..0cb23c0 100644 --- a/frontend/src/pages/Editor.tsx +++ b/frontend/src/pages/Editor.tsx @@ -34,6 +34,7 @@ import { UploadManager } from '../stores/uploadManager'; import { InboxZone } from '../canvas/InboxZone'; import { getItemWorldBounds } from '../canvas/SceneManager'; import { getPointAnchorWorld } from '../canvas/reviewAnchors'; +import { resolveReviewTargetAtPoint } from '../canvas/reviewTargeting'; import type { TextObject } from '../canvas/scene-format'; import { VideoSprite } from '../canvas/sprites/VideoSprite'; import * as ops from '../canvas/operations'; @@ -291,21 +292,21 @@ export default function Editor({ isPublicView }: EditorProps) { // Priority 2: scene object (place draft pin) — only for editor/owner const scene = canvasRef.current?.getScene(); if (scene && userRole !== 'viewer') { - // Use 10px screen-space radius converted to world space for forgiving click detection const clickRadius = 10 / vp.scale.x; - const hitItems = scene.queryRegion(worldPos.x - clickRadius, worldPos.y - clickRadius, clickRadius * 2, clickRadius * 2); - // Take the topmost (highest z) item - if (hitItems.length > 0) { - const item = hitItems.sort((a: any, b: any) => (b.data.z || 0) - (a.data.z || 0))[0]; - const bounds = getItemWorldBounds(item); - const pinX = Math.max(0, Math.min(1, (worldPos.x - bounds.x) / bounds.w)); - const pinY = Math.max(0, Math.min(1, (worldPos.y - bounds.y) / bounds.h)); + const target = resolveReviewTargetAtPoint({ + scene, + worldX: worldPos.x, + worldY: worldPos.y, + clickRadius, + }); + // Defensive guard: never attach a comment to a group + if (target && target.objectType !== 'group') { setDraftPin({ - objectId: item.id, - pinX, - pinY, - worldX: worldPos.x, - worldY: worldPos.y, + objectId: target.objectId, + pinX: target.pinX, + pinY: target.pinY, + worldX: target.worldX, + worldY: target.worldY, }); setFocusedThreadId(null); return;