Skip to content

Commit b52e321

Browse files
committed
fix(comment): dedup edits, preserve drafts, keep bubble open on failure
Four concrete bugs in the comment send flow, plus two UX hardening items. Layered on top of the liveRects pin-follow-scroll work so anchor math stays as-is — this change is strictly about the send/edit/close loop. 1. Editing a reopened chip duplicated instead of updating the original. existingCommentId was defined on the anchor but never consumed. New submitComment action routes by id: with → updateComment (no new row), without → addComment. updateComment now returns CommentRow | null so callers can detect write failures. 2. addComment / updateComment failures (no snapshot, IPC error, validator reject) silently closed the bubble and wiped the user's draft. The bubble now checks the return value and stays open on null. The toast surfaced by the store is enough context for the user. 3. Mousedown-outside auto-close discarded in-flight text whenever the user clicked any surrounding UI — toolbar, sidebar, preview. Replaced with explicit Esc / × only, matching chat/dialog norms. 4. React key used selector alone, so two comments on the same element shared state across mounts. Now edit:<id> / new:<selector>. Plus: - Unsent drafts persist across chip / element switches via a ref-backed Map keyed by bubbleKey. Cleared on successful submit; explicit close deliberately preserves so users can resume. - CommentChipBar's Apply button now toasts "onboardingIncomplete" when config isn't ready, instead of letting sendPrompt silently early-return.
1 parent 680709b commit b52e321

4 files changed

Lines changed: 105 additions & 18 deletions

File tree

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

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
296296
const commentBubble = useCodesignStore((s) => s.commentBubble);
297297
const openCommentBubble = useCodesignStore((s) => s.openCommentBubble);
298298
const closeCommentBubble = useCodesignStore((s) => s.closeCommentBubble);
299-
const addComment = useCodesignStore((s) => s.addComment);
299+
const submitComment = useCodesignStore((s) => s.submitComment);
300300
const applyLiveRects = useCodesignStore((s) => s.applyLiveRects);
301301
const clearLiveRects = useCodesignStore((s) => s.clearLiveRects);
302302
const liveRects = useCodesignStore((s) => s.liveRects);
@@ -305,6 +305,11 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
305305
// window.message guard. We re-point this whenever the active design changes
306306
// or the active iframe element re-mounts.
307307
const iframeRef = useRef<HTMLIFrameElement | null>(null);
308+
// Unsent bubble drafts, keyed by bubbleKey (edit:<id> | new:<selector>).
309+
// Lives across bubble remounts so switching to another chip / element and
310+
// coming back restores the text the user had typed. Cleared on successful
311+
// submit; explicit close (Esc / ×) deliberately preserves.
312+
const bubbleDraftsRef = useRef<Map<string, string>>(new Map());
308313
const iframesByDesign = useRef<Map<string, HTMLIFrameElement>>(new Map());
309314
// Bumped every time the active iframe fires onLoad — used to re-trigger
310315
// the WATCH_SELECTORS effect so we don't race past overlay installation
@@ -551,16 +556,28 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
551556
const scaled = liveForBubble
552557
? scaleRectForZoom(liveForBubble, previewZoom)
553558
: commentBubble.rect;
559+
const existingId = commentBubble.existingCommentId;
560+
// Keying by comment id (when editing) rather than selector alone
561+
// means two comments on the same element each get their own draft
562+
// state and don't stomp each other on reopen.
563+
const bubbleKey = existingId ? `edit:${existingId}` : `new:${commentBubble.selector}`;
564+
// Draft precedence: prior unsent draft for this anchor > DB text
565+
// on a reopened chip > empty. This preserves mid-typing context
566+
// when the user clicks another chip and comes back.
567+
const stashed = bubbleDraftsRef.current.get(bubbleKey);
568+
const initialText = stashed ?? commentBubble.initialText;
554569
return (
555570
<CommentBubble
556-
key={commentBubble.selector}
571+
key={bubbleKey}
557572
selector={commentBubble.selector}
558573
tag={commentBubble.tag}
559574
outerHTML={commentBubble.outerHTML}
560575
rect={scaled}
561-
{...(commentBubble.initialText !== undefined
562-
? { initialText: commentBubble.initialText }
563-
: {})}
576+
{...(initialText !== undefined ? { initialText } : {})}
577+
onDraftChange={(text) => {
578+
if (text.length === 0) bubbleDraftsRef.current.delete(bubbleKey);
579+
else bubbleDraftsRef.current.set(bubbleKey, text);
580+
}}
564581
onClose={() => {
565582
const win = iframeRef.current?.contentWindow;
566583
if (win) {
@@ -573,18 +590,26 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
573590
closeCommentBubble();
574591
}}
575592
onSendToClaude={async (text: string) => {
576-
await addComment({
593+
const row = await submitComment({
577594
kind: 'edit',
578595
selector: commentBubble.selector,
579596
tag: commentBubble.tag,
580597
outerHTML: commentBubble.outerHTML,
581598
rect: commentBubble.rect,
582599
text,
583600
scope: 'element',
601+
...(existingId ? { existingCommentId: existingId } : {}),
584602
...(commentBubble.parentOuterHTML
585603
? { parentOuterHTML: commentBubble.parentOuterHTML }
586604
: {}),
587605
});
606+
// On failure (no snapshot, IPC error, duplicate) keep the
607+
// bubble open so the user's draft survives. A toast has
608+
// already been surfaced by the store layer.
609+
if (!row) return;
610+
// Persisted — wipe the stashed draft so the next open
611+
// starts clean (a reopened chip re-reads from DB).
612+
bubbleDraftsRef.current.delete(bubbleKey);
588613
const win = iframeRef.current?.contentWindow;
589614
if (win) {
590615
try {

apps/desktop/src/renderer/src/components/chat/CommentChipBar.tsx

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,32 @@ export function CommentChipBar() {
1919
const removeComment = useCodesignStore((s) => s.removeComment);
2020
const previewZoom = useCodesignStore((s) => s.previewZoom);
2121
const sendPrompt = useCodesignStore((s) => s.sendPrompt);
22+
const pushToast = useCodesignStore((s) => s.pushToast);
23+
const config = useCodesignStore((s) => s.config);
2224
const isGenerating = useCodesignStore(
2325
(s) => s.isGenerating && s.generatingDesignId === s.currentDesignId,
2426
);
2527

2628
const pending = comments.filter((c) => c.kind === 'edit' && c.status === 'pending');
2729
if (pending.length === 0) return null;
2830

31+
const isReady =
32+
config?.hasKey === true && config.provider !== null && config.modelPrimary !== null;
33+
34+
function handleApply(): void {
35+
// sendPrompt silently early-returns on !isReady or during another run.
36+
// The button disables during `isGenerating`, but onboarding-incomplete
37+
// users would otherwise click Apply to no effect. Surface it explicitly.
38+
if (!isReady) {
39+
pushToast({
40+
variant: 'error',
41+
title: t('notifications.onboardingIncomplete'),
42+
});
43+
return;
44+
}
45+
void sendPrompt({ prompt: '' });
46+
}
47+
2948
return (
3049
<div className="flex flex-wrap items-center gap-[var(--space-1_5)]">
3150
<ul
@@ -77,7 +96,7 @@ export function CommentChipBar() {
7796
<button
7897
type="button"
7998
disabled={isGenerating}
80-
onClick={() => void sendPrompt({ prompt: '' })}
99+
onClick={handleApply}
81100
className="inline-flex items-center gap-[var(--space-1)] h-[24px] px-[var(--space-2_5)] rounded-full bg-[var(--color-accent)] text-[var(--color-on-accent)] text-[var(--text-2xs)] font-medium hover:bg-[var(--color-accent-hover)] active:scale-[var(--scale-press-down)] disabled:opacity-50 transition-[transform,background-color] duration-[var(--duration-faster)] shrink-0"
82101
aria-label={t('commentChip.applyAll')}
83102
>

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ export interface CommentBubbleProps {
99
outerHTML: string;
1010
rect: { top: number; left: number; width: number; height: number };
1111
initialText?: string;
12+
/** Called on every keystroke so the host (PreviewPane) can persist an
13+
* unsent draft keyed by anchor id. Without this, switching to a different
14+
* chip / element silently discarded the current text. */
15+
onDraftChange?: (text: string) => void;
1216
onClose: () => void;
1317
onSendToClaude: (text: string) => Promise<void> | void;
1418
}
@@ -30,6 +34,7 @@ export function CommentBubble({
3034
outerHTML,
3135
rect,
3236
initialText,
37+
onDraftChange,
3338
onClose,
3439
onSendToClaude,
3540
}: CommentBubbleProps) {
@@ -45,19 +50,17 @@ export function CommentBubble({
4550
}, []);
4651

4752
useEffect(() => {
53+
// Esc + the × button are the only ways to close. The previous mousedown-
54+
// outside handler silently discarded the user's draft whenever they
55+
// clicked surrounding UI (toolbar, sidebar, preview) — the single most
56+
// frustrating failure mode. Explicit close mirrors how chat / dialog UIs
57+
// treat in-progress text.
4858
function onKey(e: KeyboardEvent) {
4959
if (e.key === 'Escape') onClose();
5060
}
51-
function onDocClick(e: MouseEvent) {
52-
if (!rootRef.current) return;
53-
if (e.target instanceof Node && rootRef.current.contains(e.target)) return;
54-
onClose();
55-
}
5661
document.addEventListener('keydown', onKey);
57-
document.addEventListener('mousedown', onDocClick);
5862
return () => {
5963
document.removeEventListener('keydown', onKey);
60-
document.removeEventListener('mousedown', onDocClick);
6164
};
6265
}, [onClose]);
6366

@@ -119,7 +122,9 @@ export function CommentBubble({
119122
ref={textareaRef}
120123
value={draft}
121124
onChange={(e) => {
122-
setDraft(e.target.value);
125+
const next = e.target.value;
126+
setDraft(next);
127+
onDraftChange?.(next);
123128
const el = e.currentTarget;
124129
el.style.height = 'auto';
125130
el.style.height = `${Math.min(el.scrollHeight, 120)}px`;

apps/desktop/src/renderer/src/store.ts

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,22 @@ interface CodesignState {
294294
scope?: CommentScope;
295295
parentOuterHTML?: string;
296296
}) => Promise<CommentRow | null>;
297-
updateComment: (id: string, patch: { text?: string }) => Promise<void>;
297+
updateComment: (id: string, patch: { text?: string }) => Promise<CommentRow | null>;
298+
/** Single entry point used by CommentBubble. If `existingCommentId` is set,
299+
* routes to updateComment (editing a saved comment); otherwise addComment
300+
* (creating a new one). Returns the resulting row on success, null on
301+
* failure — callers must check before closing UI so drafts aren't lost. */
302+
submitComment: (input: {
303+
existingCommentId?: string;
304+
kind: CommentKind;
305+
selector: string;
306+
tag: string;
307+
outerHTML: string;
308+
rect: CommentRect;
309+
text: string;
310+
scope?: CommentScope;
311+
parentOuterHTML?: string;
312+
}) => Promise<CommentRow | null>;
298313
removeComment: (id: string) => Promise<void>;
299314
/** Replace the live rects map — called from PreviewPane when the sandbox
300315
* broadcasts an ELEMENT_RECTS message. Entries are iframe-viewport-relative
@@ -2073,23 +2088,46 @@ export const useCodesignStore = create<CodesignState>((set, get) => ({
20732088
},
20742089

20752090
async updateComment(id, patch) {
2076-
if (!window.codesign) return;
2091+
if (!window.codesign) return null;
20772092
try {
20782093
const updated = await window.codesign.comments.update(id, patch);
2079-
if (!updated) return;
2094+
if (!updated) return null;
20802095
set((s) => ({
20812096
comments: s.comments.map((c) => (c.id === id ? updated : c)),
20822097
}));
2098+
return updated;
20832099
} catch (err) {
20842100
const msg = err instanceof Error ? err.message : tr('errors.unknown');
20852101
get().pushToast({
20862102
variant: 'error',
20872103
title: tr('notifications.commentUpdateFailed'),
20882104
description: msg,
20892105
});
2106+
return null;
20902107
}
20912108
},
20922109

2110+
async submitComment(input) {
2111+
// Route by presence of existingCommentId. The anchor on a reopened chip
2112+
// carries the id, so editing text hits updateComment (no duplicate row);
2113+
// a fresh click in comment mode falls through to addComment. Both return
2114+
// the row on success so the bubble can decide whether to close.
2115+
if (input.existingCommentId) {
2116+
return get().updateComment(input.existingCommentId, { text: input.text });
2117+
}
2118+
const payload: Parameters<CodesignState['addComment']>[0] = {
2119+
kind: input.kind,
2120+
selector: input.selector,
2121+
tag: input.tag,
2122+
outerHTML: input.outerHTML,
2123+
rect: input.rect,
2124+
text: input.text,
2125+
};
2126+
if (input.scope) payload.scope = input.scope;
2127+
if (input.parentOuterHTML) payload.parentOuterHTML = input.parentOuterHTML;
2128+
return get().addComment(payload);
2129+
},
2130+
20932131
async removeComment(id) {
20942132
if (!window.codesign) return;
20952133
try {

0 commit comments

Comments
 (0)