Skip to content

Commit 87f996d

Browse files
committed
fix(comment): keep pins glued to their element across iframe scroll
Root cause: overlay.ts captured getBoundingClientRect() once at click time and sent viewport-relative coords to the parent. The PinOverlay sits outside the iframe and has no way to observe iframe-internal scroll, so the pin stayed at the old viewport coordinates while the selected element moved with the scrolled content. Fix: iframe becomes the source of truth for rect. - New WATCH_SELECTORS message (parent→iframe) registers the selectors the parent cares about for the current design/snapshot. - New ELEMENT_RECTS message (iframe→parent) broadcasts current rects on scroll/resize, rAF-coalesced so bursts collapse into one post. - Parent stores a liveRects map keyed by selector; PinOverlay and the CommentBubble prefer it over the stored rect when present. - Freshly-pinned elements auto-add themselves to the iframe's watch list so scroll tracking works before the parent round-trip lands. DB schema untouched — stored rect is still the initial value before the iframe broadcasts its first update. liveRects is cleared on design switch. Test coverage: overlay rAF coalescing, scroll re-broadcast, selector resolution fallbacks, ELEMENT_RECTS handler validation, store merge/clear semantics, PinOverlay rendering with live rect override.
1 parent 189184f commit 87f996d

9 files changed

Lines changed: 523 additions & 68 deletions

File tree

apps/desktop/src/renderer/src/components/PreviewPane.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ describe('handlePreviewMessage trust boundary', () => {
4848
return {
4949
onElementSelected: vi.fn(),
5050
onIframeError: vi.fn(),
51+
onElementRects: vi.fn(),
5152
};
5253
}
5354

@@ -112,6 +113,42 @@ describe('handlePreviewMessage trust boundary', () => {
112113
expect(errorOutcome).toEqual({ status: 'handled', type: 'IFRAME_ERROR' });
113114
expect(handlers.onIframeError).toHaveBeenCalledOnce();
114115
});
116+
117+
it('accepts well-formed ELEMENT_RECTS payloads and forwards entries', () => {
118+
const handlers = makeHandlers();
119+
const outcome = handlePreviewMessage(
120+
{
121+
__codesign: true,
122+
type: 'ELEMENT_RECTS',
123+
entries: [
124+
{ selector: '#a', rect: { top: 10, left: 20, width: 30, height: 40 } },
125+
{ selector: '[data-codesign-id="x"]', rect: { top: 1, left: 2, width: 3, height: 4 } },
126+
],
127+
},
128+
handlers,
129+
);
130+
expect(outcome).toEqual({ status: 'handled', type: 'ELEMENT_RECTS' });
131+
expect(handlers.onElementRects).toHaveBeenCalledOnce();
132+
const payload = handlers.onElementRects.mock.calls[0]?.[0] as {
133+
entries: Array<{ selector: string }>;
134+
};
135+
expect(payload.entries).toHaveLength(2);
136+
expect(payload.entries[0]?.selector).toBe('#a');
137+
});
138+
139+
it('rejects ELEMENT_RECTS with a malformed rect entry', () => {
140+
const handlers = makeHandlers();
141+
const outcome = handlePreviewMessage(
142+
{
143+
__codesign: true,
144+
type: 'ELEMENT_RECTS',
145+
entries: [{ selector: '#bad', rect: { top: 'NaN' } }],
146+
},
147+
handlers,
148+
);
149+
expect(outcome.status).toBe('rejected');
150+
expect(handlers.onElementRects).not.toHaveBeenCalled();
151+
});
115152
});
116153

117154
describe('postModeToPreviewWindow', () => {

apps/desktop/src/renderer/src/components/PreviewPane.tsx

Lines changed: 116 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { useT } from '@open-codesign/i18n';
22
import {
3+
type ElementRectsMessage,
34
type IframeErrorMessage,
45
type OverlayMessage,
56
buildSrcdoc,
7+
isElementRectsMessage,
68
isIframeErrorMessage,
79
isOverlayMessage,
810
} from '@open-codesign/runtime';
@@ -69,11 +71,12 @@ export function scaleRectForZoom(
6971
};
7072
}
7173

72-
export type AllowedPreviewMessageType = 'ELEMENT_SELECTED' | 'IFRAME_ERROR';
74+
export type AllowedPreviewMessageType = 'ELEMENT_SELECTED' | 'IFRAME_ERROR' | 'ELEMENT_RECTS';
7375

7476
export interface PreviewMessageHandlers {
7577
onElementSelected: (msg: OverlayMessage) => void;
7678
onIframeError: (msg: IframeErrorMessage) => void;
79+
onElementRects: (msg: ElementRectsMessage) => void;
7780
}
7881

7982
export type PreviewMessageOutcome =
@@ -105,6 +108,12 @@ export function handlePreviewMessage(
105108
return { status: 'handled', type: 'IFRAME_ERROR' };
106109
}
107110
return { status: 'rejected', reason: 'shape', type: envelope.type };
111+
case 'ELEMENT_RECTS':
112+
if (isElementRectsMessage(data)) {
113+
handlers.onElementRects(data);
114+
return { status: 'handled', type: 'ELEMENT_RECTS' };
115+
}
116+
return { status: 'rejected', reason: 'shape', type: envelope.type };
108117
default:
109118
return { status: 'rejected', reason: 'unknown-type', type: envelope.type };
110119
}
@@ -276,6 +285,9 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
276285
const openCommentBubble = useCodesignStore((s) => s.openCommentBubble);
277286
const closeCommentBubble = useCodesignStore((s) => s.closeCommentBubble);
278287
const addComment = useCodesignStore((s) => s.addComment);
288+
const applyLiveRects = useCodesignStore((s) => s.applyLiveRects);
289+
const clearLiveRects = useCodesignStore((s) => s.clearLiveRects);
290+
const liveRects = useCodesignStore((s) => s.liveRects);
279291

280292
// Active iframe ref consumed by TweakPanel (postMessage target) and by the
281293
// window.message guard. We re-point this whenever the active design changes
@@ -304,7 +316,35 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
304316
if (el) {
305317
postModeToPreviewWindow(el.contentWindow, interactionMode, pushIframeError);
306318
}
307-
}, [currentDesignId, interactionMode, pushIframeError]);
319+
// New iframe / new design → liveRects from the old one are stale.
320+
clearLiveRects();
321+
}, [currentDesignId, interactionMode, pushIframeError, clearLiveRects]);
322+
323+
// Tell the sandbox which selectors to track. The sandbox re-measures each
324+
// on scroll/resize and broadcasts ELEMENT_RECTS; we merge into liveRects.
325+
// Selectors: all comments on the current snapshot + the active bubble's
326+
// selector (usually the freshly-pinned one, included for the moment
327+
// between click and save).
328+
// biome-ignore lint/correctness/useExhaustiveDependencies: currentDesignId is intentional — iframeRef.current is a ref so biome can't see that it changes with the active design. When the design switches we MUST resend WATCH_SELECTORS to the newly-active iframe; the currentDesignId dependency is what triggers that re-run.
329+
useEffect(() => {
330+
const win = iframeRef.current?.contentWindow;
331+
if (!win) return;
332+
const selectors = new Set<string>();
333+
if (currentSnapshotId) {
334+
for (const c of comments) {
335+
if (c.snapshotId === currentSnapshotId) selectors.add(c.selector);
336+
}
337+
}
338+
if (commentBubble) selectors.add(commentBubble.selector);
339+
try {
340+
win.postMessage(
341+
{ __codesign: true, type: 'WATCH_SELECTORS', selectors: Array.from(selectors) },
342+
'*',
343+
);
344+
} catch {
345+
/* sandbox gone — retry happens next render */
346+
}
347+
}, [comments, currentSnapshotId, commentBubble, currentDesignId]);
308348

309349
useEffect(() => {
310350
function onMessage(event: MessageEvent): void {
@@ -334,6 +374,9 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
334374
},
335375
onIframeError: (msg) =>
336376
pushIframeError(formatIframeError(msg.kind, msg.message, msg.source, msg.lineno)),
377+
onElementRects: (msg) => {
378+
applyLiveRects(msg.entries);
379+
},
337380
});
338381

339382
if (outcome.status === 'rejected' && outcome.reason === 'unknown-type') {
@@ -343,7 +386,7 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
343386

344387
window.addEventListener('message', onMessage);
345388
return () => window.removeEventListener('message', onMessage);
346-
}, [pushIframeError, selectCanvasElement, openCommentBubble, previewZoom]);
389+
}, [pushIframeError, selectCanvasElement, openCommentBubble, previewZoom, applyLiveRects]);
347390

348391
// Pool entries: active design first (using the freshest in-memory
349392
// previewHtml), then any other recently-visited designs that still have a
@@ -379,21 +422,23 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
379422
<PinOverlay
380423
comments={snapshotComments}
381424
zoom={previewZoom}
382-
onPinClick={(c) =>
425+
liveRects={liveRects}
426+
onPinClick={(c) => {
427+
const live = liveRects[c.selector] ?? c.rect;
383428
openCommentBubble({
384429
selector: c.selector,
385430
tag: c.tag,
386431
outerHTML: c.outerHTML,
387432
rect: {
388-
top: c.rect.top * (previewZoom / 100),
389-
left: c.rect.left * (previewZoom / 100),
390-
width: c.rect.width * (previewZoom / 100),
391-
height: c.rect.height * (previewZoom / 100),
433+
top: live.top * (previewZoom / 100),
434+
left: live.left * (previewZoom / 100),
435+
width: live.width * (previewZoom / 100),
436+
height: live.height * (previewZoom / 100),
392437
},
393438
existingCommentId: c.id,
394439
initialText: c.text,
395-
})
396-
}
440+
});
441+
}}
397442
/>
398443
);
399444

@@ -481,54 +526,67 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
481526
{body}
482527
{previewHtml ? <TweakPanel iframeRef={iframeRef} /> : null}
483528
</div>
484-
{commentBubble && interactionMode === 'comment' ? (
485-
<CommentBubble
486-
key={commentBubble.selector}
487-
selector={commentBubble.selector}
488-
tag={commentBubble.tag}
489-
outerHTML={commentBubble.outerHTML}
490-
rect={commentBubble.rect}
491-
{...(commentBubble.initialText !== undefined
492-
? { initialText: commentBubble.initialText }
493-
: {})}
494-
onClose={() => {
495-
const win = iframeRef.current?.contentWindow;
496-
if (win) {
497-
try {
498-
win.postMessage({ __codesign: true, type: 'CLEAR_PIN' }, '*');
499-
} catch {
500-
/* noop */
501-
}
502-
}
503-
closeCommentBubble();
504-
}}
505-
onSendToClaude={async (text: string) => {
506-
await addComment({
507-
kind: 'edit',
508-
selector: commentBubble.selector,
509-
tag: commentBubble.tag,
510-
outerHTML: commentBubble.outerHTML,
511-
rect: commentBubble.rect,
512-
text,
513-
scope: 'element',
514-
...(commentBubble.parentOuterHTML
515-
? { parentOuterHTML: commentBubble.parentOuterHTML }
516-
: {}),
517-
});
518-
const win = iframeRef.current?.contentWindow;
519-
if (win) {
520-
try {
521-
win.postMessage({ __codesign: true, type: 'CLEAR_PIN' }, '*');
522-
} catch {
523-
/* noop */
524-
}
525-
}
526-
closeCommentBubble();
527-
// Stage only — user clicks the "Apply" button on the chip bar
528-
// to send all accumulated edits in one go.
529-
}}
530-
/>
531-
) : null}
529+
{commentBubble && interactionMode === 'comment'
530+
? (() => {
531+
const liveForBubble = liveRects[commentBubble.selector];
532+
const scaled = liveForBubble
533+
? {
534+
top: liveForBubble.top * (previewZoom / 100),
535+
left: liveForBubble.left * (previewZoom / 100),
536+
width: liveForBubble.width * (previewZoom / 100),
537+
height: liveForBubble.height * (previewZoom / 100),
538+
}
539+
: commentBubble.rect;
540+
return (
541+
<CommentBubble
542+
key={commentBubble.selector}
543+
selector={commentBubble.selector}
544+
tag={commentBubble.tag}
545+
outerHTML={commentBubble.outerHTML}
546+
rect={scaled}
547+
{...(commentBubble.initialText !== undefined
548+
? { initialText: commentBubble.initialText }
549+
: {})}
550+
onClose={() => {
551+
const win = iframeRef.current?.contentWindow;
552+
if (win) {
553+
try {
554+
win.postMessage({ __codesign: true, type: 'CLEAR_PIN' }, '*');
555+
} catch {
556+
/* noop */
557+
}
558+
}
559+
closeCommentBubble();
560+
}}
561+
onSendToClaude={async (text: string) => {
562+
await addComment({
563+
kind: 'edit',
564+
selector: commentBubble.selector,
565+
tag: commentBubble.tag,
566+
outerHTML: commentBubble.outerHTML,
567+
rect: commentBubble.rect,
568+
text,
569+
scope: 'element',
570+
...(commentBubble.parentOuterHTML
571+
? { parentOuterHTML: commentBubble.parentOuterHTML }
572+
: {}),
573+
});
574+
const win = iframeRef.current?.contentWindow;
575+
if (win) {
576+
try {
577+
win.postMessage({ __codesign: true, type: 'CLEAR_PIN' }, '*');
578+
} catch {
579+
/* noop */
580+
}
581+
}
582+
closeCommentBubble();
583+
// Stage only — user clicks the "Apply" button on the chip bar
584+
// to send all accumulated edits in one go.
585+
}}
586+
/>
587+
);
588+
})()
589+
: null}
532590
</div>
533591
</div>
534592
);

apps/desktop/src/renderer/src/components/comment/PinOverlay.test.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { CommentRow } from '@open-codesign/shared';
22
import { describe, expect, it } from 'vitest';
3-
import { pinStyle, variantFor } from './PinOverlay';
3+
import { pinStyle, pinStyleFromRect, variantFor } from './PinOverlay';
44

55
function comment(overrides: Partial<CommentRow> = {}): CommentRow {
66
return {
@@ -50,4 +50,12 @@ describe('pinStyle', () => {
5050
// scale = 0.5; top = 100*0.5 - 10 = 40; left = 50*0.5 + 80*0.5 - 10 = 55
5151
expect(pos).toEqual({ top: '40px', left: '55px' });
5252
});
53+
54+
it('pinStyleFromRect uses the supplied rect instead of the stored one', () => {
55+
// This is the path PinOverlay takes when liveRects has an entry for the
56+
// comment's selector: the badge stays glued to the element's current
57+
// position in the iframe viewport, even after the user scrolls.
58+
const live = pinStyleFromRect({ top: 10, left: 5, width: 80, height: 40 }, 100);
59+
expect(live).toEqual({ top: '0px', left: '75px' });
60+
});
5361
});

apps/desktop/src/renderer/src/components/comment/PinOverlay.tsx

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useT } from '@open-codesign/i18n';
2-
import type { CommentRow } from '@open-codesign/shared';
2+
import type { CommentRect, CommentRow } from '@open-codesign/shared';
33

44
export interface PinOverlayProps {
55
/** Comments filtered to the currently-viewed snapshot. */
@@ -8,6 +8,10 @@ export interface PinOverlayProps {
88
zoom: number;
99
/** Called when a pin is clicked; opens the CommentBubble anchored at its rect. */
1010
onPinClick: (comment: CommentRow) => void;
11+
/** Live, iframe-viewport-relative rects by selector. When present for a
12+
* comment's selector, it overrides the stored rect — this is how pins
13+
* stay glued to their element across iframe scroll/resize. Unscaled. */
14+
liveRects?: Record<string, CommentRect>;
1115
}
1216

1317
export interface PinVariant {
@@ -47,24 +51,29 @@ export function variantFor(comment: CommentRow): PinVariant {
4751
};
4852
}
4953

50-
export function pinStyle(comment: CommentRow, zoom: number): { top: string; left: string } {
54+
export function pinStyleFromRect(rect: CommentRect, zoom: number): { top: string; left: string } {
5155
const scale = zoom / 100;
5256
// Position pin at the outer top-right corner: half-overlapping the corner
5357
// so it reads as a "badge" attached to the element rather than floating
5458
// randomly. Pin is 20px, so offset by -10 = half outside.
55-
const top = comment.rect.top * scale - 10;
56-
const left = comment.rect.left * scale + comment.rect.width * scale - 10;
59+
const top = rect.top * scale - 10;
60+
const left = rect.left * scale + rect.width * scale - 10;
5761
return { top: `${top}px`, left: `${left}px` };
5862
}
5963

60-
export function PinOverlay({ comments, zoom, onPinClick }: PinOverlayProps) {
64+
export function pinStyle(comment: CommentRow, zoom: number): { top: string; left: string } {
65+
return pinStyleFromRect(comment.rect, zoom);
66+
}
67+
68+
export function PinOverlay({ comments, zoom, onPinClick, liveRects }: PinOverlayProps) {
6169
const t = useT();
6270
if (comments.length === 0) return null;
6371
return (
6472
<div className="pointer-events-none absolute inset-0 z-[5]">
6573
{comments.map((comment, index) => {
6674
const v = variantFor(comment);
67-
const pos = pinStyle(comment, zoom);
75+
const rect = liveRects?.[comment.selector] ?? comment.rect;
76+
const pos = pinStyleFromRect(rect, zoom);
6877
const label =
6978
comment.kind === 'note'
7079
? t('pinOverlay.note', { n: index + 1 })

0 commit comments

Comments
 (0)