From 3e647717f6cf055ac6a2cd33751afe2535aa7b2b Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:53:55 -0700 Subject: [PATCH] fix(diff-comments): show save error and restore focus intelligently - Toast error when draft submission fails, so users see why it didn't save - Return focus to editor only if the card still holds it when save completes, preventing focus theft on slow saves --- .../DiffCommentDraftCard.test.tsx | 21 ++++++++++++ .../diff-comments/DiffCommentDraftCard.tsx | 4 +++ .../diff-comment-draft-zone.test.tsx | 32 +++++++++++++++++++ .../diff-comments/diff-comment-draft-zone.ts | 4 ++- .../diff-comment-editor-test-fixture.ts | 7 ++++ 5 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/components/diff-comments/DiffCommentDraftCard.test.tsx b/src/renderer/src/components/diff-comments/DiffCommentDraftCard.test.tsx index 47bd6b52df3..42ce03d7aa6 100644 --- a/src/renderer/src/components/diff-comments/DiffCommentDraftCard.test.tsx +++ b/src/renderer/src/components/diff-comments/DiffCommentDraftCard.test.tsx @@ -1,8 +1,11 @@ // @vitest-environment happy-dom import { act, cleanup, fireEvent, render } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { toast } from 'sonner' import { DiffCommentDraftCard } from './DiffCommentDraftCard' +vi.mock('sonner', () => ({ toast: { error: vi.fn(), success: vi.fn(), info: vi.fn() } })) + describe('DiffCommentDraftCard', () => { let scrollHeight = 60 @@ -138,6 +141,24 @@ describe('DiffCommentDraftCard', () => { await act(async () => resolveSubmit(true)) }) + it('reports a rejected save to the user and re-enables the card', async () => { + const error = new Error('network down') + const onSubmit = vi.fn().mockRejectedValue(error) + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const view = render( + + ) + fireEvent.change(view.getByRole('textbox'), { target: { value: 'Rejected note' } }) + + await act(async () => { + fireEvent.click(view.getByRole('button', { name: 'Add note' })) + }) + + expect(toast.error).toHaveBeenCalledWith('Failed to save comment') + expect(consoleError).toHaveBeenCalled() + expect(view.getByRole('button', { name: 'Add note' }).hasAttribute('disabled')).toBe(false) + }) + it('seeds and reports draft text so a re-anchored card can carry it', () => { const onBodyChange = vi.fn() const view = render( diff --git a/src/renderer/src/components/diff-comments/DiffCommentDraftCard.tsx b/src/renderer/src/components/diff-comments/DiffCommentDraftCard.tsx index 667879da08d..c3085b92d8b 100644 --- a/src/renderer/src/components/diff-comments/DiffCommentDraftCard.tsx +++ b/src/renderer/src/components/diff-comments/DiffCommentDraftCard.tsx @@ -157,6 +157,10 @@ export function DiffCommentDraftCard({ } } catch (err) { console.error('Failed to submit diff comment draft:', err) + // A rejected save never reaches the caller's own failure toast, so report it here. + toast.error( + translate('auto.components.editor.diffCommentSaveFailed', 'Failed to save comment') + ) if (mountedRef.current) { setSubmitting(false) } 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 index 4a3fd47735b..db894e7ef0a 100644 --- 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 @@ -200,6 +200,38 @@ describe('useDiffCommentDraftZone in-flight submit', () => { }) }) +describe('useDiffCommentDraftZone focus handoff', () => { + it('returns focus to the editor when a save succeeds while the card holds it', async () => { + const fake = createFakeDiffCommentEditor() + const { onCreateComment, settle } = deferredCreateComment() + const hook = renderDraftZone(fake, onCreateComment) + openDraftAt(hook, DRAFT_LINE) + draftCard(fake).textarea.focus() + submitDraft(fake, BODY) + + await settle(true) + + expect(fake.zones.size).toBe(0) + expect(fake.focusCount()).toBe(1) + }) + + it('leaves focus alone when the save settles after the user clicked away', async () => { + const fake = createFakeDiffCommentEditor() + const { onCreateComment, settle } = deferredCreateComment() + const hook = renderDraftZone(fake, onCreateComment) + openDraftAt(hook, DRAFT_LINE) + submitDraft(fake, BODY) + + const elsewhere = document.createElement('input') + document.body.appendChild(elsewhere) + elsewhere.focus() + await settle(true) + + expect(fake.zones.size).toBe(0) + expect(fake.focusCount()).toBe(0) + }) +}) + describe('useDiffCommentDraftZone open state', () => { it('reports the draft closed again after cancel so the chord is not deadened', () => { const fake = createFakeDiffCommentEditor() 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 85128a32746..eacb75d590c 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 @@ -199,7 +199,9 @@ export function useDiffCommentDraftZone({ entry.submitting = false if (succeeded) { if (draftZoneRef.current === entry) { - disposeDraftZone() + // Hand focus back to Monaco only if the card still holds it: a slow save can + // settle after the user has clicked away, and stealing focus back is worse. + disposeDraftZone(entry.domNode.contains(document.activeElement)) } } else if (draftZoneRef.current !== entry) { restoreFailedSubmit(draft, body) diff --git a/src/renderer/src/components/diff-comments/diff-comment-editor-test-fixture.ts b/src/renderer/src/components/diff-comments/diff-comment-editor-test-fixture.ts index 253bb785074..3e13e6784a5 100644 --- a/src/renderer/src/components/diff-comments/diff-comment-editor-test-fixture.ts +++ b/src/renderer/src/components/diff-comments/diff-comment-editor-test-fixture.ts @@ -22,6 +22,8 @@ export type FakeDiffCommentEditor = { decorations: () => readonly FakeDecoration[] decorationWrites: () => number scrollTop: () => number + /** How many times the editor was asked to take focus back. */ + focusCount: () => number emitMouseMove: (lineNumber: number) => void emitDispose: () => void /** Viewport-relative Y for a line, as the fake hit-test reads it. */ @@ -69,6 +71,7 @@ export function createFakeDiffCommentEditor( let scrollTop = 0 let decorations: FakeDecoration[] = [] let decorationWrites = 0 + let focusCount = 0 const mouseMoveListeners: ((e: { target: { position: { lineNumber: number } } }) => void)[] = [] const disposeListeners: (() => void)[] = [] const noopDisposable: IDisposable = { dispose: () => {} } @@ -100,6 +103,9 @@ export function createFakeDiffCommentEditor( getScrollHeight: () => lineCount * FAKE_LINE_HEIGHT_PX, getLayoutInfo: () => ({ height: FAKE_EDITOR_HEIGHT_PX, contentLeft: 60 }), getSelection: () => null, + focus: () => { + focusCount += 1 + }, deltaDecorations: () => [], createDecorationsCollection: () => ({ set: (next: MonacoEditor.IModelDeltaDecoration[]) => { @@ -161,6 +167,7 @@ export function createFakeDiffCommentEditor( decorations: () => decorations, decorationWrites: () => decorationWrites, scrollTop: () => scrollTop, + focusCount: () => focusCount, clientYForLine, setLineCount: (next) => { lineCount = next