fix: name the security settings fields and let a settings row's label name its field

This commit is contained in:
Linus Rath
2026-10-29 18:03:00 +01:00
parent 2038054b0b
commit dcc21acd07
3 changed files with 88 additions and 26 deletions
@@ -0,0 +1,28 @@
import { act, fireEvent, render, screen } from '@testing-library/react';
import { describe, expect, it } from 'vitest';
import { unnamedFields } from '@/lib/__tests__/helpers/form-labels';
import { AccountSecuritySettings } from '@/components/settings/account-security-settings';
import { useAccountSecurityStore } from '@/stores/account-security-store';
import { useAuthStore } from '@/stores/auth-store';
// Settings > Security: the password, app password, API key and public key
// forms name every field (see form-labels.test.tsx).
describe('security settings fields have labels', () => {
it('names every field, with each add form open', () => {
useAuthStore.setState({ isAuthenticated: false, authMode: 'basic', client: null } as never);
useAccountSecurityStore.setState({
isStalwart: true, isProbing: false, otpEnabled: false, isSaving: false,
isLoadingAuth: false, isLoadingPrincipal: false, displayName: 'Ann',
appPasswords: [], apiKeys: [], publicKeys: [],
} as never);
render(<AccountSecuritySettings />);
// Open every add form there is.
for (const name of [/app_passwords\.new/, /api_keys\.new/, /public_keys\.add/]) {
const button = screen.queryAllByRole('button', { name }).at(0);
if (button) act(() => { fireEvent.click(button); });
}
expect(document.querySelectorAll('input, select, textarea').length).toBeGreaterThan(5);
expect(unnamedFields()).toEqual([]);
});
});
@@ -24,8 +24,15 @@ import { ConfirmDialog } from '@/components/ui/confirm-dialog';
import { IconButton } from '@/components/ui/icon-button';
import { useConfirmDialog } from '@/hooks/use-confirm-dialog';
/** Ids that tie a visible label to its field: labelFor("key"). */
function useLabelIds() {
const base = useId();
return (key: string) => `${base}-${key}`;
}
function PasswordChangeSection() {
const t = useTranslations('settings.security');
const labelFor = useLabelIds();
const tLogin = useTranslations('login');
const { changePassword, isSaving, otpEnabled } = useAccountSecurityStore();
const [currentPassword, setCurrentPassword] = useState('');
@@ -78,9 +85,9 @@ function PasswordChangeSection() {
</div>
<form onSubmit={handleSubmit} className="space-y-3">
<div>
<label className="text-xs text-muted-foreground mb-1 block">{t('password.current')}</label>
<label htmlFor={labelFor("current")} className="text-xs text-muted-foreground mb-1 block">{t('password.current')}</label>
<div className="relative">
<Input
<Input id={labelFor("current")}
type={showCurrent ? 'text' : 'password'}
value={currentPassword}
onChange={(e) => setCurrentPassword(e.target.value)}
@@ -100,8 +107,8 @@ function PasswordChangeSection() {
</div>
{otpEnabled && (
<div>
<label className="text-xs text-muted-foreground mb-1 block">{t('totp.verification_code')}</label>
<Input
<label htmlFor={labelFor("verification_code")} className="text-xs text-muted-foreground mb-1 block">{t('totp.verification_code')}</label>
<Input id={labelFor("verification_code")}
value={otpCode}
onChange={(e) => setOtpCode(e.target.value)}
required
@@ -112,9 +119,9 @@ function PasswordChangeSection() {
</div>
)}
<div>
<label className="text-xs text-muted-foreground mb-1 block">{t('password.new')}</label>
<label htmlFor={labelFor("new")} className="text-xs text-muted-foreground mb-1 block">{t('password.new')}</label>
<div className="relative">
<Input
<Input id={labelFor("new")}
type={showNew ? 'text' : 'password'}
value={newPassword}
onChange={(e) => setNewPassword(e.target.value)}
@@ -134,8 +141,8 @@ function PasswordChangeSection() {
</div>
</div>
<div>
<label className="text-xs text-muted-foreground mb-1 block">{t('password.confirm')}</label>
<Input
<label htmlFor={labelFor("confirm")} className="text-xs text-muted-foreground mb-1 block">{t('password.confirm')}</label>
<Input id={labelFor("confirm")}
type="password"
value={confirmPassword}
onChange={(e) => setConfirmPassword(e.target.value)}
@@ -228,6 +235,7 @@ function generateTotp(accountLabel: string, issuer: string): { totp: OTPAuth.TOT
function TotpSection() {
const t = useTranslations('settings.security');
const labelFor = useLabelIds();
const { otpEnabled, enableTotp, disableTotp, isSaving, isLoadingAuth } = useAccountSecurityStore();
const { client } = useAuthStore();
const { appName } = useConfig();
@@ -345,12 +353,12 @@ function TotpSection() {
<code dir="ltr" className="text-xs bg-background px-2 py-1 rounded border border-border flex-1 truncate">{setupUrl}</code>
</div>
<div>
<label className="text-xs text-muted-foreground mb-1 block">{t('password.current')}</label>
<Input type="password" value={password} onChange={(e) => setPassword(e.target.value)} autoComplete="current-password" />
<label htmlFor={labelFor("current-1")} className="text-xs text-muted-foreground mb-1 block">{t('password.current')}</label>
<Input id={labelFor("current-1")} type="password" value={password} onChange={(e) => setPassword(e.target.value)} autoComplete="current-password" />
</div>
<div>
<label className="text-xs text-muted-foreground mb-1 block">{t('totp.verification_code')}</label>
<Input value={otpCode} onChange={(e) => setOtpCode(e.target.value)} inputMode="numeric" maxLength={6} />
<label htmlFor={labelFor("verification_code-1")} className="text-xs text-muted-foreground mb-1 block">{t('totp.verification_code')}</label>
<Input id={labelFor("verification_code-1")} value={otpCode} onChange={(e) => setOtpCode(e.target.value)} inputMode="numeric" maxLength={6} />
</div>
{setupError && <p className="text-xs text-destructive">{setupError}</p>}
<div className="flex gap-2">
@@ -371,12 +379,14 @@ function TotpSection() {
value={password}
onChange={(e) => setPassword(e.target.value)}
placeholder={t('password.current')}
aria-label={t('password.current')}
autoComplete="current-password"
/>
<Input
value={otpCode}
onChange={(e) => setOtpCode(e.target.value)}
placeholder={t('totp.verification_code')}
aria-label={t('totp.verification_code')}
inputMode="numeric"
autoComplete="one-time-code"
maxLength={6}
@@ -450,6 +460,7 @@ interface CredentialSectionProps {
function CredentialSection({ icon: Icon, i18nNamespace, entries, onCreate, onRemove }: CredentialSectionProps) {
const t = useTranslations('settings.security');
const labelFor = useLabelIds();
const tCommon = useTranslations('common');
const tk = (key: string) => t(`${i18nNamespace}.${key}`);
const { isSaving, isLoadingAuth } = useAccountSecurityStore();
@@ -555,8 +566,8 @@ function CredentialSection({ icon: Icon, i18nNamespace, entries, onCreate, onRem
{showAdd && (
<form onSubmit={handleAdd} className="p-3 bg-muted rounded-md space-y-2">
<div>
<label className="text-xs text-muted-foreground mb-1 block">{tk('name_label')}</label>
<Input
<label htmlFor={labelFor("field6")} className="text-xs text-muted-foreground mb-1 block">{tk('name_label')}</label>
<Input id={labelFor("field6")}
value={newDescription}
onChange={(e) => setNewDescription(e.target.value)}
placeholder={tk('name_placeholder')}
@@ -564,12 +575,12 @@ function CredentialSection({ icon: Icon, i18nNamespace, entries, onCreate, onRem
/>
</div>
<div>
<label className="text-xs text-muted-foreground mb-1 block">{t('app_passwords.expires_label')}</label>
<Input type="date" value={expiresAt} onChange={(e) => setExpiresAt(e.target.value)} />
<label htmlFor={labelFor("expires_label")} className="text-xs text-muted-foreground mb-1 block">{t('app_passwords.expires_label')}</label>
<Input id={labelFor("expires_label")} type="date" value={expiresAt} onChange={(e) => setExpiresAt(e.target.value)} />
</div>
<div>
<label className="text-xs text-muted-foreground mb-1 block">{t('app_passwords.allowed_ips_label')}</label>
<textarea
<label htmlFor={labelFor("allowed_ips_label")} className="text-xs text-muted-foreground mb-1 block">{t('app_passwords.allowed_ips_label')}</label>
<textarea id={labelFor("allowed_ips_label")}
value={allowedIpsRaw}
onChange={(e) => setAllowedIpsRaw(e.target.value)}
placeholder={t('app_passwords.allowed_ips_placeholder')}
@@ -632,6 +643,7 @@ function ApiKeysSection() {
function PublicKeysSection() {
const t = useTranslations('settings.security');
const labelFor = useLabelIds();
const tCommon = useTranslations('common');
const tk = (key: string) => t(`public_keys.${key}`);
const {
@@ -766,8 +778,8 @@ function PublicKeysSection() {
{showAdd && (
<form onSubmit={handleAdd} className="p-3 bg-muted rounded-md space-y-2">
<div>
<label className="text-xs text-muted-foreground mb-1 block">{tk('name_label')}</label>
<Input
<label htmlFor={labelFor("field9")} className="text-xs text-muted-foreground mb-1 block">{tk('name_label')}</label>
<Input id={labelFor("field9")}
value={description}
onChange={(e) => setDescription(e.target.value)}
placeholder={tk('name_placeholder')}
@@ -775,8 +787,8 @@ function PublicKeysSection() {
/>
</div>
<div>
<label className="text-xs text-muted-foreground mb-1 block">{tk('key_label')}</label>
<textarea
<label htmlFor={labelFor("field10")} className="text-xs text-muted-foreground mb-1 block">{tk('key_label')}</label>
<textarea id={labelFor("field10")}
value={publicKey}
onChange={(e) => setPublicKey(e.target.value)}
placeholder={tk('key_placeholder')}
@@ -802,8 +814,8 @@ function PublicKeysSection() {
<h5 className="text-xs font-semibold text-foreground">{t('encryption.configure_title')}</h5>
<div>
<label className="text-xs text-muted-foreground mb-1 block">{t('encryption.algorithm_label')}</label>
<select
<label htmlFor={labelFor("algorithm_label")} className="text-xs text-muted-foreground mb-1 block">{t('encryption.algorithm_label')}</label>
<select id={labelFor("algorithm_label")}
value={selectedAlgorithm}
onChange={(e) => setSelectedAlgorithm(e.target.value as 'Aes128' | 'Aes256')}
className="w-full text-xs px-3 py-1.5 rounded-md border border-border bg-background focus:outline-none focus:ring-2 focus:ring-ring"
@@ -916,6 +928,7 @@ function PublicKeysSection() {
function EmailClientSection() {
const t = useTranslations('settings.security');
const labelFor = useLabelIds();
const { client } = useAuthStore();
const [copied, setCopied] = useState(false);
@@ -937,11 +950,12 @@ function EmailClientSection() {
<p className="text-xs text-muted-foreground">{t('email_client.description')}</p>
<div className="p-3 bg-muted/70 dark:bg-muted/40 rounded-md space-y-2">
<div>
<label className="text-xs text-muted-foreground mb-1 block">
<label htmlFor={labelFor('jmap-username')} className="text-xs text-muted-foreground mb-1 block">
{t('email_client.jmap_username_label')}
</label>
<div className="flex rounded-lg">
<input
id={labelFor('jmap-username')}
type="text"
readOnly
value={jmapUsername}
+21 -1
View File
@@ -221,9 +221,29 @@ interface SettingItemProps {
nested?: boolean;
}
/** A field that has a name of its own: aria-label, aria-labelledby or a <label>. */
function hasOwnName(el: HTMLElement): boolean {
if (el.getAttribute('aria-label') || el.getAttribute('aria-labelledby')) return true;
if (el.closest('label')) return true;
return !!el.id && !!el.ownerDocument.querySelector(`label[for="${CSS.escape(el.id)}"]`);
}
export function SettingItem({ label, description, children, locked, advanced, nested }: SettingItemProps) {
const labelId = useId();
const visible = useAdvancedVisible(advanced);
// A row with one text field, select or textarea and nothing naming it
// (an <Input> dropped into the row) borrows the row's label, like the
// toggles do through SettingLabelContext. Rows with several fields name
// them one by one.
const controlsRef = useRef<HTMLDivElement>(null);
useEffect(() => {
const box = controlsRef.current;
if (!box) return;
const fields = box.querySelectorAll<HTMLElement>('input:not([type="hidden"]):not([type="checkbox"]):not([type="radio"]), select, textarea');
if (fields.length !== 1) return;
const field = fields[0];
if (!hasOwnName(field)) field.setAttribute('aria-labelledby', labelId);
});
if (!visible) return null;
return (
<div
@@ -245,7 +265,7 @@ export function SettingItem({ label, description, children, locked, advanced, ne
</div>
<SettingLabelContext.Provider value={labelId}>
{/* The admin's value stays readable; inert keeps mouse and keyboard off it. */}
<div className={cn("flex-shrink-0", locked && "opacity-60")} inert={locked || undefined}>{children}</div>
<div ref={controlsRef} className={cn("flex-shrink-0", locked && "opacity-60")} inert={locked || undefined}>{children}</div>
</SettingLabelContext.Provider>
</div>
);