fix: make the account picker and the admin OAuth and plugin code dialogs proper modals

This commit is contained in:
Linus Rath
2026-10-29 18:02:58 +01:00
parent bead8ccbab
commit b96d875776
4 changed files with 174 additions and 104 deletions
+79 -84
View File
@@ -8,6 +8,7 @@ import { useAdminUnsavedChanges } from '@/stores/admin-unsaved-store';
import { DEFAULT_OAUTH_SCOPES } from '@/lib/oauth/scopes';
import { SettingsSection, SettingItem, SectionNav } from '@/components/settings/settings-section';
import { Button } from '@/components/ui/button';
import { Dialog } from '@/components/ui/dialog';
import { AdminPageHeader } from '@/components/admin/page-header';
import { AdminSaveBar } from '@/components/admin/save-bar';
import { withEdit } from '@/lib/admin/edits';
@@ -177,92 +178,86 @@ export function AuthPage() {
<div>
<AdminPageHeader title={PAGE.label} description={PAGE.description} />
{setupOpen && (
<div
className="fixed inset-0 z-50 flex items-center justify-center bg-black/50 backdrop-blur-sm p-4"
role="dialog"
aria-modal="true"
aria-labelledby="oauth-setup-title"
onClick={(e) => { if (e.target === e.currentTarget && !setupRunning) setSetupOpen(false); }}
>
<div className="w-full max-w-md rounded-lg border border-border bg-background shadow-xl">
<div className="px-5 py-4 border-b border-border">
<h3 id="oauth-setup-title" className="text-base font-medium text-foreground">Auto-configure OAuth</h3>
<p className="text-xs text-muted-foreground mt-1">
Verify the URLs below before continuing. The webmail and Stalwart can live on different domains.
</p>
</div>
<div className="px-5 py-4 space-y-4">
<div>
<label htmlFor="setup-origin" className="block text-xs font-medium text-foreground mb-1">
Webmail origin
</label>
<input
id="setup-origin"
type="url"
value={setupOrigin}
onChange={(e) => setSetupOrigin(e.target.value)}
disabled={setupRunning}
placeholder="https://webmail.example.com"
className={cn(inputClass, 'w-full')}
/>
<p className="text-[11px] text-muted-foreground mt-1">
Used to register redirect URIs (one per locale: <code>{setupOrigin.trim().replace(/\/+$/, '') || 'https://…'}/&lt;locale&gt;/auth/callback</code>) on Stalwart.
</p>
{!setupOriginValid && setupOrigin.length > 0 && (
<p className="text-[11px] text-destructive mt-1">Must be like https://host with no path.</p>
)}
</div>
<div>
<label htmlFor="setup-issuer" className="block text-xs font-medium text-foreground mb-1">
Stalwart issuer URL
</label>
<input
id="setup-issuer"
type="url"
value={setupIssuer}
onChange={(e) => setSetupIssuer(e.target.value)}
disabled={setupRunning}
placeholder="https://mail.example.com"
className={cn(inputClass, 'w-full')}
/>
<p className="text-[11px] text-muted-foreground mt-1">
Where Stalwart serves <code>/.well-known/oauth-authorization-server</code>. Saved as <code>OAUTH_ISSUER_URL</code>. Pre-filled from your JMAP server URL.
</p>
{!setupIssuerValid && setupIssuer.length > 0 && (
<p className="text-[11px] text-destructive mt-1">Must be like https://host with no path.</p>
)}
</div>
<label className="inline-flex items-center gap-2 text-xs text-foreground select-none cursor-pointer">
<input
type="checkbox"
checked={setupOauthOnly}
onChange={(e) => setSetupOauthOnly(e.target.checked)}
className="h-3.5 w-3.5 rounded border-input"
disabled={setupRunning}
/>
Also enable “OAuth only” (hide password login)
</label>
</div>
<div className="px-5 py-3 border-t border-border flex items-center justify-end gap-2 bg-muted/30 rounded-b-lg">
<Button type="button" variant="outline" size="sm" onClick={() => setSetupOpen(false)} disabled={setupRunning}>
Cancel
</Button>
<Button
type="button"
size="sm"
className="gap-2"
onClick={handleAutoSetup}
disabled={setupRunning || !setupOriginValid || !setupIssuerValid}
>
{setupRunning ? <Loader2 className="w-4 h-4 animate-spin" /> : <Sparkles className="w-4 h-4" />}
{setupRunning ? 'Configuring…' : 'Configure'}
</Button>
</div>
{/* The shared dialog: focus moved in and given back, Tab kept inside,
Escape and an outside click close it unless setup is running. */}
<Dialog
isOpen={setupOpen}
onClose={() => { if (!setupRunning) setSetupOpen(false); }}
preventOutsideClose={setupRunning}
showClose={!setupRunning}
title="Auto-configure OAuth"
description="Verify the URLs below before continuing. The webmail and Stalwart can live on different domains."
footer={
<>
<Button type="button" variant="outline" size="sm" onClick={() => setSetupOpen(false)} disabled={setupRunning}>
Cancel
</Button>
<Button
type="button"
size="sm"
className="gap-2"
onClick={handleAutoSetup}
disabled={setupRunning || !setupOriginValid || !setupIssuerValid}
>
{setupRunning ? <Loader2 className="w-4 h-4 animate-spin" aria-hidden="true" /> : <Sparkles className="w-4 h-4" aria-hidden="true" />}
{setupRunning ? 'Configuring…' : 'Configure'}
</Button>
</>
}
>
<div className="space-y-4">
<div>
<label htmlFor="setup-origin" className="block text-xs font-medium text-foreground mb-1">
Webmail origin
</label>
<input
id="setup-origin"
type="url"
value={setupOrigin}
onChange={(e) => setSetupOrigin(e.target.value)}
disabled={setupRunning}
placeholder="https://webmail.example.com"
className={cn(inputClass, 'w-full')}
/>
<p className="text-[11px] text-muted-foreground mt-1">
Used to register redirect URIs (one per locale: <code>{setupOrigin.trim().replace(/\/+$/, '') || 'https://…'}/&lt;locale&gt;/auth/callback</code>) on Stalwart.
</p>
{!setupOriginValid && setupOrigin.length > 0 && (
<p className="text-[11px] text-destructive mt-1">Must be like https://host with no path.</p>
)}
</div>
<div>
<label htmlFor="setup-issuer" className="block text-xs font-medium text-foreground mb-1">
Stalwart issuer URL
</label>
<input
id="setup-issuer"
type="url"
value={setupIssuer}
onChange={(e) => setSetupIssuer(e.target.value)}
disabled={setupRunning}
placeholder="https://mail.example.com"
className={cn(inputClass, 'w-full')}
/>
<p className="text-[11px] text-muted-foreground mt-1">
Where Stalwart serves <code>/.well-known/oauth-authorization-server</code>. Saved as <code>OAUTH_ISSUER_URL</code>. Pre-filled from your JMAP server URL.
</p>
{!setupIssuerValid && setupIssuer.length > 0 && (
<p className="text-[11px] text-destructive mt-1">Must be like https://host with no path.</p>
)}
</div>
<label className="inline-flex items-center gap-2 text-xs text-foreground select-none cursor-pointer">
<input
type="checkbox"
checked={setupOauthOnly}
onChange={(e) => setSetupOauthOnly(e.target.checked)}
className="h-3.5 w-3.5 rounded border-input"
disabled={setupRunning}
/>
Also enable “OAuth only” (hide password login)
</label>
</div>
)}
</Dialog>
<div className="space-y-10">
<SectionNav />
@@ -6,6 +6,7 @@ import { apiFetch } from '@/lib/browser-navigation';
import { HIGH_RISK_PERMISSIONS } from '@/lib/plugin-sandbox/permission-text';
import { SettingsSection } from '@/components/settings/settings-section';
import { Button } from '@/components/ui/button';
import { Dialog } from '@/components/ui/dialog';
import { useAdminShell } from '@/components/admin/shell-context';
import { Badge, type BadgeTone } from '@/components/admin/badge';
@@ -186,24 +187,26 @@ export function PluginApprovalsPanel() {
</div>
)}
{source && (
<div className="fixed inset-0 z-50 flex items-center justify-center bg-black/50 p-4" onMouseDown={(e) => { if (e.target === e.currentTarget) setSource(null); }}>
<div role="dialog" aria-modal="true" aria-label="Plugin code" className="flex max-h-[90vh] w-full max-w-4xl flex-col rounded-lg border border-border bg-background shadow-xl">
<div className="flex items-center justify-between border-b border-border px-4 py-3">
<span className="text-sm font-medium">{source.entry.manifest.name ?? source.entry.pluginId} v{source.entry.manifest.version}</span>
<button type="button" onClick={() => setSource(null)} aria-label="Close" className="rounded p-1 hover:bg-accent"><X className="h-4 w-4" /></button>
</div>
{source.warnings && source.warnings.length > 0 && (
<div className="flex items-start gap-2 border-b border-border px-4 py-2 text-xs text-foreground">
<AlertTriangle className="mt-0.5 h-3.5 w-3.5 text-warning" /> {source.warnings.join('; ')}
</div>
)}
<pre className="min-h-0 flex-1 overflow-auto whitespace-pre-wrap break-all p-4 font-mono text-[11px] leading-relaxed">
{source.error ?? source.code ?? (<span className="inline-flex items-center gap-2"><Loader2 className="h-3.5 w-3.5 animate-spin" /> Loading</span>)}
</pre>
{/* The shared dialog: focus moved in and given back, Tab kept inside,
Escape closes, and the title names the plugin. */}
<Dialog
isOpen={!!source}
onClose={() => setSource(null)}
title={source ? `Code of ${source.entry.manifest.name ?? source.entry.pluginId} v${source.entry.manifest.version}` : ''}
size="lg"
className="max-w-4xl"
bodyClassName="flex flex-col"
>
{source?.warnings && source.warnings.length > 0 && (
<div className="mb-2 flex items-start gap-2 text-xs text-foreground">
<AlertTriangle className="mt-0.5 h-3.5 w-3.5 text-warning" aria-hidden="true" /> {source.warnings.join('; ')}
</div>
</div>
)}
)}
{/* Focusable, so the keyboard can scroll the code. */}
<pre tabIndex={0} aria-label="Plugin code" className="min-h-0 flex-1 overflow-auto whitespace-pre-wrap break-all rounded-md border border-border p-4 font-mono text-[11px] leading-relaxed">
{source?.error ?? source?.code ?? (<span className="inline-flex items-center gap-2"><Loader2 className="h-3.5 w-3.5 animate-spin" aria-hidden="true" /> Loading</span>)}
</pre>
</Dialog>
</SettingsSection>
);
}
@@ -0,0 +1,53 @@
import { fireEvent, render, screen } from '@testing-library/react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { ProtocolAccountPicker } from '../protocol-account-picker';
import type { AccountEntry } from '@/stores/account-store';
// The "which account?" question for a mailto: or webcal: link is a modal:
// the focus starts on the account in use, Escape cancels, and the focus
// comes back to where it was.
const account = (id: string, email: string) =>
({ id, email, label: email, username: email, serverUrl: 'https://mail.example.org', displayName: '' }) as unknown as AccountEntry;
let opener: HTMLButtonElement;
beforeEach(() => {
opener = document.createElement('button');
document.body.appendChild(opener);
opener.focus();
});
afterEach(() => opener.remove());
describe('ProtocolAccountPicker', () => {
it('focuses the active account, cancels on Escape and gives the focus back', () => {
const onCancel = vi.fn();
const { unmount } = render(
<ProtocolAccountPicker
kind="mailto"
accounts={[account('a', 'ann@example.org'), account('b', 'bob@example.org')]}
activeAccountId="b"
onSelect={() => {}}
onCancel={onCancel}
/>,
);
const dialog = screen.getByRole('dialog', { name: 'select_account_title' });
const active = screen.getByText('bob@example.org', { selector: 'span' }).closest('button')!;
expect(active).toHaveFocus();
expect(active).toHaveAttribute('aria-current', 'true');
fireEvent.keyDown(active, { key: 'Escape' });
expect(onCancel).toHaveBeenCalledTimes(1);
expect(dialog).toBeInTheDocument();
unmount();
expect(opener).toHaveFocus();
});
it('ignores Escape while it switches accounts', () => {
const onCancel = vi.fn();
render(
<ProtocolAccountPicker kind="webcal" accounts={[account('a', 'ann@example.org')]} activeAccountId="a" isSwitching onSelect={() => {}} onCancel={onCancel} />,
);
fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' });
expect(onCancel).not.toHaveBeenCalled();
});
});
@@ -7,6 +7,8 @@ import type { ParsedWebcal } from "@/lib/protocol-handlers/webcal";
import type { AccountEntry } from "@/stores/account-store";
import { cn } from "@/lib/utils";
import { Avatar } from "@/components/ui/avatar";
import { useId } from "react";
import { useFocusTrap } from "@/hooks/use-focus-trap";
type ProtocolAccountPickerProps = {
accounts: AccountEntry[];
@@ -50,18 +52,32 @@ export function ProtocolAccountPicker({
]
: [];
// A modal: the focus starts on the account in use (else the first one),
// Tab stays inside, Escape cancels unless a switch is under way, and the
// focus goes back to where it was.
const titleId = useId();
const panelRef = useFocusTrap({
isActive: true,
onEscape: () => { if (!isSwitching) onCancel(); },
restoreFocus: true,
initialFocus: () =>
panelRef.current?.querySelector<HTMLElement>('[data-active-account="true"]')
?? panelRef.current?.querySelector<HTMLElement>('[data-account-option]'),
});
return (
<div className="fixed inset-0 z-50 flex items-center justify-center p-4">
<div className="absolute inset-0 bg-black/50 backdrop-blur-[1px]" onClick={onCancel} aria-hidden="true" />
<div
ref={panelRef}
role="dialog"
aria-modal="true"
aria-label={t("select_account_title")}
aria-labelledby={titleId}
className="relative w-full max-w-md rounded-lg border border-border bg-background shadow-xl animate-in zoom-in-95 duration-200"
>
<div className="flex items-start justify-between gap-4 border-b border-border px-5 py-4">
<div>
<h2 className="text-lg font-semibold text-foreground">{t("select_account_title")}</h2>
<h2 id={titleId} className="text-lg font-semibold text-foreground">{t("select_account_title")}</h2>
<p className="mt-1 text-sm text-muted-foreground">
{kind === "mailto" ? t("select_mailto_account") : t("select_webcal_account")}
</p>
@@ -72,7 +88,7 @@ export function ProtocolAccountPicker({
className="rounded-md p-1.5 text-muted-foreground transition-colors hover:bg-muted hover:text-foreground"
aria-label={tCommon("close")}
>
<X className="h-5 w-5" />
<X className="h-5 w-5" aria-hidden="true" />
</button>
</div>
@@ -103,6 +119,9 @@ export function ProtocolAccountPicker({
<button
key={account.id}
type="button"
data-account-option=""
data-active-account={isActive || undefined}
aria-current={isActive || undefined}
disabled={isSwitching}
onClick={() => onSelect(account.id)}
className={cn(