feat(review): group-aware comment targeting — resolve clicks to stable children
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.
This commit is contained in:
@@ -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,
|
||||||
|
};
|
||||||
|
}
|
||||||
@@ -34,6 +34,7 @@ import { UploadManager } from '../stores/uploadManager';
|
|||||||
import { InboxZone } from '../canvas/InboxZone';
|
import { InboxZone } from '../canvas/InboxZone';
|
||||||
import { getItemWorldBounds } from '../canvas/SceneManager';
|
import { getItemWorldBounds } from '../canvas/SceneManager';
|
||||||
import { getPointAnchorWorld } from '../canvas/reviewAnchors';
|
import { getPointAnchorWorld } from '../canvas/reviewAnchors';
|
||||||
|
import { resolveReviewTargetAtPoint } from '../canvas/reviewTargeting';
|
||||||
import type { TextObject } from '../canvas/scene-format';
|
import type { TextObject } from '../canvas/scene-format';
|
||||||
import { VideoSprite } from '../canvas/sprites/VideoSprite';
|
import { VideoSprite } from '../canvas/sprites/VideoSprite';
|
||||||
import * as ops from '../canvas/operations';
|
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
|
// Priority 2: scene object (place draft pin) — only for editor/owner
|
||||||
const scene = canvasRef.current?.getScene();
|
const scene = canvasRef.current?.getScene();
|
||||||
if (scene && userRole !== 'viewer') {
|
if (scene && userRole !== 'viewer') {
|
||||||
// Use 10px screen-space radius converted to world space for forgiving click detection
|
|
||||||
const clickRadius = 10 / vp.scale.x;
|
const clickRadius = 10 / vp.scale.x;
|
||||||
const hitItems = scene.queryRegion(worldPos.x - clickRadius, worldPos.y - clickRadius, clickRadius * 2, clickRadius * 2);
|
const target = resolveReviewTargetAtPoint({
|
||||||
// Take the topmost (highest z) item
|
scene,
|
||||||
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));
|
|
||||||
setDraftPin({
|
|
||||||
objectId: item.id,
|
|
||||||
pinX,
|
|
||||||
pinY,
|
|
||||||
worldX: worldPos.x,
|
worldX: worldPos.x,
|
||||||
worldY: worldPos.y,
|
worldY: worldPos.y,
|
||||||
|
clickRadius,
|
||||||
|
});
|
||||||
|
// Defensive guard: never attach a comment to a group
|
||||||
|
if (target && target.objectType !== 'group') {
|
||||||
|
setDraftPin({
|
||||||
|
objectId: target.objectId,
|
||||||
|
pinX: target.pinX,
|
||||||
|
pinY: target.pinY,
|
||||||
|
worldX: target.worldX,
|
||||||
|
worldY: target.worldY,
|
||||||
});
|
});
|
||||||
setFocusedThreadId(null);
|
setFocusedThreadId(null);
|
||||||
return;
|
return;
|
||||||
|
|||||||
Reference in New Issue
Block a user