diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx index 60de5f5ca5..21e276bb04 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx @@ -247,12 +247,19 @@ describe('SessionActionsMenu', () => { expect(await screen.queryByRole('dialog')).toBeNull() expect(onDelete).not.toHaveBeenCalled() - // Re-open and confirm with Enter: the delete call fires. + // Re-open and confirm with Enter at wherever focus actually is. Firing on + // the dialog node would pass even when the menu leaves focus on the row + // trigger — where Enter re-activates the row instead of confirming. fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' }) fireEvent.click(trigger) fireEvent.click(await screen.findByRole('menuitem', { name: /delete/i })) - fireEvent.keyDown(await screen.findByRole('dialog'), { key: 'Enter' }) + + const reopened = await screen.findByRole('dialog') + // eslint-disable-next-line no-restricted-globals -- asserting real focus requires the live document + await waitFor(() => expect(reopened.contains(document.activeElement)).toBe(true)) + // eslint-disable-next-line no-restricted-globals -- asserting real focus requires the live document + fireEvent.keyDown(document.activeElement!, { key: 'Enter' }) expect(await screen.findByText('Session deleted')).toBeTruthy() expect(onDelete).toHaveBeenCalledTimes(1) diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx index 2550800e72..70962073db 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx @@ -22,14 +22,7 @@ import { Codicon } from '@/components/ui/codicon' import { ColorSwatches } from '@/components/ui/color-swatches' import { ConfirmDialog } from '@/components/ui/confirm-dialog' import { CopyButton } from '@/components/ui/copy-button' -import { - Dialog, - DialogContent, - DialogFooter, - DialogHeader, - DialogTitle, - preventCloseButtonAutoFocus -} from '@/components/ui/dialog' +import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from '@/components/ui/dialog' import { Input } from '@/components/ui/input' import { renameSession } from '@/hermes' import { useI18n } from '@/i18n' @@ -580,7 +573,6 @@ function DeleteSessionDialog({ open, onOpenChange, onConfirm, sessionTitle }: De doneLabel={r.deleted} onClose={() => onOpenChange(false)} onConfirm={onConfirm} - onOpenAutoFocus={preventCloseButtonAutoFocus} open={open} title={r.deleteTitle} /> diff --git a/apps/desktop/src/components/ui/confirm-dialog.tsx b/apps/desktop/src/components/ui/confirm-dialog.tsx index d4d5fc327d..3d60069c4b 100644 --- a/apps/desktop/src/components/ui/confirm-dialog.tsx +++ b/apps/desktop/src/components/ui/confirm-dialog.tsx @@ -1,5 +1,5 @@ import type { ReactNode } from 'react' -import { useEffect, useState } from 'react' +import { useEffect, useRef, useState } from 'react' import { ActionStatus } from '@/components/ui/action-status' import { Button } from '@/components/ui/button' @@ -28,15 +28,12 @@ interface ConfirmDialogProps { destructive?: boolean /** Close as soon as onConfirm resolves — for optimistic actions that finish in the background. */ dismissOnConfirm?: boolean - /** Focus control for dialogs with no input. Pass `preventCloseButtonAutoFocus` - * so opening doesn't land focus on the close/cancel button (which would pop - * its tooltip with no pointer near it). */ - onOpenAutoFocus?: (event: Event) => void } -// Shared confirmation dialog: Enter confirms (from anywhere in the dialog), -// Esc/Cancel/backdrop dismiss. Owns the pending → done → close beat and inline -// error, so callers pass only an async onConfirm that does the work. +// Shared confirmation dialog: opens focused on Confirm, Enter confirms (from +// anywhere in the dialog), Esc/Cancel/backdrop dismiss. Owns the pending → done +// → close beat and inline error, so callers pass only an async onConfirm that +// does the work. export function ConfirmDialog({ open, onClose, @@ -48,10 +45,10 @@ export function ConfirmDialog({ doneLabel, cancelLabel, destructive = false, - dismissOnConfirm = false, - onOpenAutoFocus + dismissOnConfirm = false }: ConfirmDialogProps) { const { t } = useI18n() + const confirmRef = useRef(null) const [status, setStatus] = useState<'done' | 'idle' | 'saving'>('idle') const [error, setError] = useState(null) const busy = status === 'saving' || status === 'done' @@ -109,7 +106,14 @@ export function ConfirmDialog({ void run() } }} - onOpenAutoFocus={onOpenAutoFocus} + onOpenAutoFocus={event => { + // Focus must land inside the dialog or the handler above never sees + // the key: it stays on whatever opened the dialog (a menu item, a + // sidebar row) and Enter re-triggers that instead. Radix's default + // would take the X — confirm is the button Enter maps to. + event.preventDefault() + confirmRef.current?.focus() + }} > {title} @@ -127,7 +131,12 @@ export function ConfirmDialog({ -