From 4371db10cb1c5d5c5eee3c16956c44d517b5d0c2 Mon Sep 17 00:00:00 2001 From: Cody Partington Date: Wed, 13 May 2026 22:40:24 +1000 Subject: [PATCH 1/3] fix(review): persist file comment drafts across close/reopen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File comment popover lost in-progress text when dismissed via click-outside / Escape / X — local state died with the unmount. Line-comment toolbar already does this via a module-level draft map; mirror the pattern in `CommentPopover` behind an opt-in `draftKey` prop and wire both file-comment call sites (`AllFilesDiffView`, `DiffViewer`). --- .../components/AllFilesDiffView.tsx | 1 + .../review-editor/components/DiffViewer.tsx | 1 + packages/ui/components/CommentPopover.tsx | 28 +++++++++++++++++-- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/review-editor/components/AllFilesDiffView.tsx b/packages/review-editor/components/AllFilesDiffView.tsx index 6b9cd94ee..9ea673cf1 100644 --- a/packages/review-editor/components/AllFilesDiffView.tsx +++ b/packages/review-editor/components/AllFilesDiffView.tsx @@ -501,6 +501,7 @@ export const AllFilesDiffView: React.FC = ({ anchorEl={fileCommentAnchor.el} contextText={fileCommentAnchor.filePath.split('/').pop() || fileCommentAnchor.filePath} isGlobal={false} + draftKey={`file:${fileCommentAnchor.filePath}`} onSubmit={(text) => { onAddFileComment(fileCommentAnchor.filePath, text); setFileCommentAnchor(null); diff --git a/packages/review-editor/components/DiffViewer.tsx b/packages/review-editor/components/DiffViewer.tsx index 912737c10..2136513e1 100644 --- a/packages/review-editor/components/DiffViewer.tsx +++ b/packages/review-editor/components/DiffViewer.tsx @@ -621,6 +621,7 @@ export const DiffViewer: React.FC = ({ anchorEl={fileCommentAnchor} contextText={filePath.split('/').pop() || filePath} isGlobal={false} + draftKey={`file:${filePath}`} onSubmit={(text) => { onAddFileComment(text); setFileCommentAnchor(null); diff --git a/packages/ui/components/CommentPopover.tsx b/packages/ui/components/CommentPopover.tsx index 4baa11750..d56e50f6b 100644 --- a/packages/ui/components/CommentPopover.tsx +++ b/packages/ui/components/CommentPopover.tsx @@ -20,11 +20,28 @@ interface CommentPopoverProps { onSubmit: (text: string, images?: ImageAttachment[]) => void; /** Called when popover is closed/cancelled */ onClose: () => void; + /** Opt-in: persist text + images across close/reopen, keyed by this string. Cleared on submit. */ + draftKey?: string; } const MAX_POPOVER_WIDTH = 384; const GAP = 8; +// Module-level draft store: survives popover unmount so reopening the same key restores in-progress text. +const draftStore = new Map(); + +/** Mirrors the latest text + images into `draftStore[draftKey]` so they outlive popover unmount. No-op without a key. */ +function useCommentDraftSync(draftKey: string | undefined, text: string, images: ImageAttachment[]) { + useEffect(() => { + if (!draftKey) return; + if (text.trim() || images.length > 0) { + draftStore.set(draftKey, { text, images }); + } else { + draftStore.delete(draftKey); + } + }, [draftKey, text, images]); +} + function computePosition(anchorRect: DOMRect): { top: number; left: number; flipAbove: boolean; width: number } { const spaceBelow = window.innerHeight - anchorRect.bottom; const flipAbove = spaceBelow < 280; @@ -48,15 +65,19 @@ export const CommentPopover: React.FC = ({ initialText = '', onSubmit, onClose, + draftKey, }) => { const [mode, setMode] = useState<'popover' | 'dialog'>('popover'); - const [text, setText] = useState(initialText); - const [images, setImages] = useState([]); + const initialDraft = draftKey ? draftStore.get(draftKey) : undefined; + const [text, setText] = useState(initialDraft?.text ?? initialText); + const [images, setImages] = useState(initialDraft?.images ?? []); const [position, setPosition] = useState<{ top: number; left: number; flipAbove: boolean; width: number } | null>(null); const textareaRef = useRef(null); const popoverRef = useRef(null); const { dragPosition, dragHandleProps, wasDragged, reset: resetDrag } = useDraggable(popoverRef); + useCommentDraftSync(draftKey, text, images); + // Reset drag when anchor changes (new annotation) or mode switches useEffect(() => { resetDrag(); }, [anchorEl, anchorRect, resetDrag]); useEffect(() => { if (mode === 'popover') resetDrag(); }, [mode, resetDrag]); @@ -111,9 +132,10 @@ export const CommentPopover: React.FC = ({ const handleSubmit = useCallback(() => { if (text.trim() || images.length > 0) { + if (draftKey) draftStore.delete(draftKey); onSubmit(text, images.length > 0 ? images : undefined); } - }, [text, images, onSubmit]); + }, [text, images, onSubmit, draftKey]); const handleKeyDown = (e: React.KeyboardEvent) => { if (e.key === 'Escape') { From cdd9bf34d765d62f816636e5568160d635615460 Mon Sep 17 00:00:00 2001 From: Cody Partington Date: Wed, 13 May 2026 23:05:11 +1000 Subject: [PATCH 2/3] fix(review): scope file-comment draft key by PR identity File-path-only key leaked drafts across in-place PR switches. --- packages/review-editor/components/AllFilesDiffView.tsx | 2 +- packages/review-editor/components/DiffViewer.tsx | 7 ++++++- packages/review-editor/dock/panels/ReviewDiffPanel.tsx | 2 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/review-editor/components/AllFilesDiffView.tsx b/packages/review-editor/components/AllFilesDiffView.tsx index 1c92f9ce3..0b549eddb 100644 --- a/packages/review-editor/components/AllFilesDiffView.tsx +++ b/packages/review-editor/components/AllFilesDiffView.tsx @@ -518,7 +518,7 @@ export const AllFilesDiffView: React.FC = ({ anchorEl={fileCommentAnchor.el} contextText={fileCommentAnchor.filePath.split('/').pop() || fileCommentAnchor.filePath} isGlobal={false} - draftKey={`file:${fileCommentAnchor.filePath}`} + draftKey={`file:${prUrl ?? ''}:${prDiffScope ?? ''}:${fileCommentAnchor.filePath}`} onSubmit={(text) => { onAddFileComment(fileCommentAnchor.filePath, text); setFileCommentAnchor(null); diff --git a/packages/review-editor/components/DiffViewer.tsx b/packages/review-editor/components/DiffViewer.tsx index 2136513e1..fd99a2b4c 100644 --- a/packages/review-editor/components/DiffViewer.tsx +++ b/packages/review-editor/components/DiffViewer.tsx @@ -120,6 +120,9 @@ interface DiffViewerProps { oldPath?: string; /** Base branch override used for file-content lookups (branch / merge-base modes only). */ reviewBase?: string; + /** Current PR url + diff scope — used to namespace file-comment drafts so they don't leak across in-place PR switches. */ + prUrl?: string; + prDiffScope?: string; isFocused?: boolean; diffStyle: 'split' | 'unified'; diffOverflow?: 'scroll' | 'wrap'; @@ -165,6 +168,8 @@ export const DiffViewer: React.FC = ({ filePath, oldPath, reviewBase, + prUrl, + prDiffScope, isFocused = false, diffStyle, diffOverflow, @@ -621,7 +626,7 @@ export const DiffViewer: React.FC = ({ anchorEl={fileCommentAnchor} contextText={filePath.split('/').pop() || filePath} isGlobal={false} - draftKey={`file:${filePath}`} + draftKey={`file:${prUrl ?? ''}:${prDiffScope ?? ''}:${filePath}`} onSubmit={(text) => { onAddFileComment(text); setFileCommentAnchor(null); diff --git a/packages/review-editor/dock/panels/ReviewDiffPanel.tsx b/packages/review-editor/dock/panels/ReviewDiffPanel.tsx index 7de62e3ea..5e08df4fb 100644 --- a/packages/review-editor/dock/panels/ReviewDiffPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewDiffPanel.tsx @@ -71,6 +71,8 @@ export const ReviewDiffPanel: React.FC = (props) => { filePath={file.path} oldPath={file.oldPath} reviewBase={state.reviewBase} + prUrl={state.prMetadata?.url} + prDiffScope={state.prDiffScope} isFocused={isFocusedFile} diffStyle={state.diffStyle} diffOverflow={state.diffOverflow} From aae06bc1b28d821a91980c2bbe471db3d8f4628e Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 13 May 2026 06:49:33 -0700 Subject: [PATCH 3/3] fix(review): remount all-files comment popover on context change The all-files panel shares one CommentPopover instance across every file. Switching files (or PR/scope) batches close+open into a single state transition, so React reuses the instance with stale text in state and the draft-sync effect writes it under the new draftKey. Key the popover by the same expression as draftKey so any change in file, PR, or scope forces a fresh mount that reads its own draft. Also clear the anchor when the file list reloads so a popover anchored to a removed DOM node doesn't survive a diff swap. --- packages/review-editor/components/AllFilesDiffView.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/review-editor/components/AllFilesDiffView.tsx b/packages/review-editor/components/AllFilesDiffView.tsx index 0b549eddb..3effd0011 100644 --- a/packages/review-editor/components/AllFilesDiffView.tsx +++ b/packages/review-editor/components/AllFilesDiffView.tsx @@ -101,6 +101,7 @@ export const AllFilesDiffView: React.FC = ({ setActiveFilePath(null); setCollapsedFiles(new Set()); collapseHistory.current = []; + setFileCommentAnchor(null); }, [files]); const scrollRef = useRef(null); @@ -515,6 +516,7 @@ export const AllFilesDiffView: React.FC = ({ {fileCommentAnchor && onAddFileComment && (