From d69d16a3fa0d8ae037f7f11a5b4d9b59235b3b83 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:39:50 -0700 Subject: [PATCH] fix(agent-dashboard): restore focus to the opener when the drawer closes (#34398) The store-driven Agents drawer has no Radix trigger, so dismissing it with Escape or the close button dropped focus to document.body. Capture the focused element on each open transition and return focus to it on close, unless focus already moved elsewhere or the close hands focus off (outside pointer dismissal, agent reveal, switching to the pop-out). Co-authored-by: kambarakun --- .../AgentDashboardDrawer.focus.test.tsx | 192 ++++++++++++++++++ .../dashboard/AgentDashboardDrawer.tsx | 60 +++++- 2 files changed, 242 insertions(+), 10 deletions(-) create mode 100644 src/renderer/src/components/dashboard/AgentDashboardDrawer.focus.test.tsx diff --git a/src/renderer/src/components/dashboard/AgentDashboardDrawer.focus.test.tsx b/src/renderer/src/components/dashboard/AgentDashboardDrawer.focus.test.tsx new file mode 100644 index 00000000000..b7bdea79cd6 --- /dev/null +++ b/src/renderer/src/components/dashboard/AgentDashboardDrawer.focus.test.tsx @@ -0,0 +1,192 @@ +// @vitest-environment happy-dom + +import '@testing-library/jest-dom/vitest' +import type { ComponentProps } from 'react' +import { act, cleanup, render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import type { AgentKanbanBoard } from '../dashboard-popout/AgentKanbanBoard' + +const mocks = vi.hoisted(() => ({ + activateAndRevealWorkspace: vi.fn(() => true), + activateTabAndFocusPane: vi.fn(), + openPopout: vi.fn().mockResolvedValue(undefined) +})) + +vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: mocks.activateAndRevealWorkspace +})) +vi.mock('@/lib/activate-tab-and-focus-pane', () => ({ + activateTabAndFocusPane: mocks.activateTabAndFocusPane +})) +vi.mock('./useLiveDashboardSnapshot', () => ({ + useLiveDashboardSnapshot: () => ({ generatedAt: 1, cards: [] }) +})) +vi.mock('../dashboard-popout/AgentKanbanBoard', () => ({ + AgentKanbanBoard: ({ + onClose, + onRevealAgent, + headerActions + }: ComponentProps) => ( + <> + + + + {headerActions} + + ) +})) +vi.mock('./AgentDashboardSettingsMenu', () => ({ + AgentDashboardSettingsMenu: ({ onSwitchToPopout }: { onSwitchToPopout: () => void }) => ( + + ) +})) + +import { AgentDashboardDrawer } from './AgentDashboardDrawer' + +const initialState = useAppStore.getInitialState() + +beforeEach(() => { + vi.clearAllMocks() + mocks.activateAndRevealWorkspace.mockReturnValue(true) + useAppStore.setState({ agentDashboardDrawerOpen: false, sidebarOpen: true, sidebarWidth: 320 }) + Object.defineProperty(window, 'api', { + configurable: true, + value: { dashboard: { openPopout: mocks.openPopout } } + }) +}) + +afterEach(async () => { + cleanup() + // Radix schedules close autofocus after unmount, including cleanup. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + useAppStore.setState(initialState, true) + vi.restoreAllMocks() +}) + +function fixture(): ReturnType { + const open = (): void => useAppStore.getState().setAgentDashboardDrawerOpen(true) + return render( + <> + + + + + + ) +} + +async function openFrom(name = 'Opener A'): Promise { + const opener = screen.getByRole('button', { name }) + opener.focus() + await userEvent.keyboard('{Enter}') + expect(document.activeElement).toBe(opener) + screen.getByRole('textbox', { name: 'Search agents' }).focus() + return opener +} + +async function closed(): Promise { + await waitFor(() => expect(screen.queryByRole('textbox', { name: 'Search agents' })).toBeNull()) + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) +} + +describe('AgentDashboardDrawer focus return', () => { + it.each(['Escape', 'Close dashboard'])('returns to the opener after %s', async (dismiss) => { + fixture() + const opener = await openFrom() + await (dismiss === 'Escape' + ? userEvent.keyboard('{Escape}') + : userEvent.click(screen.getByRole('button', { name: dismiss }))) + await closed() + expect(document.activeElement).toBe(opener) + }) + + it('keeps outside focus when the store closes the drawer', async () => { + fixture() + await openFrom() + const outside = screen.getByRole('textbox', { name: 'Outside input' }) + outside.focus() + act(() => useAppStore.getState().setAgentDashboardDrawerOpen(false)) + await closed() + expect(document.activeElement).toBe(outside) + }) + + it('lets the real outside pointerdown dismissal hand focus to its target', async () => { + fixture() + const opener = await openFrom() + const sheet = screen.getByRole('dialog', { name: 'Agents' }) + vi.spyOn(sheet, 'getBoundingClientRect').mockReturnValue(new DOMRect(320, 40, 600, 600)) + const outside = screen.getByRole('textbox', { name: 'Outside input' }) + const returned: Event[] = [] + opener.addEventListener('focus', (event) => returned.push(event)) + await userEvent.pointer({ + target: outside, + keys: '[MouseLeft]', + coords: { clientX: 1000, clientY: 100 } + }) + await closed() + expect(document.activeElement).toBe(outside) + expect(returned).toHaveLength(0) + await userEvent.keyboard('focus-test') + expect(outside).toHaveValue('focus-test') + }) + + it('restores after store dismissal and captures the next opener', async () => { + fixture() + const first = await openFrom() + act(() => useAppStore.getState().setAgentDashboardDrawerOpen(false)) + await closed() + expect(document.activeElement).toBe(first) + const second = await openFrom('Opener B') + await userEvent.keyboard('{Escape}') + await closed() + expect(document.activeElement).toBe(second) + }) + + it('does not let an old close callback take focus from a reopened drawer', async () => { + fixture() + await openFrom() + act(() => useAppStore.getState().setAgentDashboardDrawerOpen(false)) + const second = screen.getByRole('button', { name: 'Opener B' }) + second.focus() + act(() => useAppStore.getState().setAgentDashboardDrawerOpen(true)) + const search = screen.getByRole('textbox', { name: 'Search agents' }) + search.focus() + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + expect(document.activeElement).toBe(search) + await userEvent.keyboard('{Escape}') + await closed() + expect(document.activeElement).toBe(second) + }) + + it.each([true, false])('restores only when agent reveal fails (success: %s)', async (success) => { + fixture() + const opener = await openFrom() + mocks.activateAndRevealWorkspace.mockReturnValue(success) + await userEvent.click(screen.getByRole('button', { name: 'Reveal agent' })) + await closed() + expect(document.activeElement === opener).toBe(!success) + }) + + it('leaves focus handoff to the popout', async () => { + fixture() + const opener = await openFrom() + await userEvent.click(screen.getByRole('button', { name: 'Pop out' })) + await closed() + expect(mocks.openPopout).toHaveBeenCalledOnce() + expect(document.activeElement).not.toBe(opener) + }) +}) diff --git a/src/renderer/src/components/dashboard/AgentDashboardDrawer.tsx b/src/renderer/src/components/dashboard/AgentDashboardDrawer.tsx index 7e4a744d8b4..665295c835c 100644 --- a/src/renderer/src/components/dashboard/AgentDashboardDrawer.tsx +++ b/src/renderer/src/components/dashboard/AgentDashboardDrawer.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react' import { useAppStore } from '@/store' import type { AgentSubjectReadIntent } from '@/attention/agent-subject-read-actions' import { Sheet, SheetContent, SheetTitle } from '@/components/ui/sheet' @@ -33,9 +33,11 @@ const AGENT_BOARD_ESCAPE_BLOCKING_OVERLAY_SELECTOR = [ /** Mount only while open to keep live snapshot derivation off the closed path. */ function AgentDashboardDrawerBody({ onClose, + onFocusHandoff, onMenuOpenChange }: { onClose: () => void + onFocusHandoff: () => void onMenuOpenChange: (open: boolean) => void }): React.JSX.Element { const snapshot = useLiveDashboardSnapshot() @@ -47,17 +49,20 @@ function AgentDashboardDrawerBody({ }, []) const handleRevealAgent = useCallback( (args: AgentRevealArgs) => { - revealDashboardAgent(args) - onClose() + if (revealDashboardAgent(args)) { + onFocusHandoff() + } else { + onClose() + } }, - [onClose] + [onClose, onFocusHandoff] ) // Hand off to pop-out mode instead of retaining a drawer inconsistent with the setting. const handleSwitchToPopout = useCallback(() => { - onClose() + onFocusHandoff() void window.api.dashboard.openPopout?.() - }, [onClose]) + }, [onFocusHandoff]) return ( s.sidebarOpen) const sidebarWidth = useAppStore((s) => s.sidebarWidth) const [menuOpen, setMenuOpen] = useState(false) + const boardRef = useRef(null) + const focusReturnRef = useRef<{ opener: HTMLElement | null; handoff: boolean }>({ + opener: null, + handoff: false + }) + + // Capture per open transition: a reopen during the exit animation reuses the Sheet without open autofocus. + useLayoutEffect(() => { + if (!open) { + return + } + const focusReturn = focusReturnRef.current + focusReturn.handoff = false + const active = document.activeElement + if (!boardRef.current?.closest('[data-slot="sheet-content"]')?.contains(active)) { + focusReturn.opener = active instanceof HTMLElement && active !== document.body ? active : null + } + }, [open]) + + // No Radix trigger exists for this store-driven drawer, so return focus only when unmount dropped it to body. + const handleCloseAutoFocus = useCallback((event: Event) => { + event.preventDefault() + const { opener, handoff } = focusReturnRef.current + if (!handoff && (!document.activeElement || document.activeElement === document.body)) { + opener?.focus({ preventScroll: true }) + } + }, []) // Radix does not report close for unmounted menus; reset or the next drawer open could block outside dismissal. const close = useCallback(() => { setMenuOpen(false) setOpen(false) }, [setOpen]) + // Outside dismissal, agent reveal and pop-out move focus themselves. + const closeForFocusHandoff = useCallback(() => { + focusReturnRef.current.handoff = true + close() + }, [close]) // Store-driven closes bypass close(); clear menu state for sidebar collapse and workspace-board exclusivity too. useEffect(() => { if (!open) { @@ -112,13 +149,11 @@ export function AgentDashboardDrawer({ }, [setOpen] ) - const boardRef = useRef(null) - useWorkspaceKanbanOutsideDismiss({ open, boardRef, preserveOpenForMenu: menuOpen, - onOpenChange: setOpen + onOpenChange: closeForFocusHandoff }) useEffect(() => { @@ -203,13 +238,18 @@ export function AgentDashboardDrawer({ // Suppress default header focus, which reveals hover-style affordances without a pointer. event.preventDefault() }} + onCloseAutoFocus={handleCloseAutoFocus} onPointerDownOutside={guardSidebarInteraction} onInteractOutside={guardSidebarInteraction} > {translate('dashboardPopout.title', 'Agents')} {/* Closed SheetContent unmounts the live snapshot body, avoiding closed-path derivation. */}
- +