fix: name every icon button and switch in Settings for what it does

This commit is contained in:
Linus Rath
2026-10-28 04:50:27 +01:00
parent ab3addefc3
commit d0134c3f46
35 changed files with 215 additions and 79 deletions
+12 -13
View File
@@ -4,7 +4,8 @@ import { useState, useCallback, useMemo, useRef } from "react";
import { useTranslations } from "next-intl";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import { Plus, Trash2 } from "@/components/icons";
import { Plus, X } from "@/components/icons";
import { IconButton } from "@/components/ui/icon-button";
import { Dialog } from "@/components/ui/dialog";
import { useDiscardChanges } from "@/components/settings/use-discard-changes";
import type {
@@ -602,15 +603,14 @@ export function FilterRuleModal({
/>
)}
<button
type="button"
<IconButton
label={t("remove_condition")}
onClick={() => removeCondition(index)}
disabled={conditions.length <= 1}
className="p-1.5 rounded hover:bg-muted text-muted-foreground hover:text-destructive transition-colors disabled:opacity-30 disabled:pointer-events-none"
aria-label={t("delete_rule")}
className="text-muted-foreground hover:text-destructive disabled:opacity-30"
>
<Trash2 className="w-4 h-4" />
</button>
<X />
</IconButton>
</div>
))}
</div>
@@ -727,15 +727,14 @@ export function FilterRuleModal({
</select>
)}
<button
type="button"
<IconButton
label={t("remove_action")}
onClick={() => removeAction(index)}
disabled={actions.length <= 1}
className="p-1.5 rounded hover:bg-muted text-muted-foreground hover:text-destructive transition-colors disabled:opacity-30 disabled:pointer-events-none"
aria-label={t("delete_rule")}
className="text-muted-foreground hover:text-destructive disabled:opacity-30"
>
<Trash2 className="w-4 h-4" />
</button>
<X />
</IconButton>
</div>
))}
</div>
@@ -0,0 +1,60 @@
import { render, screen } from '@testing-library/react';
import { afterEach, describe, expect, it, vi } from 'vitest';
import type { FilterRule } from '@/lib/jmap/sieve-types';
import { FilterRuleModal } from '@/components/filters/filter-rule-modal';
import { useAuthStore } from '@/stores/auth-store';
import { useEmailStore } from '@/stores/email-store';
import { useFilterStore } from '@/stores/filter-store';
import { FilterSettings } from '../filter-settings';
// SET-11: icon buttons and switches in Settings have a name that says what
// they do. The rule dialog's remove buttons were both named "Delete Rule";
// the rule switches on the Filters page had none.
vi.mock('@/stores/toast-store', () => ({ toast: { success: vi.fn(), error: vi.fn(), info: vi.fn() } }));
vi.mock('../settings-section', async (importOriginal) => ({
...(await importOriginal<typeof import('../settings-section')>()),
SettingsSection: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
}));
const rule: FilterRule = {
id: 'r1', name: 'Newsletters', enabled: true, matchType: 'all',
conditions: [{ field: 'from', comparator: 'contains', value: 'a' }, { field: 'subject', comparator: 'contains', value: 'b' }],
actions: [{ type: 'star' }, { type: 'mark_read' }], stopProcessing: false,
};
const initial = { auth: useAuthStore.getState(), email: useEmailStore.getState(), filter: useFilterStore.getState() };
afterEach(() => {
useAuthStore.setState(initial.auth, true);
useEmailStore.setState(initial.email, true);
useFilterStore.setState(initial.filter, true);
});
function unnamedButtons(root: HTMLElement) {
return Array.from(root.querySelectorAll('button, [role="switch"]')).filter((b) => {
const name = b.getAttribute('aria-label') || b.getAttribute('aria-labelledby') || b.textContent?.trim() || b.getAttribute('title');
return !name;
});
}
describe('names of settings controls', () => {
it('names the remove buttons of a rule for what they remove', () => {
render(<FilterRuleModal rule={rule} mailboxes={[]} onSave={vi.fn()} onClose={vi.fn()} />);
expect(screen.getAllByRole('button', { name: 'remove_condition' })).toHaveLength(2);
expect(screen.getAllByRole('button', { name: 'remove_action' })).toHaveLength(2);
expect(screen.queryByRole('button', { name: 'delete_rule' })).toBeNull();
expect(unnamedButtons(screen.getByRole('dialog'))).toEqual([]);
});
it('names each rule switch on the Filters page after its rule', () => {
useAuthStore.setState({ client: { updateSieveScript: vi.fn(), getSieveAccountId: () => 'primary' } as never });
useEmailStore.setState({ fetchMailboxes: vi.fn(async () => {}) as never });
useFilterStore.setState({
rules: [rule], isSupported: true, isLoading: false, isOpaque: false, activeScriptId: 's',
selectedAccountId: 'primary', sieveCapabilities: null, selectAccount: vi.fn(async () => {}),
});
const { container } = render(<FilterSettings />);
expect(screen.getByRole('switch', { name: 'Newsletters' })).toBeTruthy();
expect(unnamedButtons(container)).toEqual([]);
});
});
@@ -20,10 +20,12 @@ import { IS_LITE } from '@/lib/lite';
import { useConfig } from '@/hooks/use-config';
import { totpIssuer } from '@/lib/totp-issuer';
import { ConfirmDialog } from '@/components/ui/confirm-dialog';
import { IconButton } from '@/components/ui/icon-button';
import { useConfirmDialog } from '@/hooks/use-confirm-dialog';
function PasswordChangeSection() {
const t = useTranslations('settings.security');
const tLogin = useTranslations('login');
const { changePassword, isSaving, otpEnabled } = useAccountSecurityStore();
const [currentPassword, setCurrentPassword] = useState('');
const [otpCode, setOtpCode] = useState('');
@@ -78,13 +80,14 @@ function PasswordChangeSection() {
autoComplete="current-password"
className="pe-10"
/>
<button
type="button"
<IconButton
label={showCurrent ? tLogin('hide_password') : tLogin('show_password')}
aria-pressed={showCurrent}
onClick={() => setShowCurrent(!showCurrent)}
className="absolute right-3 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground"
className="absolute end-1 top-1/2 -translate-y-1/2 text-muted-foreground"
>
{showCurrent ? <EyeOff className="w-4 h-4" /> : <Eye className="w-4 h-4" />}
</button>
{showCurrent ? <EyeOff /> : <Eye />}
</IconButton>
</div>
</div>
{otpEnabled && (
@@ -112,13 +115,14 @@ function PasswordChangeSection() {
autoComplete="new-password"
className="pe-10"
/>
<button
type="button"
<IconButton
label={showNew ? tLogin('hide_password') : tLogin('show_password')}
aria-pressed={showNew}
onClick={() => setShowNew(!showNew)}
className="absolute right-3 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground"
className="absolute end-1 top-1/2 -translate-y-1/2 text-muted-foreground"
>
{showNew ? <EyeOff className="w-4 h-4" /> : <Eye className="w-4 h-4" />}
</button>
{showNew ? <EyeOff /> : <Eye />}
</IconButton>
</div>
</div>
<div>
@@ -392,7 +396,7 @@ function parseIpList(raw: string): string[] {
.filter(Boolean);
}
function CredentialRow({ entry, onRemove, isSaving }: { entry: AppPasswordInfo | ApiKeyInfo; onRemove: (id: string) => void; isSaving: boolean }) {
function CredentialRow({ entry, onRemove, isSaving, deleteLabel }: { entry: AppPasswordInfo | ApiKeyInfo; onRemove: (id: string) => void; isSaving: boolean; deleteLabel: string }) {
return (
<div className="flex items-start justify-between py-2 px-3 bg-muted/50 rounded-md gap-2">
<div className="flex flex-col min-w-0 flex-1">
@@ -416,15 +420,14 @@ function CredentialRow({ entry, onRemove, isSaving }: { entry: AppPasswordInfo |
</div>
)}
</div>
<Button
variant="ghost"
size="sm"
<IconButton
label={deleteLabel}
onClick={() => onRemove(entry.id)}
disabled={isSaving}
className="text-destructive hover:text-destructive shrink-0"
>
<Trash2 className="w-3 h-3" />
</Button>
<Trash2 />
</IconButton>
</div>
);
}
@@ -582,7 +585,7 @@ function CredentialSection({ icon: Icon, i18nNamespace, entries, onCreate, onRem
{entries.length > 0 ? (
<div className="space-y-1">
{entries.map((entry) => (
<CredentialRow key={entry.id} entry={entry} onRemove={handleRemove} isSaving={isSaving} />
<CredentialRow key={entry.id} entry={entry} onRemove={handleRemove} isSaving={isSaving} deleteLabel={tCommon('delete')} />
))}
</div>
) : (
@@ -869,30 +872,27 @@ function PublicKeysSection() {
</div>
<div className="flex items-center gap-1 shrink-0">
<Button
variant="ghost"
size="sm"
<IconButton
label={isEncryptedWithThisKey ? t('encryption.disable_tooltip') : t('encryption.enable_tooltip')}
onClick={() => handleToggleEncryption(key.id)}
disabled={isSaving}
title={isEncryptedWithThisKey ? t('encryption.disable_tooltip') : t('encryption.enable_tooltip')}
className={cn(
isEncryptedWithThisKey
? 'text-green-600 hover:text-green-700 dark:text-green-400'
: 'text-muted-foreground hover:text-foreground'
)}
>
{isEncryptedWithThisKey ? <Lock className="w-4 h-4" /> : <Unlock className="w-4 h-4" />}
</Button>
{isEncryptedWithThisKey ? <Lock /> : <Unlock />}
</IconButton>
<Button
variant="ghost"
size="sm"
<IconButton
label={tCommon('delete')}
onClick={() => handleRemove(key.id)}
disabled={isSaving || isEncryptedWithThisKey}
className="text-destructive hover:text-destructive"
>
<Trash2 className="w-3 h-3" />
</Button>
<Trash2 />
</IconButton>
</div>
</div>
);
+2
View File
@@ -672,6 +672,7 @@ export function FilterSettings() {
<ToggleSwitch
checked={rule.enabled}
onChange={() => handleToggle(rule.id)}
ariaLabel={rule.name}
/>
</div>
@@ -755,6 +756,7 @@ export function FilterSettings() {
<ToggleSwitch
checked={expandedFilterView}
onChange={(v) => updateSetting("expandedFilterView", v)}
ariaLabel={t("expanded_view")}
/>
</div>
)}
+17 -28
View File
@@ -28,6 +28,7 @@ import {
type AppIcon,
} from '@/components/icons';
import { Button } from '@/components/ui/button';
import { IconButton } from '@/components/ui/icon-button';
import { Input } from '@/components/ui/input';
import { AppearanceSettings } from '@/components/settings/appearance-settings';
import { AppTopBannerSlot } from '@/components/plugins/app-top-banner-slot';
@@ -868,14 +869,9 @@ export function SettingsApp({ linkSegments: routeSegments }: SettingsAppProps =
<div className={cn("flex flex-col bg-background pt-[env(safe-area-inset-top)]", isEmbedded ? "h-full" : "h-dvh")}>
<AppTopBannerSlot />
<div className="flex items-center gap-2 px-4 h-14 border-b border-border bg-background shrink-0">
<Button
variant="ghost"
size="icon"
onClick={() => window.history.back()}
className="h-10 w-10"
>
<ArrowLeft className="w-5 h-5" />
</Button>
<IconButton label={t('back_to_settings')} size="icon-lg" onClick={() => window.history.back()}>
<ArrowLeft />
</IconButton>
<h1 className="font-semibold text-lg truncate">{activeTabLabel}</h1>
</div>
@@ -901,14 +897,9 @@ export function SettingsApp({ linkSegments: routeSegments }: SettingsAppProps =
<div className={cn("flex flex-col bg-background pt-[env(safe-area-inset-top)]", isEmbedded ? "h-full" : "h-dvh")}>
<AppTopBannerSlot />
<div className="flex items-center gap-2 px-4 h-14 border-b border-border bg-background shrink-0">
<Button
variant="ghost"
size="icon"
onClick={() => router.push('/')}
className="h-10 w-10"
>
<ArrowLeft className="w-5 h-5" />
</Button>
<IconButton label={t('back_to_mail')} size="icon-lg" onClick={() => router.push('/')}>
<ArrowLeft />
</IconButton>
<div className="flex items-center gap-2">
<SettingsIcon className="w-5 h-5 text-muted-foreground" />
<h1 className="font-semibold text-lg">{t('title')}</h1>
@@ -928,14 +919,13 @@ export function SettingsApp({ linkSegments: routeSegments }: SettingsAppProps =
aria-label={t('search_placeholder')}
/>
{searchQuery && (
<button
type="button"
<IconButton
label={t('search_clear')}
onClick={() => setSearchQuery('')}
className="absolute end-2 top-1/2 -translate-y-1/2 p-1 rounded-full hover:bg-muted text-muted-foreground hover:text-foreground transition-colors"
aria-label={t('search_clear')}
className="absolute end-1 top-1/2 -translate-y-1/2 h-7 w-7 rounded-full text-muted-foreground"
>
<X className="w-4 h-4" />
</button>
<X />
</IconButton>
)}
</div>
</div>
@@ -1090,14 +1080,13 @@ export function SettingsApp({ linkSegments: routeSegments }: SettingsAppProps =
aria-label={t('search_placeholder')}
/>
{searchQuery && (
<button
type="button"
<IconButton
label={t('search_clear')}
onClick={() => setSearchQuery('')}
className="absolute end-2 top-1/2 -translate-y-1/2 p-1 rounded-full hover:bg-muted text-muted-foreground hover:text-foreground transition-colors"
aria-label={t('search_clear')}
className="absolute end-1 top-1/2 -translate-y-1/2 h-7 w-7 rounded-full text-muted-foreground"
>
<X className="w-4 h-4" />
</button>
<X />
</IconButton>
)}
</div>
</div>
+9 -10
View File
@@ -5,6 +5,7 @@ import { useTranslations } from "next-intl";
import { Plus, Pencil, Trash2, ExternalLink, PanelRight, GripVertical, Lock } from "@/components/icons";
import { iconForName } from "@/components/icons";
import { Button } from "@/components/ui/button";
import { IconButton } from "@/components/ui/icon-button";
import { Input } from "@/components/ui/input";
import { SettingsSection, SettingItem, ToggleSwitch } from "./settings-section";
import { IconPicker } from "@/components/layout/icon-picker";
@@ -341,18 +342,16 @@ export function SidebarAppsSettings() {
)}>
{app.openMode === "inline" ? tApps("inline_badge") : tApps("tab_badge")}
</span>
<button
onClick={() => setEditingApp(app.id)}
className="p-1.5 rounded-md hover:bg-muted text-muted-foreground hover:text-foreground transition-colors"
>
<Pencil className="w-3.5 h-3.5" />
</button>
<button
<IconButton label={tApps("edit_app")} onClick={() => setEditingApp(app.id)} className="text-muted-foreground">
<Pencil />
</IconButton>
<IconButton
label={tApps("delete")}
onClick={() => handleDelete(app)}
className="p-1.5 rounded-md hover:bg-destructive/10 text-muted-foreground hover:text-destructive transition-colors"
className="text-muted-foreground hover:text-destructive hover:bg-destructive/10"
>
<Trash2 className="w-3.5 h-3.5" />
</button>
<Trash2 />
</IconButton>
</div>
);
})}