diff --git a/src/renderer/src/components/diff-comments/DiffCommentDraftCard.tsx b/src/renderer/src/components/diff-comments/DiffCommentDraftCard.tsx index 18d5205acf9..667879da08d 100644 --- a/src/renderer/src/components/diff-comments/DiffCommentDraftCard.tsx +++ b/src/renderer/src/components/diff-comments/DiffCommentDraftCard.tsx @@ -44,13 +44,19 @@ export function DiffCommentDraftCard({ }: DiffCommentDraftCardProps): React.JSX.Element { const [body, setBody] = useState(initialBody) const bodyRef = useRef(body) - bodyRef.current = body const [submitting, setSubmitting] = useState(false) const mountedRef = useMountedRef() const textareaRef = useRef(null) const cardRef = useRef(null) const onContentResizeRef = useRef(onContentResize) - onContentResizeRef.current = onContentResize + + useEffect(() => { + bodyRef.current = body + }, [body]) + + useEffect(() => { + onContentResizeRef.current = onContentResize + }, [onContentResize]) const labelId = useId() const headerLabel = diff --git a/src/renderer/src/components/diff-comments/diff-comment-add-note-shortcut.ts b/src/renderer/src/components/diff-comments/diff-comment-add-note-shortcut.ts index 3dab9d053b8..a58ac12d725 100644 --- a/src/renderer/src/components/diff-comments/diff-comment-add-note-shortcut.ts +++ b/src/renderer/src/components/diff-comments/diff-comment-add-note-shortcut.ts @@ -55,8 +55,9 @@ export function installDiffCommentAddNoteShortcut({ onOpenComposer: (args: { lineNumber: number; startLine?: number; top: number }) => void }): () => void { return installEditorAddReviewNoteShortcut(editor.getContainerDomNode(), () => { - // Why: an open draft consumes the chord itself (DiffCommentPopover's guard); claiming it here - // too would remount the composer and drop what the user already typed. + // Why: an open draft card owns the chord (isComposerOpen reports it, and the card's own guard + // eats presses from its textarea); claiming it here too would re-open the card at the editor's + // selection and move what the user already typed. if (isComposerOpen()) { return true } diff --git a/src/renderer/src/components/diff-comments/diff-comment-draft-zone.test.tsx b/src/renderer/src/components/diff-comments/diff-comment-draft-zone.test.tsx new file mode 100644 index 00000000000..4a3fd47735b --- /dev/null +++ b/src/renderer/src/components/diff-comments/diff-comment-draft-zone.test.tsx @@ -0,0 +1,236 @@ +// @vitest-environment happy-dom +import { act, fireEvent, renderHook, within } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + useDiffCommentDraftZone, + type UseDiffCommentDraftZoneArgs +} from './diff-comment-draft-zone' +import { + createFakeDiffCommentEditor, + type FakeDiffCommentEditor +} from './diff-comment-editor-test-fixture' + +const DRAFT_LINE = 5 +const OTHER_LINE = 9 +const BODY = 'Needs revision' + +type CreateComment = NonNullable + +function renderDraftZone(fake: FakeDiffCommentEditor, onCreateComment: CreateComment) { + return renderHook( + ({ monacoModelIdentity }: { monacoModelIdentity: string }) => + useDiffCommentDraftZone({ editor: fake.editor, monacoModelIdentity, onCreateComment }), + { initialProps: { monacoModelIdentity: 'model-v1' } } + ) +} + +function openDraftAt(hook: ReturnType, lineNumber: number): void { + act(() => { + hook.result.current.onAddCommentClickRef.current({ lineNumber, top: 0 }) + }) +} + +// The single open draft card, attached to the editor node the way Monaco would mount its zone. +function draftCard(fake: FakeDiffCommentEditor): { + dom: HTMLElement + textarea: HTMLTextAreaElement +} { + const zones = [...fake.zones.values()] + expect(zones).toHaveLength(1) + const dom = zones[0].domNode + if (!dom.isConnected) { + fake.domNode.appendChild(dom) + } + const textarea = dom.querySelector('textarea') + if (!(textarea instanceof HTMLTextAreaElement)) { + throw new Error('draft card did not render a textarea') + } + return { dom, textarea } +} + +function typeDraft(fake: FakeDiffCommentEditor, body: string): void { + fireEvent.change(draftCard(fake).textarea, { target: { value: body } }) +} + +function submitDraft(fake: FakeDiffCommentEditor, body: string): void { + typeDraft(fake, body) + fireEvent.click(within(draftCard(fake).dom).getByRole('button', { name: 'Add note' })) +} + +function deferredCreateComment(): { + onCreateComment: CreateComment & ReturnType + settle: (result: boolean) => Promise +} { + let resolve: (result: boolean) => void = () => {} + const onCreateComment = vi.fn( + () => + new Promise((r) => { + resolve = r + }) + ) + return { + onCreateComment, + settle: (result) => + act(async () => { + resolve(result) + }) + } +} + +const frames = new Map() +let nextFrameId = 0 + +function pumpFrames(): void { + const pending = [...frames.values()] + frames.clear() + act(() => { + for (const callback of pending) { + callback(16) + } + }) +} + +beforeEach(() => { + frames.clear() + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback): number => { + nextFrameId += 1 + frames.set(nextFrameId, callback) + return nextFrameId + }) + vi.stubGlobal('cancelAnimationFrame', (id: number) => { + frames.delete(id) + }) +}) + +afterEach(() => { + vi.unstubAllGlobals() + document.body.replaceChildren() + vi.clearAllMocks() +}) + +describe('useDiffCommentDraftZone re-anchoring', () => { + it('keeps the carried body when a second model swap lands before the re-anchor frame', () => { + const fake = createFakeDiffCommentEditor() + const hook = renderDraftZone(fake, vi.fn().mockResolvedValue(true)) + openDraftAt(hook, DRAFT_LINE) + typeDraft(fake, BODY) + + hook.rerender({ monacoModelIdentity: 'model-v2' }) + hook.rerender({ monacoModelIdentity: 'model-v3' }) + expect(fake.zones.size).toBe(0) + pumpFrames() + + const { textarea } = draftCard(fake) + expect(textarea.value).toBe(BODY) + expect([...fake.zones.values()][0].afterLineNumber).toBe(DRAFT_LINE) + expect(hook.result.current.isDraftOpen()).toBe(true) + }) + + it('lets a click on another line win over a scheduled re-anchor', () => { + const fake = createFakeDiffCommentEditor() + const hook = renderDraftZone(fake, vi.fn().mockResolvedValue(true)) + openDraftAt(hook, DRAFT_LINE) + typeDraft(fake, BODY) + hook.rerender({ monacoModelIdentity: 'model-v2' }) + + openDraftAt(hook, OTHER_LINE) + expect(draftCard(fake).textarea.value).toBe(BODY) + pumpFrames() + + const zones = [...fake.zones.values()] + expect(zones).toHaveLength(1) + expect(zones[0].afterLineNumber).toBe(OTHER_LINE) + expect(draftCard(fake).textarea.value).toBe(BODY) + }) +}) + +describe('useDiffCommentDraftZone in-flight submit', () => { + it('does not carry a submitting draft across a model swap and clears it once the save lands', async () => { + const fake = createFakeDiffCommentEditor() + const { onCreateComment, settle } = deferredCreateComment() + const hook = renderDraftZone(fake, onCreateComment) + openDraftAt(hook, DRAFT_LINE) + submitDraft(fake, BODY) + expect(onCreateComment).toHaveBeenCalledTimes(1) + + hook.rerender({ monacoModelIdentity: 'model-v2' }) + pumpFrames() + // The in-flight save owns the text: no replacement card that could submit it a second time. + expect(fake.zones.size).toBe(0) + + await settle(true) + pumpFrames() + expect(fake.zones.size).toBe(0) + expect(hook.result.current.isDraftOpen()).toBe(false) + expect(onCreateComment).toHaveBeenCalledTimes(1) + }) + + it('brings the draft back for retry when its save fails after a model swap', async () => { + const fake = createFakeDiffCommentEditor() + const { onCreateComment, settle } = deferredCreateComment() + const hook = renderDraftZone(fake, onCreateComment) + openDraftAt(hook, DRAFT_LINE) + submitDraft(fake, BODY) + hook.rerender({ monacoModelIdentity: 'model-v2' }) + expect(fake.zones.size).toBe(0) + + await settle(false) + + const { textarea } = draftCard(fake) + expect(textarea.value).toBe(BODY) + expect([...fake.zones.values()][0].afterLineNumber).toBe(DRAFT_LINE) + expect(hook.result.current.isDraftOpen()).toBe(true) + }) + + it('does not carry a submitting body into a draft opened on another line', async () => { + const fake = createFakeDiffCommentEditor() + const { onCreateComment, settle } = deferredCreateComment() + const hook = renderDraftZone(fake, onCreateComment) + openDraftAt(hook, DRAFT_LINE) + submitDraft(fake, BODY) + + openDraftAt(hook, OTHER_LINE) + expect(draftCard(fake).textarea.value).toBe('') + + await settle(true) + const zones = [...fake.zones.values()] + expect(zones).toHaveLength(1) + expect(zones[0].afterLineNumber).toBe(OTHER_LINE) + expect(onCreateComment).toHaveBeenCalledTimes(1) + }) +}) + +describe('useDiffCommentDraftZone open state', () => { + it('reports the draft closed again after cancel so the chord is not deadened', () => { + const fake = createFakeDiffCommentEditor() + const hook = renderDraftZone(fake, vi.fn().mockResolvedValue(true)) + openDraftAt(hook, DRAFT_LINE) + expect(hook.result.current.isDraftOpen()).toBe(true) + + fireEvent.click(within(draftCard(fake).dom).getByRole('button', { name: 'Cancel' })) + + expect(hook.result.current.isDraftOpen()).toBe(false) + expect(fake.zones.size).toBe(0) + + openDraftAt(hook, OTHER_LINE) + expect(hook.result.current.isDraftOpen()).toBe(true) + expect([...fake.zones.values()][0].afterLineNumber).toBe(OTHER_LINE) + }) +}) + +describe('useDiffCommentDraftZone teardown', () => { + it('does not resurrect a card when a failed save settles after unmount', async () => { + const fake = createFakeDiffCommentEditor() + const { onCreateComment, settle } = deferredCreateComment() + const hook = renderDraftZone(fake, onCreateComment) + openDraftAt(hook, DRAFT_LINE) + submitDraft(fake, BODY) + + hook.unmount() + expect(fake.zones.size).toBe(0) + + await settle(false) + pumpFrames() + expect(fake.zones.size).toBe(0) + }) +}) diff --git a/src/renderer/src/components/diff-comments/diff-comment-draft-zone.ts b/src/renderer/src/components/diff-comments/diff-comment-draft-zone.ts index 8366282cefa..85128a32746 100644 --- a/src/renderer/src/components/diff-comments/diff-comment-draft-zone.ts +++ b/src/renderer/src/components/diff-comments/diff-comment-draft-zone.ts @@ -14,6 +14,8 @@ export type DiffCommentDraft = { startLine?: number } +type CarriedDraft = { draft: DiffCommentDraft; body: string } + export type UseDiffCommentDraftZoneArgs = { editor: monacoEditor.ICodeEditor | null monacoModelIdentity?: string @@ -30,6 +32,7 @@ export type UseDiffCommentDraftZoneArgs = { export type DiffCommentDraftZoneHandle = { disposeDraftZone: (focusEditor?: boolean) => void + isDraftOpen: () => boolean onAddCommentClickRef: React.RefObject< (args: { lineNumber: number; startLine?: number; top: number }) => void > @@ -45,17 +48,40 @@ export function useDiffCommentDraftZone({ onAddCommentClick }: UseDiffCommentDraftZoneArgs): DiffCommentDraftZoneHandle { const draftZoneRef = useRef(null) - const pendingDraftRef = useRef<{ draft: DiffCommentDraft; body: string } | null>(null) + // The only copy of a draft between its card being torn down and the replacement mounting. + const pendingDraftRef = useRef(null) const previousModelIdentityRef = useRef(monacoModelIdentity) const reanchorFrameRef = useRef(null) const onAddCommentClickRef = useRef< (args: { lineNumber: number; startLine?: number; top: number }) => void >(() => {}) + // A save can settle after unmount; without this its retry would mount a card nobody disposes. + const unmountedRef = useRef(false) const onCreateCommentRef = useRef(onCreateComment) const onLegacyAddCommentClickRef = useRef(onAddCommentClick) - onCreateCommentRef.current = onCreateComment - onLegacyAddCommentClickRef.current = onAddCommentClick + + useEffect(() => { + unmountedRef.current = false + return () => { + unmountedRef.current = true + } + }, []) + + useEffect(() => { + onCreateCommentRef.current = onCreateComment + onLegacyAddCommentClickRef.current = onAddCommentClick + }, [onAddCommentClick, onCreateComment]) + + const isDraftOpen = useCallback((): boolean => draftZoneRef.current !== null, []) + + const cancelReanchorFrame = useCallback((): void => { + if (reanchorFrameRef.current === null) { + return + } + cancelAnimationFrame(reanchorFrameRef.current) + reanchorFrameRef.current = null + }, []) const disposeDraftZone = useCallback((focusEditor = false): void => { const entry = draftZoneRef.current @@ -73,10 +99,24 @@ export function useDiffCommentDraftZone({ } }, []) + // Latest openDraft for callers that outlive a render: the re-anchor frame and a settling submit. + const openDraftRef = useRef<(draft: DiffCommentDraft, initialBody?: string) => boolean>( + () => false + ) + + // A save that failed after its card was torn down (model swap, editor refresh) brings the text + // back for a retry; an open card means the user already moved on, and the failure toast covers it. + const restoreFailedSubmit = useCallback((draft: DiffCommentDraft, body: string): void => { + if (unmountedRef.current || draftZoneRef.current || openDraftRef.current(draft, body)) { + return + } + pendingDraftRef.current = { draft, body } + }, []) + const openDraft = useCallback( - (draft: DiffCommentDraft, initialBody = ''): void => { + (draft: DiffCommentDraft, initialBody = ''): boolean => { if (!editor || !editor.getModel() || !onCreateCommentRef.current || !canOpenDraft) { - return + return false } disposeDraftZone() @@ -117,6 +157,7 @@ export function useDiffCommentDraftZone({ root, draft, body: initialBody, + submitting: false, disposeMouseDownStopper: () => { disposeDomMouseDownStopper() disposeMarginMouseDownStopper() @@ -144,63 +185,108 @@ export function useDiffCommentDraftZone({ if (!createComment) { return false } - const result = await createComment({ - lineNumber: draft.lineNumber, - startLine: draft.startLine, - body - }) - if (result !== false && draftZoneRef.current === entry) { - disposeDraftZone() + entry.submitting = true + let succeeded = false + try { + const result = await createComment({ + lineNumber: draft.lineNumber, + startLine: draft.startLine, + body + }) + succeeded = result !== false + return result + } finally { + entry.submitting = false + if (succeeded) { + if (draftZoneRef.current === entry) { + disposeDraftZone() + } + } else if (draftZoneRef.current !== entry) { + restoreFailedSubmit(draft, body) + } } - return result } }) }) + // Report what actually mounted: the re-anchor frame drops its only copy of the draft on true. + return draftZoneRef.current !== null }, - [canOpenDraft, disposeDraftZone, draftPlaceholder, draftSubmitLabel, editor] + [ + canOpenDraft, + disposeDraftZone, + draftPlaceholder, + draftSubmitLabel, + editor, + restoreFailedSubmit + ] ) - onAddCommentClickRef.current = (args) => { - if (!canOpenDraft) { + + useEffect(() => { + openDraftRef.current = openDraft + }, [openDraft]) + + // The stash is only cleared once a card actually mounts: the editor may still be mid-refresh + // when the frame fires, and a later editor/identity change re-schedules from the same stash. + const scheduleReanchor = useCallback((): void => { + if (reanchorFrameRef.current !== null) { return } - const current = draftZoneRef.current - const pending = pendingDraftRef.current - const carriedBody = current?.body || pending?.body || '' - if (onCreateCommentRef.current) { - openDraft({ lineNumber: args.lineNumber, startLine: args.startLine }, carriedBody) - return - } - onLegacyAddCommentClickRef.current?.(args) - } + reanchorFrameRef.current = requestAnimationFrame(() => { + reanchorFrameRef.current = null + const pending = pendingDraftRef.current + if (pending && openDraftRef.current(pending.draft, pending.body)) { + pendingDraftRef.current = null + } + }) + }, []) + + const openDraftFromArgs = useCallback( + (args: { lineNumber: number; startLine?: number; top: number }): void => { + if (!canOpenDraft) { + return + } + if (!onCreateCommentRef.current) { + onLegacyAddCommentClickRef.current?.(args) + return + } + const current = draftZoneRef.current + const pending = pendingDraftRef.current + // A submitting card's text is already on its way to the store; carrying it would post it twice. + const carriedBody = + (current && !current.submitting ? current.body : '') || pending?.body || '' + // The user picked a new anchor, so a scheduled re-anchor must not move the card back afterwards. + cancelReanchorFrame() + pendingDraftRef.current = null + if (!openDraft({ lineNumber: args.lineNumber, startLine: args.startLine }, carriedBody)) { + pendingDraftRef.current = pending + } + }, + [canOpenDraft, cancelReanchorFrame, openDraft] + ) + + useEffect(() => { + onAddCommentClickRef.current = openDraftFromArgs + }, [openDraftFromArgs]) useEffect(() => { if (previousModelIdentityRef.current === monacoModelIdentity) { return } previousModelIdentityRef.current = monacoModelIdentity - if (reanchorFrameRef.current !== null) { - cancelAnimationFrame(reanchorFrameRef.current) - reanchorFrameRef.current = null - pendingDraftRef.current = null - } + cancelReanchorFrame() const current = draftZoneRef.current - if (!current) { - return - } - pendingDraftRef.current = { draft: current.draft, body: current.body } - disposeDraftZone() - if (!editor || !editor.getModel() || !onCreateCommentRef.current || !canOpenDraft) { - return - } - reanchorFrameRef.current = requestAnimationFrame(() => { - reanchorFrameRef.current = null - const pending = pendingDraftRef.current - pendingDraftRef.current = null - if (pending) { - openDraft(pending.draft, pending.body) + if (current) { + // A save in flight owns its text: the settling submit disposes on success and re-opens on + // failure, so carrying it here would mount a second card that could submit the same note. + if (!current.submitting) { + pendingDraftRef.current = { draft: current.draft, body: current.body } } - }) - }, [canOpenDraft, disposeDraftZone, editor, monacoModelIdentity, openDraft]) + disposeDraftZone() + } + if (pendingDraftRef.current) { + scheduleReanchor() + } + }, [cancelReanchorFrame, disposeDraftZone, monacoModelIdentity, scheduleReanchor]) // A combined-diff model refresh can briefly clear the editor ref before the replacement mounts. // Re-anchor any carried draft when that replacement becomes available. @@ -208,38 +294,23 @@ export function useDiffCommentDraftZone({ if (!editor || !canOpenDraft || !pendingDraftRef.current) { return } - const pending = pendingDraftRef.current - pendingDraftRef.current = null - reanchorFrameRef.current = requestAnimationFrame(() => { - reanchorFrameRef.current = null - openDraft(pending.draft, pending.body) - }) - return () => { - if (reanchorFrameRef.current !== null) { - cancelAnimationFrame(reanchorFrameRef.current) - reanchorFrameRef.current = null - } - if (!draftZoneRef.current) { - pendingDraftRef.current = pending - } - } - }, [canOpenDraft, editor, openDraft]) + scheduleReanchor() + return cancelReanchorFrame + }, [cancelReanchorFrame, canOpenDraft, editor, scheduleReanchor]) useEffect(() => { if (!editor) { return } return () => { - if (reanchorFrameRef.current !== null) { - cancelAnimationFrame(reanchorFrameRef.current) - reanchorFrameRef.current = null - } + cancelReanchorFrame() disposeDraftZone(false) } - }, [disposeDraftZone, editor]) + }, [cancelReanchorFrame, disposeDraftZone, editor]) return { disposeDraftZone, + isDraftOpen, onAddCommentClickRef } } diff --git a/src/renderer/src/components/diff-comments/diff-comment-view-zone-entry.ts b/src/renderer/src/components/diff-comments/diff-comment-view-zone-entry.ts index 1cec4933db6..56e785f51b3 100644 --- a/src/renderer/src/components/diff-comments/diff-comment-view-zone-entry.ts +++ b/src/renderer/src/components/diff-comments/diff-comment-view-zone-entry.ts @@ -22,6 +22,8 @@ export type DraftZoneEntry = { root: Root draft: { lineNumber: number; startLine?: number } body: string + // A save is in flight: the body must not be carried to another card, or the note could post twice. + submitting: boolean disposeMouseDownStopper: () => void } diff --git a/src/renderer/src/components/diff-comments/useDiffCommentDecorator.range-selection.test.tsx b/src/renderer/src/components/diff-comments/useDiffCommentDecorator.range-selection.test.tsx index e09747b5932..2708e63a137 100644 --- a/src/renderer/src/components/diff-comments/useDiffCommentDecorator.range-selection.test.tsx +++ b/src/renderer/src/components/diff-comments/useDiffCommentDecorator.range-selection.test.tsx @@ -1,5 +1,5 @@ // @vitest-environment happy-dom -import { renderHook } from '@testing-library/react' +import { act, renderHook } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { Selection } from 'monaco-editor' import type * as DiffCommentZoneCardModule from './diff-comment-zone-card' @@ -37,6 +37,12 @@ type DecoratorProps = { commentableLineNumbers?: readonly number[] addNoteShortcutEnabled?: boolean onAddCommentClick?: (args: { lineNumber: number; startLine?: number; top: number }) => void + // Present on the inline-draft surfaces; absent for the legacy popover callers. + onCreateComment?: (args: { + lineNumber: number + startLine?: number + body: string + }) => Promise } function renderDecorator(fake: FakeDiffCommentEditor, initialProps: DecoratorProps = {}) { @@ -51,6 +57,7 @@ function renderDecorator(fake: FakeDiffCommentEditor, initialProps: DecoratorPro pendingCommentTarget: props.pendingCommentTarget ?? null, addNoteShortcutEnabled: props.addNoteShortcutEnabled ?? false, onAddCommentClick: props.onAddCommentClick ?? vi.fn(), + onCreateComment: props.onCreateComment, onDeleteComment: vi.fn() }), { initialProps } @@ -102,19 +109,19 @@ function firePointerEvent( } // Mod+Shift+A on the platform this test's user agent reports. -function pressAddReviewNoteChord(node: HTMLElement): void { +function pressAddReviewNoteChord(node: HTMLElement): KeyboardEvent { const isMac = navigator.userAgent.includes('Mac') - node.dispatchEvent( - new KeyboardEvent('keydown', { - key: 'A', - code: 'KeyA', - shiftKey: true, - metaKey: isMac, - ctrlKey: !isMac, - bubbles: true, - cancelable: true - }) - ) + const event = new KeyboardEvent('keydown', { + key: 'A', + code: 'KeyA', + shiftKey: true, + metaKey: isMac, + ctrlKey: !isMac, + bubbles: true, + cancelable: true + }) + node.dispatchEvent(event) + return event } const frameCallbacks: FrameRequestCallback[] = [] @@ -377,4 +384,29 @@ describe('useDiffCommentDecorator add-note chord', () => { expect(onAddCommentClick).toHaveBeenCalledTimes(1) }) + + it('leaves the chord to an open inline draft card instead of re-anchoring it', () => { + const fake = createFakeDiffCommentEditor() + vi.spyOn(fake.editor, 'getSelection').mockReturnValue(selectionOf(9, 1, 14, 8)) + renderDecorator(fake, { + addNoteShortcutEnabled: true, + onCreateComment: vi.fn().mockResolvedValue(true) + }) + + act(() => { + pressAddReviewNoteChord(fake.domNode) + }) + const [zone] = [...fake.zones.values()] + expect(zone?.afterLineNumber).toBe(14) + + // The selection moved on, but the open card must keep its anchor and its text. + vi.spyOn(fake.editor, 'getSelection').mockReturnValue(selectionOf(30, 1, 32, 4)) + const secondChord: { event?: KeyboardEvent } = {} + act(() => { + secondChord.event = pressAddReviewNoteChord(fake.domNode) + }) + + expect(secondChord.event?.defaultPrevented).toBe(true) + expect([...fake.zones.values()]).toEqual([zone]) + }) }) diff --git a/src/renderer/src/components/diff-comments/useDiffCommentDecorator.tsx b/src/renderer/src/components/diff-comments/useDiffCommentDecorator.tsx index 60a505f5dbd..e94bb6fc59e 100644 --- a/src/renderer/src/components/diff-comments/useDiffCommentDecorator.tsx +++ b/src/renderer/src/components/diff-comments/useDiffCommentDecorator.tsx @@ -102,7 +102,7 @@ export function useDiffCommentDecorator({ onUpdateCommentRef.current = onUpdateComment onPendingScrollConsumedRef.current = onPendingScrollConsumed - const { onAddCommentClickRef } = useDiffCommentDraftZone({ + const { onAddCommentClickRef, isDraftOpen } = useDiffCommentDraftZone({ editor, monacoModelIdentity, onCreateComment, @@ -172,6 +172,7 @@ export function useDiffCommentDecorator({ setPendingCommentRange(range) }, [pendingLineNumber, pendingStartLine, setPendingCommentRange]) + const hasDraftComposer = onCreateComment !== undefined useEffect(() => { if (!editor || !addNoteShortcutEnabled) { return @@ -179,11 +180,24 @@ export function useDiffCommentDecorator({ return installDiffCommentAddNoteShortcut({ editor, commentableLineSet, - // A live gutter drag owns the band; opening from the stale editor selection during a drag - // would remount the composer before the gesture completes. + // An open draft card owns the chord: claiming it here would re-open the card at the editor's + // selection and move the user's text. The card mounts synchronously, so a second chord in the + // same event turn already sees it. A live gutter drag owns the band the same way; opening + // from the stale editor selection during a drag would remount the card mid-gesture. isComposerOpen: () => - pendingCommentRangeRef.current !== null || overlayRef.current?.isDragging() === true, + isDraftOpen() || + pendingCommentRangeRef.current !== null || + overlayRef.current?.isDragging() === true, onOpenComposer: (args) => { + // Legacy popover callers commit composer state through React, so claim now or a same-turn + // repeat opens a second one; their pendingCommentTarget releases the claim on close. The + // inline draft card never passes one, so a claim here would never be released. + if (!hasDraftComposer) { + setPendingCommentRange({ + startLine: args.startLine ?? args.lineNumber, + endLine: args.lineNumber + }) + } onAddCommentClickRef.current(args) } }) @@ -191,7 +205,10 @@ export function useDiffCommentDecorator({ addNoteShortcutEnabled, commentableLineSet, editor, + hasDraftComposer, + isDraftOpen, monacoModelIdentity, + onAddCommentClickRef, setPendingCommentRange ]) diff --git a/src/renderer/src/components/editor/DiffViewer.tsx b/src/renderer/src/components/editor/DiffViewer.tsx index f84c1ba42d8..e48ff20b325 100644 --- a/src/renderer/src/components/editor/DiffViewer.tsx +++ b/src/renderer/src/components/editor/DiffViewer.tsx @@ -8,6 +8,7 @@ import { useContextualCopySetup } from './useContextualCopySetup' import { selectWorktreeDiffComments } from '@/store/worktree-diff-comments-selector' import { useDiffCommentDecorator } from '../diff-comments/useDiffCommentDecorator' import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' import { applyDiffEditorLineNumberOptions } from './diff-editor-line-number-options' import type { DiffComment } from '../../../../shared/diff-comment-types' import { isDiffComment } from '@/lib/diff-comment-compat' @@ -115,7 +116,9 @@ export default function DiffViewer({ side: 'modified' }) if (!result) { - toast.error('Failed to save comment') + toast.error( + translate('auto.components.editor.diffCommentSaveFailed', 'Failed to save comment') + ) } return Boolean(result) }, diff --git a/src/renderer/src/components/editor/diff-section-comment-submit.ts b/src/renderer/src/components/editor/diff-section-comment-submit.ts index db266fba0fb..0d011a9dd6f 100644 --- a/src/renderer/src/components/editor/diff-section-comment-submit.ts +++ b/src/renderer/src/components/editor/diff-section-comment-submit.ts @@ -1,5 +1,6 @@ import type { DiffSection } from './diff-section-types' import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' type DiffSectionCommentTarget = { lineNumber: number @@ -60,7 +61,7 @@ export async function submitDiffSectionComment({ side: 'modified' }) if (!result) { - toast.error('Failed to save comment') + toast.error(translate('auto.components.editor.diffCommentSaveFailed', 'Failed to save comment')) } return Boolean(result) } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 6a8d0b8a030..d39b361b9d0 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -14845,6 +14845,7 @@ "f1aa04b5cf": "This file", "8b87612461": "All unsent notes" }, + "diffCommentSaveFailed": "Failed to save comment", "DiffSectionBody": { "35d6afb5be": "Binary file changed", "cef4cf0ff5": "Retry",