From bead8ccbab7e16ad2fe61583ec3cdb71f94d27e1 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:48:32 +0100 Subject: [PATCH] fix: keep the keyboard focus inside plugin dialogs and return it when they close --- .../__tests__/plugin-connect-dialog.test.tsx | 19 +++++++ .../__tests__/plugin-dialog-focus.test.tsx | 55 +++++++++++++++++++ components/plugins/plugin-connect-dialog.tsx | 17 ++++-- components/plugins/plugin-dialog-host.tsx | 31 +++++++++-- 4 files changed, 111 insertions(+), 11 deletions(-) create mode 100644 components/plugins/__tests__/plugin-dialog-focus.test.tsx diff --git a/components/plugins/__tests__/plugin-connect-dialog.test.tsx b/components/plugins/__tests__/plugin-connect-dialog.test.tsx index ec676144f..d10484c30 100644 --- a/components/plugins/__tests__/plugin-connect-dialog.test.tsx +++ b/components/plugins/__tests__/plugin-connect-dialog.test.tsx @@ -78,6 +78,25 @@ describe('connect dialog', () => { await expect(result).resolves.toEqual({ connected: false }); }); + // A modal: the first field takes the focus, Tab stays inside, and the + // focus goes back to what opened it. + it('holds the focus while open and gives it back after', async () => { + const opener = document.createElement('button'); + document.body.appendChild(opener); + opener.focus(); + render(); + const result = open(); + const server = screen.getByText('server').parentElement!.querySelector('input')!; + expect(server).toHaveFocus(); + const dialog = screen.getByRole('dialog'); + fireEvent.keyDown(server, { key: 'Tab', shiftKey: true }); + expect(dialog).toContainElement(document.activeElement as HTMLElement); + fireEvent.click(screen.getByRole('button', { name: 'cancel' })); + await expect(result).resolves.toEqual({ connected: false }); + await waitFor(() => expect(opener).toHaveFocus()); + opener.remove(); + }); + it('shows the server error and stays open', async () => { apiFetch.mockResolvedValueOnce(new Response(JSON.stringify({ error: 'The server address must start with https://' }), { status: 400 })); render(); diff --git a/components/plugins/__tests__/plugin-dialog-focus.test.tsx b/components/plugins/__tests__/plugin-dialog-focus.test.tsx new file mode 100644 index 000000000..577ed2f3b --- /dev/null +++ b/components/plugins/__tests__/plugin-dialog-focus.test.tsx @@ -0,0 +1,55 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; + +// A plugin's dialog is a real modal for the keyboard: the focus moves in to +// the safe choice, Tab can't leave it, and the focus comes back to where it +// was once the dialog is answered. + +vi.mock('next-intl', () => ({ useTranslations: () => (key: string) => key })); +vi.mock('../plugin-iframe-slot', () => ({ PluginIframeSlot: () =>
})); + +import { PluginDialogHost } from '../plugin-dialog-host'; +import { awaitDialog, awaitPrompt, head, resetDialogLimits, resolveHead } from '@/lib/plugin-sandbox/host-dialog'; + +let opener: HTMLButtonElement; +beforeEach(() => { + while (head()) resolveHead(null); + resetDialogLimits(); + opener = document.createElement('button'); + opener.textContent = 'opener'; + document.body.appendChild(opener); + opener.focus(); +}); +afterEach(() => opener.remove()); + +describe('plugin dialog focus', () => { + it('starts on Cancel for a destructive confirm, keeps Tab inside, and returns the focus', async () => { + render(); + let answer: Promise; + act(() => { answer = awaitDialog({ pluginId: 'p', kind: 'confirm', title: 'Delete it?', message: '', danger: true }); }); + + const cancel = screen.getByRole('button', { name: 'cancel' }); + expect(cancel).toHaveFocus(); + const dialog = screen.getByRole('dialog', { name: 'Delete it?' }); + fireEvent.keyDown(cancel, { key: 'Tab', shiftKey: true }); + expect(dialog).toContainElement(document.activeElement as HTMLElement); + + await act(async () => { fireEvent.click(cancel); }); + await expect(answer!).resolves.toBe(false); + await waitFor(() => expect(opener).toHaveFocus()); + }); + + it('starts in the first field of a prompt and marks required fields', async () => { + render(); + let answer: Promise | null>; + act(() => { + answer = awaitPrompt({ pluginId: 'p', kind: 'prompt', title: 'Name', message: '', fields: [{ name: 'n', label: 'Name', required: true }] }); + }); + const input = screen.getByRole('textbox', { name: 'Name' }); + expect(input).toHaveFocus(); + expect(input).toHaveAttribute('aria-required', 'true'); + act(() => { fireEvent.keyDown(document, { key: 'Escape' }); }); + await expect(answer!).resolves.toBeNull(); + await waitFor(() => expect(opener).toHaveFocus()); + }); +}); diff --git a/components/plugins/plugin-connect-dialog.tsx b/components/plugins/plugin-connect-dialog.tsx index 6e644facd..50abe8874 100644 --- a/components/plugins/plugin-connect-dialog.tsx +++ b/components/plugins/plugin-connect-dialog.tsx @@ -5,6 +5,7 @@ import { useTranslations } from 'next-intl'; import { KeyRound } from '@/components/icons'; import { Button } from '@/components/ui/button'; import { useConfig } from '@/hooks/use-config'; +import { useFocusTrap } from '@/hooks/use-focus-trap'; import { productName } from '@/lib/product-name'; import { currentConnect, @@ -46,6 +47,15 @@ export function PluginConnectDialog() { return () => window.removeEventListener('keydown', onKey); }, [request]); + // Tab stays in the dialog, the first field takes the focus, and the focus + // goes back to where it was when the dialog closes. (Escape is the + // listener above.) + const panelRef = useFocusTrap({ + isActive: !!request, + restoreFocus: true, + initialFocus: () => panelRef.current?.querySelector('input:not(:disabled)'), + }); + if (!request) return null; const { spec, mode, pluginName } = request; @@ -96,10 +106,10 @@ export function PluginConnectDialog() { if (e.target === e.currentTarget) request.resolve({ connected: false }); }} > -
+
{ e.preventDefault(); void submit(); }}>
- +

{intro}

@@ -116,7 +126,6 @@ export function PluginConnectDialog() { spellCheck={false} value={server} placeholder={t('server_hint')} - autoFocus onChange={(e) => setServer(e.target.value)} className="w-full rounded-md border border-input bg-background px-2.5 py-2 text-sm text-foreground outline-none focus:ring-2 focus:ring-ring" /> @@ -129,7 +138,6 @@ export function PluginConnectDialog() { type="text" value={username} autoComplete="username" - autoFocus={!!fixedOrigin} onChange={(e) => setUsername(e.target.value)} className="w-full rounded-md border border-input bg-background px-2.5 py-2 text-sm text-foreground outline-none focus:ring-2 focus:ring-ring" /> @@ -142,7 +150,6 @@ export function PluginConnectDialog() { type="password" value={secret} autoComplete="off" - autoFocus={!!fixedOrigin && !needsUsername} onChange={(e) => setSecret(e.target.value)} className="w-full rounded-md border border-input bg-background px-2.5 py-2 text-sm text-foreground outline-none focus:ring-2 focus:ring-ring" /> diff --git a/components/plugins/plugin-dialog-host.tsx b/components/plugins/plugin-dialog-host.tsx index a98ff0138..7032cd9ad 100644 --- a/components/plugins/plugin-dialog-host.tsx +++ b/components/plugins/plugin-dialog-host.tsx @@ -16,6 +16,7 @@ import { useTranslations } from 'next-intl'; import { cancelForPlugin, head, overlay, resolveDialog, subscribe, type DialogRequest } from '@/lib/plugin-sandbox/host-dialog'; import type { DialogButtonSpec } from '@/lib/plugin-sandbox/ui-kit/dialog-buttons'; import { usePluginStore } from '@/stores/plugin-store'; +import { useFocusTrap } from '@/hooks/use-focus-trap'; import { toast } from '@/stores/toast-store'; import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; @@ -165,6 +166,23 @@ function PluginDialogView({ current, layer }: { current: DialogRequest; layer: ' // eslint-disable-next-line react-hooks/exhaustive-deps }, [current, canSubmit, values]); + // Tab stays in the dialog and the focus goes back to what opened it. The + // first field of a prompt takes the focus, a confirm's safe button (Cancel + // when it is destructive), else the first control. A dialog under another + // one lets go. Escape is the listener above. + const cancelButtonRef = useRef(null); + const confirmButtonRef = useRef(null); + const panelRef = useFocusTrap({ + isActive: !!current && layer !== 'below', + restoreFocus: true, + initialFocus: () => { + if (current?.kind === 'prompt') return panelRef.current?.querySelector('input'); + if (current?.kind === 'confirm' && current.danger) return cancelButtonRef.current; + if (current?.kind === 'confirm' || current?.kind === 'alert') return confirmButtonRef.current; + return undefined; + }, + }); + if (!current) return null; const name = pluginName ?? current.pluginId; @@ -200,6 +218,7 @@ function PluginDialogView({ current, layer }: { current: DialogRequest; layer: ' }} >
- +
@@ -262,16 +281,16 @@ function PluginDialogView({ current, layer }: { current: DialogRequest; layer: ' {isPrompt ? ( { e.preventDefault(); submitPrompt(); }}>
- {fields.map((f, i) => ( + {fields.map((f) => (