diff --git a/client/src/components/Admin/CategoryManager.test.tsx b/client/src/components/Admin/CategoryManager.test.tsx index 945ea698d..4160fd1fb 100644 --- a/client/src/components/Admin/CategoryManager.test.tsx +++ b/client/src/components/Admin/CategoryManager.test.tsx @@ -19,6 +19,13 @@ beforeEach(() => { seedStore(useAuthStore, { user: buildUser({ role: 'admin' }), isAuthenticated: true }); }); +/** Answers the confirm dialog: its Delete is the last one in the document (portal). */ +async function confirmDelete(user: ReturnType) { + await screen.findByText('Delete category? Places in this category will not be deleted.'); + const deletes = screen.getAllByRole('button', { name: 'Delete' }); + await user.click(deletes[deletes.length - 1]); +} + describe('CategoryManager', () => { it('FE-COMP-CAT-001: renders without crashing', () => { render(); @@ -116,15 +123,15 @@ describe('CategoryManager', () => { return HttpResponse.json({ success: true }); }) ); - vi.spyOn(window, 'confirm').mockReturnValue(true); render(<>); await screen.findByText('Parks'); - // Delete button is icon-only (Trash2, no title) — find the second action button - const buttons = screen.getAllByRole('button'); - const actionBtns = buttons.filter(b => !b.textContent?.includes('New Category')); - await user.click(actionBtns[1]); + await user.click(screen.getByRole('button', { name: 'Delete' })); + // Nothing goes before the confirm dialog is answered + expect(await screen.findByText('Delete category? Places in this category will not be deleted.')).toBeInTheDocument(); + expect(deleteCalled).toBe(false); + await confirmDelete(user); await waitFor(() => expect(deleteCalled).toBe(true)); - vi.restoreAllMocks(); + await waitFor(() => expect(screen.queryByText('Parks')).not.toBeInTheDocument()); }); it('FE-COMP-CAT-010: shows subtitle text', async () => { @@ -209,23 +216,23 @@ describe('CategoryManager', () => { expect(screen.getByDisplayValue('Parks')).toBeInTheDocument(); }); - it('FE-COMP-CAT-016: declining the delete confirm keeps the category', async () => { + it('FE-COMP-CAT-016: cancelling the delete confirm keeps the category', async () => { const user = userEvent.setup(); let deleteCalled = false; server.use( http.get('/api/categories', () => HttpResponse.json({ categories: [buildCategory({ id: 9, name: 'Parks' })] })), http.delete('/api/categories/9', () => { deleteCalled = true; return HttpResponse.json({ success: true }); }), ); - vi.spyOn(window, 'confirm').mockReturnValue(false); render(); await screen.findByText('Parks'); - const actionBtns = screen.getAllByRole('button').filter(b => !b.textContent?.includes('New Category')); - await user.click(actionBtns[1]); + await user.click(screen.getByRole('button', { name: 'Delete' })); + await screen.findByText('Delete category? Places in this category will not be deleted.'); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + expect(screen.queryByText('Delete category? Places in this category will not be deleted.')).not.toBeInTheDocument(); expect(deleteCalled).toBe(false); expect(screen.getByText('Parks')).toBeInTheDocument(); - vi.restoreAllMocks(); }); it('FE-COMP-CAT-017: a failing delete toasts and keeps the row', async () => { @@ -234,16 +241,14 @@ describe('CategoryManager', () => { http.get('/api/categories', () => HttpResponse.json({ categories: [buildCategory({ id: 9, name: 'Parks' })] })), http.delete('/api/categories/9', () => HttpResponse.json({ error: 'category in use' }, { status: 409 })), ); - vi.spyOn(window, 'confirm').mockReturnValue(true); render(<>); await screen.findByText('Parks'); - const actionBtns = screen.getAllByRole('button').filter(b => !b.textContent?.includes('New Category')); - await user.click(actionBtns[1]); + await user.click(screen.getByRole('button', { name: 'Delete' })); + await confirmDelete(user); expect(await screen.findByText('category in use')).toBeInTheDocument(); expect(screen.getByText('Parks')).toBeInTheDocument(); - vi.restoreAllMocks(); }); it('FE-COMP-CAT-018: picking an icon and a preset colour updates the live preview', async () => { @@ -293,7 +298,7 @@ describe('CategoryManager', () => { await user.click(screen.getByText('New Category')); expect(screen.getByPlaceholderText('Category name')).toHaveValue(''); - const row = screen.getByText('Hotels').closest('.p-3') as HTMLElement; + const row = screen.getByText('Hotels').closest('[data-category-row]') as HTMLElement; await user.click(within(row).getAllByRole('button')[0]); // Only the inline edit form remains, pre-filled with the row's name diff --git a/client/src/components/Admin/CategoryManager.tsx b/client/src/components/Admin/CategoryManager.tsx index 0ec10e9db..9966aef92 100644 --- a/client/src/components/Admin/CategoryManager.tsx +++ b/client/src/components/Admin/CategoryManager.tsx @@ -4,6 +4,7 @@ import { useToast } from '../shared/Toast' import { Plus, Pencil, Trash2, Pipette, Tags, Loader2 } from 'lucide-react' import { DialogButton, DialogFooter, DialogHeader, DialogSection, DialogShell, DialogTile, FooterSpacer, NEUTRAL_TINT, fs } from '../shared/DialogShell' import { Tooltip } from '../shared/Tooltip' +import ConfirmDialog from '../shared/ConfirmDialog' import { SettingRows, SettingsCard, SettingsHint, StatusPill, SETTINGS_BUTTON_PRIMARY, SETTINGS_ICON_BUTTON } from '../Settings/settingsKit' import { CATEGORY_ICON_MAP, ICON_LABELS, getCategoryIcon } from '../shared/categoryIcons' import { useTranslation } from '../../i18n' @@ -27,6 +28,8 @@ export default function CategoryManager() { const [form, setForm] = useState({ name: '', color: DEFAULT_COLOR, icon: 'MapPin' }) const [isSaving, setIsSaving] = useState(false) const [isLoading, setIsLoading] = useState(true) + // The category whose delete waits for the answer in the confirm dialog. + const [deleteId, setDeleteId] = useState(null) const colorInputRef = useRef(null) const toast = useToast() const { t } = useTranslation() @@ -87,7 +90,6 @@ export default function CategoryManager() { } const handleDelete = async (id) => { - if (!confirm(t('categories.confirm.delete'))) return try { await categoriesApi.delete(id) setCategories(prev => prev.filter(c => c.id !== id)) @@ -236,7 +238,7 @@ export default function CategoryManager() { {categories.map(cat => { const Icon = getCategoryIcon(cat.icon) return ( -
+
@@ -255,7 +257,7 @@ export default function CategoryManager() { - @@ -266,6 +268,14 @@ export default function CategoryManager() { )} {formOpen && categoryDialog} + setDeleteId(null)} + onConfirm={() => { if (deleteId !== null) void handleDelete(deleteId) }} + message={t('categories.confirm.delete')} + confirmLabel={t('common.delete')} + danger + /> ) } diff --git a/client/src/components/Layout/PageSidebar.test.tsx b/client/src/components/Layout/PageSidebar.test.tsx index e67f341a2..a7eac95dc 100644 --- a/client/src/components/Layout/PageSidebar.test.tsx +++ b/client/src/components/Layout/PageSidebar.test.tsx @@ -23,7 +23,8 @@ function setup(overrides: Partial> = {}) { describe('PageSidebar', () => { it('FE-W4PSB-001: renders the label, every tab and the panel children', () => { - setup() + // Ungrouped tabs: the label heads the list. Grouped ones lead with their group instead. + setup({ tabs: TABS.map(({ group: _group, ...tab }) => tab) }) expect(screen.getAllByText('SETTINGS').length).toBeGreaterThan(0) expect(screen.getByRole('button', { name: /^Appearance$/ })).toBeInTheDocument() @@ -36,6 +37,7 @@ describe('PageSidebar', () => { expect(screen.getByText('General')).toBeInTheDocument() expect(screen.getByText('Account')).toBeInTheDocument() + expect(screen.queryByText('SETTINGS')).not.toBeInTheDocument() }) it('FE-W4PSB-003: highlights the active tab', () => { diff --git a/client/src/components/OAuth/ScopeGroupPicker.tsx b/client/src/components/OAuth/ScopeGroupPicker.tsx index c9c20d259..4b0738d3c 100644 --- a/client/src/components/OAuth/ScopeGroupPicker.tsx +++ b/client/src/components/OAuth/ScopeGroupPicker.tsx @@ -1,13 +1,35 @@ import React, { useState } from 'react' -import { ChevronDown, ChevronRight } from 'lucide-react' +import { Check, ChevronDown, ChevronRight, Minus } from 'lucide-react' import { getScopesByGroup } from '../../api/oauthScopes' import { useTranslation } from '../../i18n' +import { fs } from '../shared/DialogShell' +import { SettingRows, SETTINGS_BUTTON } from '../Settings/settingsKit' interface Props { selected: string[] onChange: (scopes: string[]) => void } +/** The square of a checkbox in the accent look: ticked, partly ticked (mixed) or empty. */ +function CheckBox({ checked, mixed = false }: { checked: boolean; mixed?: boolean }) { + const on = checked || mixed + return ( + + ) +} + +/** + * The scopes an OAuth client may ask for, grouped like the consent page: one row + * per group with a fold-out list of its scopes. Every tick is a button with + * role="checkbox", so it reads as one and answers to Space and Enter. + */ export default function ScopeGroupPicker({ selected, onChange }: Props): React.ReactElement { const { t } = useTranslation() const [open, setOpen] = useState>({}) @@ -17,77 +39,85 @@ export default function ScopeGroupPicker({ selected, onChange }: Props): React.R const allSelected = allScopeKeys.every(s => selected.includes(s)) return ( -
-
+
+
-
- {Object.entries(scopesByGroup).map(([group, groupScopes]) => { - const groupScopeKeys = groupScopes.map(s => s.scope) - const allGroupSelected = groupScopeKeys.every(s => selected.includes(s)) - const someGroupSelected = groupScopeKeys.some(s => selected.includes(s)) - return ( -
-
- - { if (el) el.indeterminate = someGroupSelected && !allGroupSelected }} - onChange={e => onChange( - e.target.checked - ? [...new Set([...selected, ...groupScopeKeys])] - : selected.filter(s => !groupScopeKeys.includes(s)) - )} - className="rounded" - title={allGroupSelected ? `Deselect all ${group}` : `Select all ${group}`} - /> -
- {open[group] && ( -
- {groupScopes.map(({ scope, label, description }) => ( - - ))} +
+ + {Object.entries(scopesByGroup).map(([group, groupScopes]) => { + const groupScopeKeys = groupScopes.map(s => s.scope) + const allGroupSelected = groupScopeKeys.every(s => selected.includes(s)) + const someGroupSelected = groupScopeKeys.some(s => selected.includes(s)) + const mixed = someGroupSelected && !allGroupSelected + return ( +
+
+ +
- )} -
- ) - })} + {open[group] && ( +
+ {groupScopes.map(({ scope, label, description }) => { + const on = selected.includes(scope) + return ( + + ) + })} +
+ )} +
+ ) + })} +
) diff --git a/client/src/components/Settings/AboutTab.test.tsx b/client/src/components/Settings/AboutTab.test.tsx index 688dca381..d6b27b9c6 100644 --- a/client/src/components/Settings/AboutTab.test.tsx +++ b/client/src/components/Settings/AboutTab.test.tsx @@ -80,69 +80,54 @@ describe('AboutTab', () => { expect(screen.queryByText('v2.9.10')).toBeNull(); }); - it('FE-COMP-ABOUT-012: Ko-fi link hover changes border and box-shadow styles', () => { + // The hover look lives in Tailwind classes driven by the tile's `--tile` + // colour now, so the tests pin the colour each tile hands its classes. + const tile = (link: HTMLAnchorElement) => ({ + color: link.style.getPropertyValue('--tile'), + hovers: link.className.includes('hover:border-[color:var(--tile)]'), + }); + + it('FE-COMP-ABOUT-012: the Ko-fi tile carries its brand colour for the hover border', () => { render(); const link = screen.getByText('Ko-fi').closest('a') as HTMLAnchorElement; - fireEvent.mouseEnter(link); - expect(link.style.borderColor).toBe('rgb(255, 94, 91)'); - expect(link.style.boxShadow).not.toBe(''); - fireEvent.mouseLeave(link); - expect(link.style.borderColor).toBe('var(--border-primary)'); - expect(link.style.boxShadow).toBe('none'); + expect(link).toHaveAttribute('href', 'https://ko-fi.com/mauriceboe'); + expect(tile(link)).toEqual({ color: '#ff5e5b', hovers: true }); }); - it('FE-COMP-ABOUT-013: Buy Me a Coffee link hover changes border and box-shadow styles', () => { + it('FE-COMP-ABOUT-013: the Buy Me a Coffee tile carries its brand colour for the hover border', () => { render(); const link = screen.getByText('Buy Me a Coffee').closest('a') as HTMLAnchorElement; - fireEvent.mouseEnter(link); - expect(link.style.borderColor).toBe('rgb(255, 221, 0)'); - expect(link.style.boxShadow).not.toBe(''); - fireEvent.mouseLeave(link); - expect(link.style.borderColor).toBe('var(--border-primary)'); - expect(link.style.boxShadow).toBe('none'); + expect(link).toHaveAttribute('href', 'https://buymeacoffee.com/mauriceboe'); + expect(tile(link)).toEqual({ color: '#ffdd00', hovers: true }); }); - it('FE-COMP-ABOUT-014: Discord link hover changes border and box-shadow styles', () => { + it('FE-COMP-ABOUT-014: the Discord tile carries its brand colour for the hover border', () => { render(); const link = screen.getByText('Discord').closest('a') as HTMLAnchorElement; - fireEvent.mouseEnter(link); - expect(link.style.borderColor).toBe('rgb(88, 101, 242)'); - expect(link.style.boxShadow).not.toBe(''); - fireEvent.mouseLeave(link); - expect(link.style.borderColor).toBe('var(--border-primary)'); - expect(link.style.boxShadow).toBe('none'); + expect(link).toHaveAttribute('href', 'https://discord.gg/NhZBDSd4qW'); + expect(tile(link)).toEqual({ color: '#5865F2', hovers: true }); }); - it('FE-COMP-ABOUT-015: Bug report link hover changes border and box-shadow styles', () => { + it('FE-COMP-ABOUT-015: the bug report tile uses the danger token', () => { render(); const link = document.querySelector('a[href*="issues/new"]') as HTMLAnchorElement; - fireEvent.mouseEnter(link); - expect(link.style.borderColor).toBe('rgb(239, 68, 68)'); - expect(link.style.boxShadow).not.toBe(''); - fireEvent.mouseLeave(link); - expect(link.style.borderColor).toBe('var(--border-primary)'); - expect(link.style.boxShadow).toBe('none'); + expect(link).toHaveAttribute('target', '_blank'); + expect(tile(link)).toEqual({ color: 'var(--danger)', hovers: true }); }); - it('FE-COMP-ABOUT-016: Feature request link hover changes border and box-shadow styles', () => { + it('FE-COMP-ABOUT-016: the feature request tile uses the warning token', () => { render(); const link = document.querySelector('a[href*="discussions/new"]') as HTMLAnchorElement; - fireEvent.mouseEnter(link); - expect(link.style.borderColor).toBe('rgb(245, 158, 11)'); - expect(link.style.boxShadow).not.toBe(''); - fireEvent.mouseLeave(link); - expect(link.style.borderColor).toBe('var(--border-primary)'); - expect(link.style.boxShadow).toBe('none'); + expect(link).toHaveAttribute('target', '_blank'); + expect(tile(link)).toEqual({ color: 'var(--warning)', hovers: true }); }); - it('FE-COMP-ABOUT-017: Wiki link hover changes border and box-shadow styles', () => { + it('FE-COMP-ABOUT-017: the wiki tile uses the info token and no inline hover styles are left', () => { render(); const link = document.querySelector('a[href*="wiki"]') as HTMLAnchorElement; + expect(tile(link)).toEqual({ color: 'var(--info)', hovers: true }); fireEvent.mouseEnter(link); - expect(link.style.borderColor).toBe('rgb(99, 102, 241)'); - expect(link.style.boxShadow).not.toBe(''); - fireEvent.mouseLeave(link); - expect(link.style.borderColor).toBe('var(--border-primary)'); - expect(link.style.boxShadow).toBe('none'); + expect(link.style.borderColor).toBe(''); + expect(link.style.boxShadow).toBe(''); }); }); diff --git a/client/src/components/Settings/AboutTab.tsx b/client/src/components/Settings/AboutTab.tsx index 3fdd02bd0..83d071ee9 100644 --- a/client/src/components/Settings/AboutTab.tsx +++ b/client/src/components/Settings/AboutTab.tsx @@ -14,49 +14,46 @@ const DISCORD_PATH = 'M20.317 4.37a19.791 19.791 0 0 0-4.885-1.515.074.074 0 0 0 // The services' own colours: each tile takes its brand on hover and in its icon square. const BRAND = { - kofi: '#ff5e5b', // theme-lint-disable: Ko-fi brand colour - bmac: '#ffdd00', // theme-lint-disable: Buy Me a Coffee brand colour - discord: '#5865F2', // theme-lint-disable: Discord brand colour - bug: '#ef4444', // theme-lint-disable: the bug tile's signal red, a fixed brand-like mark - idea: '#f59e0b', // theme-lint-disable: the idea tile's signal amber, a fixed brand-like mark - wiki: '#6366f1', // theme-lint-disable: the wiki tile's signal indigo, a fixed brand-like mark + kofi: '#ff5e5b', // theme-lint-disable: brand colour (Ko-fi) + bmac: '#ffdd00', // theme-lint-disable: brand colour (Buy Me a Coffee) + discord: '#5865F2', // theme-lint-disable: brand colour (Discord) } as const; +// TREK's own tiles speak in the theme's signal colours instead. +const SIGNAL = { + bug: 'var(--danger)', + idea: 'var(--warning)', + wiki: 'var(--info)', +} as const; + +/** Tile chrome that reads the tile's colour from `--tile`: the border and a faint ring on hover. */ +const TILE_HOVER = 'hover:border-[color:var(--tile)] hover:shadow-[0_0_0_1px_color-mix(in_srgb,var(--tile)_14%,transparent)]'; +/** The icon square in the tile's colour. */ +const TILE_ICON = 'bg-[color-mix(in_srgb,var(--tile)_12%,transparent)] text-[color:var(--tile)]'; + /** * One outbound link as a tile: a tinted icon square, a title and a hint, the - * external-link mark on the right. The border picks up the brand colour while + * external-link mark on the right. The border picks up the tile's colour while * the pointer is on it. */ -function LinkTile({ href, brand, icon, title, hint }: { +function LinkTile({ href, color, icon, title, hint }: { href: string; - brand?: string; + /** A brand colour or a theme token, handed to the classes as `--tile`. */ + color?: string; icon?: ReactNode; title: ReactNode; hint: ReactNode; }) { - const tint: CSSProperties | undefined = brand - ? { background: `color-mix(in srgb, ${brand} 12%, transparent)`, color: brand } - : undefined; return ( { - e.currentTarget.style.borderColor = brand; - e.currentTarget.style.boxShadow = `0 0 0 1px ${brand}22`; - } : undefined} - onMouseLeave={brand ? (e) => { - e.currentTarget.style.borderColor = 'var(--border-primary)'; - e.currentTarget.style.boxShadow = 'none'; - } : undefined} + className={`flex min-w-0 items-center gap-3 overflow-hidden rounded-[14px] border border-edge bg-surface-card px-3.5 py-3 no-underline shadow-sm transition-[border-color,box-shadow] duration-200 ease-[cubic-bezier(0.23,1,0.32,1)] ${color ? TILE_HOVER : ''}`} + style={color ? ({ '--tile': color } as CSSProperties) : undefined} > {icon && ( - + {icon} )} @@ -112,21 +109,21 @@ export default function AboutTab({ appVersion }: Props): React.ReactElement {
} title="Ko-fi" hint={t('admin.github.support')} /> } title="Buy Me a Coffee" hint={t('admin.github.support')} />
) } diff --git a/client/src/components/Settings/PluginSettingsTab.test.tsx b/client/src/components/Settings/PluginSettingsTab.test.tsx index 274a21a44..9ff8a496c 100644 --- a/client/src/components/Settings/PluginSettingsTab.test.tsx +++ b/client/src/components/Settings/PluginSettingsTab.test.tsx @@ -326,9 +326,8 @@ describe('PluginSettingsTab', () => { expect(button).toBeEnabled(); }); - it('FE-COMP-PLUGINSETTINGS-018: a dangerous action asks first and a declined confirm skips it', async () => { + it('FE-COMP-PLUGINSETTINGS-018: a dangerous action asks first and a cancelled confirm skips it', async () => { const user = userEvent.setup(); - const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(false); let ran = 0; serve('weather', { actions: [{ key: 'wipe', label: 'Wipe cache', danger: true, scope: 'user' }] }); server.use(http.post('/api/plugin-settings/weather/actions/wipe', () => { @@ -339,14 +338,18 @@ describe('PluginSettingsTab', () => { render(); await user.click(await screen.findByRole('button', { name: 'Wipe cache' })); - expect(confirmSpy).toHaveBeenCalledWith('Run this action?'); + expect(await screen.findByText('Run this action?')).toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + expect(screen.queryByText('Run this action?')).not.toBeInTheDocument(); expect(ran).toBe(0); - confirmSpy.mockReturnValue(true); await user.click(screen.getByRole('button', { name: 'Wipe cache' })); + await screen.findByText('Run this action?'); + // The dialog's confirm button carries the action's label and sits last in the document. + const confirmButtons = screen.getAllByRole('button', { name: 'Wipe cache' }); + await user.click(confirmButtons[confirmButtons.length - 1]!); await screen.findByText('Wiped'); expect(ran).toBe(1); - confirmSpy.mockRestore(); }); it('FE-COMP-PLUGINSETTINGS-019: an unconfigured OAuth integration renders no connect row', async () => { diff --git a/client/src/components/Settings/PluginSettingsTab.tsx b/client/src/components/Settings/PluginSettingsTab.tsx index 236dcb752..9907d4acb 100644 --- a/client/src/components/Settings/PluginSettingsTab.tsx +++ b/client/src/components/Settings/PluginSettingsTab.tsx @@ -2,6 +2,7 @@ import { Fragment, useEffect, useState, type ReactNode } from 'react' import { Save, Loader2, Link2, Unlink, CheckCircle, Puzzle, Zap } from 'lucide-react' import { resolvePluginIcon } from '../shared/PluginIcon' import CustomSelect from '../shared/CustomSelect' +import ConfirmDialog from '../shared/ConfirmDialog' import { fs } from '../shared/DialogShell' import { EditorField, INPUT } from '../shared/dialogParts' import { SettingRow, SettingRows, SettingsCard, SettingsHint, SETTINGS_BUTTON, SETTINGS_BUTTON_DANGER, SETTINGS_BUTTON_PRIMARY } from './settingsKit' @@ -107,6 +108,8 @@ function PluginSettingsForm({ id, name, icon }: { id: string; name: string; icon const [actions, setActions] = useState([]) const [running, setRunning] = useState(null) const [actionResult, setActionResult] = useState>({}) + // A dangerous action waits here for the user's answer in the confirm dialog. + const [pendingAction, setPendingAction] = useState(null) useEffect(() => { let alive = true @@ -128,8 +131,12 @@ function PluginSettingsForm({ id, name, icon }: { id: string; name: string; icon // An action runs AS the caller, so it sees the values they just saved — run the save // first if the form is dirty would be nicer, but keeping it explicit is less surprising. - const runAction = async (a: PluginAction) => { - if (a.danger && !window.confirm(t('settings.plugins.actions.confirm'))) return + const runAction = (a: PluginAction) => { + if (a.danger) { setPendingAction(a); return } + void performAction(a) + } + + const performAction = async (a: PluginAction) => { setRunning(a.key) try { const res = await pluginsApi.runAction(id, a.key) @@ -259,6 +266,14 @@ function PluginSettingsForm({ id, name, icon }: { id: string; name: string; icon
)} + setPendingAction(null)} + onConfirm={() => { if (pendingAction) void performAction(pendingAction) }} + message={t('settings.plugins.actions.confirm')} + confirmLabel={pendingAction?.label} + danger + /> ) } diff --git a/client/src/pages/AdminPage.test.tsx b/client/src/pages/AdminPage.test.tsx index a3099dea0..5a1368d38 100644 --- a/client/src/pages/AdminPage.test.tsx +++ b/client/src/pages/AdminPage.test.tsx @@ -440,6 +440,9 @@ describe('AdminPage', () => { // MSW returns [admin, alice]; alice's delete button is index 1 const deleteButtons = screen.getAllByLabelText(/delete/i); fireEvent.click(deleteButtons[1]); + // The desktop tab asks in its own dialog; its confirm button renders last. + const confirmButtons = await screen.findAllByRole('button', { name: /^delete$/i }); + fireEvent.click(confirmButtons[confirmButtons.length - 1]); await waitFor(() => { expect(screen.queryByText('alice')).not.toBeInTheDocument(); @@ -1298,6 +1301,8 @@ describe('AdminPage', () => { // Click delete for alice (second user — non-self) const deleteButtons = screen.getAllByLabelText('Delete user'); fireEvent.click(deleteButtons[deleteButtons.length - 1]); // last button = alice + const confirmButtons = await screen.findAllByRole('button', { name: /^delete$/i }); + fireEvent.click(confirmButtons[confirmButtons.length - 1]); await waitFor(() => { expect(deletedId).toBeDefined(); diff --git a/client/src/pages/admin/AdminUserModals.test.tsx b/client/src/pages/admin/AdminUserModals.test.tsx index 6744bff13..a4c47c173 100644 --- a/client/src/pages/admin/AdminUserModals.test.tsx +++ b/client/src/pages/admin/AdminUserModals.test.tsx @@ -52,6 +52,13 @@ function EditFormHarness() { return ; } +/** Answers the confirm dialog: its button repeats the label and sits last in the document. */ +function confirmPasskeyReset() { + expect(screen.getByText('Remove all passkeys for alice?')).toBeInTheDocument(); + const buttons = screen.getAllByRole('button', { name: /reset passkeys/i }); + fireEvent.click(buttons[buttons.length - 1]); +} + const editing = { editingUser: alice, editForm: { username: 'alice', email: 'alice@example.com', role: 'user', password: '' }, @@ -183,18 +190,31 @@ describe('AdminUserModals', () => { expect(admin.handleSaveUser).toHaveBeenCalledTimes(1); }); - it('FE-ADMMOD-012: resetting passkeys is skipped when the confirm is declined', () => { - vi.spyOn(window, 'confirm').mockReturnValue(false); + it('FE-ADMMOD-012: resetting passkeys is skipped when the confirm is cancelled', async () => { + let calls = 0; + server.use( + http.delete('/api/admin/users/:id/passkeys', () => { + calls += 1; + return HttpResponse.json({ deleted: 1 }); + }) + ); const admin = renderModals(editing); fireEvent.click(screen.getByRole('button', { name: /reset passkeys/i })); + expect(screen.getByText('Remove all passkeys for alice?')).toBeInTheDocument(); - expect(window.confirm).toHaveBeenCalledWith('Remove all passkeys for alice?'); + // The edit dialog has its own Cancel; the question's Cancel is the last one. + const cancels = screen.getAllByRole('button', { name: /^cancel$/i }); + fireEvent.click(cancels[cancels.length - 1]); + + expect(screen.queryByText('Remove all passkeys for alice?')).not.toBeInTheDocument(); + await new Promise((r) => setTimeout(r, 20)); + expect(calls).toBe(0); expect(admin.toast.success).not.toHaveBeenCalled(); + expect(admin.setEditingUser).not.toHaveBeenCalled(); }); it('FE-ADMMOD-013: a confirmed passkey reset reports how many were removed', async () => { - vi.spyOn(window, 'confirm').mockReturnValue(true); let deletedFor: string | undefined; server.use( http.delete('/api/admin/users/:id/passkeys', ({ params }) => { @@ -205,17 +225,18 @@ describe('AdminUserModals', () => { const admin = renderModals(editing); fireEvent.click(screen.getByRole('button', { name: /reset passkeys/i })); + confirmPasskeyReset(); await waitFor(() => expect(admin.toast.success).toHaveBeenCalledWith('Removed 3 passkey(s)')); expect(deletedFor).toBe('2'); }); it('FE-ADMMOD-014: a failing passkey reset toasts the generic error', async () => { - vi.spyOn(window, 'confirm').mockReturnValue(true); server.use(http.delete('/api/admin/users/:id/passkeys', () => HttpResponse.json({}, { status: 500 }))); const admin = renderModals(editing); fireEvent.click(screen.getByRole('button', { name: /reset passkeys/i })); + confirmPasskeyReset(); await waitFor(() => expect(admin.toast.error).toHaveBeenCalledWith('Error')); }); diff --git a/client/src/pages/admin/AdminUserModals.tsx b/client/src/pages/admin/AdminUserModals.tsx index 98d8e4bea..226594354 100644 --- a/client/src/pages/admin/AdminUserModals.tsx +++ b/client/src/pages/admin/AdminUserModals.tsx @@ -13,6 +13,7 @@ import { import React, { useId } from 'react'; import { adminApi } from '../../api/client'; import CustomSelect from '../../components/shared/CustomSelect'; +import ConfirmDialog from '../../components/shared/ConfirmDialog'; import { Tooltip } from '../../components/shared/Tooltip'; import { DialogButton, @@ -115,6 +116,8 @@ export default function AdminUserModals({ admin, t }: AdminUserModalsProps): Rea } = admin; const [showCreatePw, setShowCreatePw] = React.useState(false); const [showEditPw, setShowEditPw] = React.useState(false); + // The user whose passkeys wait for the admin's answer in the confirm dialog. + const [passkeyResetUser, setPasskeyResetUser] = React.useState(null); const uid = useId(); const createId = `${uid}-create`; const editId = `${uid}-edit`; @@ -280,15 +283,8 @@ export default function AdminUserModals({ admin, t }: AdminUserModalsProps): Rea

+ + setPasskeyResetUser(null)} + onConfirm={async () => { + if (!passkeyResetUser) return; + try { + const r = await adminApi.resetUserPasskeys(passkeyResetUser.id); + toast.success(t('admin.passkey.resetDone', { count: r.deleted ?? 0 })); + } catch { + toast.error(t('common.error')); + } + }} + title={t('admin.passkey.reset')} + message={passkeyResetUser ? t('admin.passkey.resetConfirm', { name: passkeyResetUser.username }) : ''} + confirmLabel={t('admin.passkey.reset')} + danger + /> ); } diff --git a/client/src/pages/admin/AdminUsersTab.test.tsx b/client/src/pages/admin/AdminUsersTab.test.tsx index 0ee65e485..3e870a96e 100644 --- a/client/src/pages/admin/AdminUsersTab.test.tsx +++ b/client/src/pages/admin/AdminUsersTab.test.tsx @@ -125,13 +125,27 @@ describe('AdminUsersTab', () => { expect(admin.handleEditUser).toHaveBeenCalledWith(alice); }); - it('FE-ADMUSR-009: delete button forwards the row user to handleDeleteUser', () => { + it('FE-ADMUSR-009: delete asks first and forwards the row user to handleDeleteUser on confirm', () => { const admin = renderTab({ users: [me, alice], currentUser: me }); const deleteButtons = screen.getAllByLabelText('Delete user'); fireEvent.click(deleteButtons[1]); - expect(admin.handleDeleteUser).toHaveBeenCalledWith(alice); + expect(admin.handleDeleteUser).not.toHaveBeenCalled(); + expect(screen.getByText(`Delete user "${alice.username}"? All trips will be permanently deleted.`)).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Delete' })); + + expect(admin.handleDeleteUser).toHaveBeenCalledWith(alice, { confirmed: true }); + }); + + it('FE-ADMUSR-009b: cancelling the delete question deletes nothing', () => { + const admin = renderTab({ users: [me, alice], currentUser: me }); + + fireEvent.click(screen.getAllByLabelText('Delete user')[1]); + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(screen.queryByText(`Delete user "${alice.username}"? All trips will be permanently deleted.`)).not.toBeInTheDocument(); + expect(admin.handleDeleteUser).not.toHaveBeenCalled(); }); it('FE-ADMUSR-010: the delete button of the current user is disabled', () => { diff --git a/client/src/pages/admin/AdminUsersTab.tsx b/client/src/pages/admin/AdminUsersTab.tsx index 599f46eb0..9456a2711 100644 --- a/client/src/pages/admin/AdminUsersTab.tsx +++ b/client/src/pages/admin/AdminUsersTab.tsx @@ -1,8 +1,9 @@ -import React, { useId, type ReactNode } from 'react' +import React, { useId, useState, type ReactNode } from 'react' import CustomSelect from '../../components/shared/CustomSelect' import { Shield, Trash2, Edit2, UserPlus, Link2, Copy, Plus, Users } from 'lucide-react' import PermissionsPanel from '../../components/Admin/PermissionsPanel' import { Tooltip } from '../../components/shared/Tooltip' +import ConfirmDialog from '../../components/shared/ConfirmDialog' import { DialogButton, DialogFooter, DialogHeader, DialogShell, DialogTile, FooterSpacer, NEUTRAL_TINT, fs } from '../../components/shared/DialogShell' import { EditorField, Segmented } from '../../components/shared/dialogParts' import { SettingsCard, SettingRows, SettingsHint, StatusPill, SETTINGS_BUTTON, SETTINGS_BUTTON_PRIMARY, SETTINGS_ICON_BUTTON } from '../../components/Settings/settingsKit' @@ -42,6 +43,8 @@ export default function AdminUsersTab({ admin, t, locale }: AdminUsersTabProps): handleEditUser, handleDeleteUser, } = admin const inviteLabelId = useId() + // The row whose delete waits for the admin's answer in the confirm dialog. + const [userToDelete, setUserToDelete] = useState<(typeof users)[number] | null>(null) return ( <> @@ -117,7 +120,7 @@ export default function AdminUsersTab({ admin, t, locale }: AdminUsersTabProps): handleEditUser(u)}> - handleDeleteUser(u)} disabled={isMe} danger> + setUserToDelete(u)} disabled={isMe} danger>
@@ -253,6 +256,16 @@ export default function AdminUsersTab({ admin, t, locale }: AdminUsersTabProps): )} + + setUserToDelete(null)} + onConfirm={() => { if (userToDelete) void handleDeleteUser(userToDelete, { confirmed: true }) }} + title={t('admin.deleteUserTitle')} + message={userToDelete ? t('admin.deleteUser', { name: userToDelete.username }) : ''} + confirmLabel={t('common.delete')} + danger + /> ) } diff --git a/client/src/pages/admin/useAdmin.test.tsx b/client/src/pages/admin/useAdmin.test.tsx index 8a00e4603..43c29b24d 100644 --- a/client/src/pages/admin/useAdmin.test.tsx +++ b/client/src/pages/admin/useAdmin.test.tsx @@ -800,6 +800,19 @@ describe('useAdmin', () => { expect(toastCalls).toContainEqual({ type: 'success', message: 'User deleted' }); }); + it('FE-ADMHOOK-042b: a delete the caller already confirmed skips the browser confirm', async () => { + const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(false); + server.use(http.delete('/api/admin/users/:id', () => HttpResponse.json({ success: true }))); + const { result } = await mountAdmin(); + + await act(async () => { + await result.current.handleDeleteUser(result.current.users[1], { confirmed: true }); + }); + + expect(confirmSpy).not.toHaveBeenCalled(); + expect(result.current.users.map(u => u.username)).toEqual(['admin']); + }); + it('FE-ADMHOOK-043: a failing delete surfaces the server error', async () => { vi.spyOn(window, 'confirm').mockReturnValue(true); server.use(http.delete('/api/admin/users/:id', () => HttpResponse.json({ error: 'busy' }, { status: 500 }))); diff --git a/client/src/pages/admin/useAdmin.ts b/client/src/pages/admin/useAdmin.ts index ffb51b7be..34d793ff1 100644 --- a/client/src/pages/admin/useAdmin.ts +++ b/client/src/pages/admin/useAdmin.ts @@ -492,12 +492,14 @@ export function useAdmin() { } } - const handleDeleteUser = async (user) => { + // `confirmed` means the caller already asked in its own ConfirmDialog (the + // desktop users tab); without it the browser's confirm asks, as on mobile. + const handleDeleteUser = async (user, opts: { confirmed?: boolean } = {}) => { if (user.id === currentUser?.id) { toast.error(t('admin.toast.cannotDeleteSelf')) return } - if (!confirm(t('admin.deleteUser', { name: user.username }))) return + if (!opts.confirmed && !confirm(t('admin.deleteUser', { name: user.username }))) return try { await adminApi.deleteUser(user.id) setUsers(prev => prev.filter(u => u.id !== user.id))