mirror of
https://github.com/stablyai/orca.git
synced 2026-10-07 00:02:29 +00:00
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
This commit is contained in:
@@ -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(
|
||||
<DiffCommentDraftCard lineNumber={10} onCancel={vi.fn()} onSubmit={onSubmit} />
|
||||
)
|
||||
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(
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user