From bd9a8cba79d0337acec3023f834c35e8f83ebc7d Mon Sep 17 00:00:00 2001 From: busya Date: Wed, 17 Jun 2026 15:13:17 +0300 Subject: [PATCH] refactor(frontend): migrate remaining confirm() to ConfirmDialog (6 files) Replaced 7 native confirm() calls with styled : - settings/git (2: delete config + delete repo) - settings/automation (delete policy) - TaskHistory (clear tasks) - ApiKeysTab (revoke key) - ConversationList (delete conversation) - Settings page (delete environment) Updated 2 test files to verify ConfirmDialog interactions. --- .../assistant/ConversationList.svelte | 20 +++++++-- .../__tests__/ConversationList.test.ts | 24 +++++----- .../lib/components/settings/ApiKeysTab.svelte | 20 +++++++-- frontend/src/pages/Settings.svelte | 43 +++++++++++++----- .../routes/settings/automation/+page.svelte | 24 +++++++++- frontend/src/routes/settings/git/+page.svelte | 45 +++++++++++++++++-- .../__tests__/git_settings_page.ux.test.ts | 9 ++-- 7 files changed, 147 insertions(+), 38 deletions(-) diff --git a/frontend/src/lib/components/assistant/ConversationList.svelte b/frontend/src/lib/components/assistant/ConversationList.svelte index a4042b6d..c3255b64 100644 --- a/frontend/src/lib/components/assistant/ConversationList.svelte +++ b/frontend/src/lib/components/assistant/ConversationList.svelte @@ -14,6 +14,7 @@ import Input from "$lib/ui/Input.svelte"; import Button from "$lib/ui/Button.svelte"; import Icon from "$lib/ui/Icon.svelte"; + import { ConfirmDialog } from "$lib/ui"; let { conversations = [], @@ -37,6 +38,8 @@ let searchQuery = $state(""); let searchTimer: ReturnType | null = null; + let showDeleteConfirm = $state(false); + let pendingDeleteId: string | null = $state(null); // Group conversations by date on client let groupedConversations = $derived.by(() => { @@ -76,9 +79,8 @@ function handleDelete(e: Event, id: string) { e.stopPropagation(); - if (confirm($t.assistant?.delete_confirm || "Delete this conversation?")) { - ondelete?.(id); - } + pendingDeleteId = id; + showDeleteConfirm = true; } @@ -148,4 +150,16 @@ {/if} + + { if (pendingDeleteId) ondelete?.(pendingDeleteId); }} + onCancel={() => { pendingDeleteId = null; }} +/> + diff --git a/frontend/src/lib/components/assistant/__tests__/ConversationList.test.ts b/frontend/src/lib/components/assistant/__tests__/ConversationList.test.ts index e03121ec..a083980a 100644 --- a/frontend/src/lib/components/assistant/__tests__/ConversationList.test.ts +++ b/frontend/src/lib/components/assistant/__tests__/ConversationList.test.ts @@ -163,12 +163,9 @@ describe("ConversationList — search", () => { // #endregion TestAgentChat.ConversationList.Search // #region TestAgentChat.ConversationList.Delete [C:2] [TYPE Test] -// @UX_STATE loaded — delete button calls ondelete after window.confirm. +// @UX_STATE loaded — delete button opens ConfirmDialog, confirm triggers ondelete. describe("ConversationList — delete action", () => { - it("calls ondelete after window.confirm returns true", () => { - // Set up window.confirm to return true - const originalConfirm = window.confirm; - window.confirm = vi.fn(() => true); + it("calls ondelete after confirming in the dialog", () => { const ondelete = vi.fn(); render(ConversationList, { @@ -186,14 +183,15 @@ describe("ConversationList — delete action", () => { expect(deleteBtns.length).toBeGreaterThan(0); fireEvent.click(deleteBtns[0]); - expect(window.confirm).toHaveBeenCalled(); - expect(ondelete).toHaveBeenCalledWith("c1"); + // ConfirmDialog appears — click the "Delete" button inside it + const confirmBtn = screen.getByText("Delete"); + expect(confirmBtn).toBeTruthy(); + fireEvent.click(confirmBtn); - window.confirm = originalConfirm; + expect(ondelete).toHaveBeenCalledWith("c1"); }); - it("does not call ondelete when window.confirm returns false", () => { - window.confirm = vi.fn(() => false); + it("does not call ondelete when dialog is cancelled", () => { const ondelete = vi.fn(); render(ConversationList, { @@ -209,7 +207,11 @@ describe("ConversationList — delete action", () => { const deleteBtns = screen.getAllByTitle(/delete/i); fireEvent.click(deleteBtns[0]); - expect(window.confirm).toHaveBeenCalled(); + // ConfirmDialog appears — click "Cancel" button + const cancelBtn = screen.getByText("Cancel"); + expect(cancelBtn).toBeTruthy(); + fireEvent.click(cancelBtn); + expect(ondelete).not.toHaveBeenCalled(); }); }); diff --git a/frontend/src/lib/components/settings/ApiKeysTab.svelte b/frontend/src/lib/components/settings/ApiKeysTab.svelte index 5db2deb8..64d5de7f 100644 --- a/frontend/src/lib/components/settings/ApiKeysTab.svelte +++ b/frontend/src/lib/components/settings/ApiKeysTab.svelte @@ -18,6 +18,7 @@ import { t } from '$lib/i18n/index.svelte.js'; import { api } from '$lib/api.js'; import { addToast } from '$lib/toasts.svelte.js'; + import { ConfirmDialog } from '$lib/ui'; let keys = $state([]); let environments = $state([]); @@ -27,6 +28,8 @@ let isGenerating = $state(false); let revokingId = $state(null); let revealedKey = $state(null); + let showRevokeConfirm = $state(false); + let pendingRevokeKey = $state(null); // Generate form state let formName = $state(''); @@ -137,9 +140,8 @@ } function confirmRevoke(key) { - if (confirm('All tools using this key will get 401. Are you sure?')) { - handleRevoke(key.id); - } + pendingRevokeKey = key.id; + showRevokeConfirm = true; } async function copyToClipboard(text) { @@ -394,4 +396,16 @@ {/if} + + { if (pendingRevokeKey) handleRevoke(pendingRevokeKey); }} + onCancel={() => { pendingRevokeKey = null; }} +/> + diff --git a/frontend/src/pages/Settings.svelte b/frontend/src/pages/Settings.svelte index 6c46ab20..9a1c108d 100755 --- a/frontend/src/pages/Settings.svelte +++ b/frontend/src/pages/Settings.svelte @@ -23,6 +23,7 @@ import { getSettings, updateGlobalSettings, addEnvironment, updateEnvironment, deleteEnvironment, testEnvironmentConnection } from '../lib/api'; import { addToast } from '$lib/toasts.svelte.js'; import { t } from '$lib/i18n/index.svelte.js'; + import { ConfirmDialog } from '$lib/ui'; // [/SECTION] let settings = { @@ -53,6 +54,8 @@ }; let editingEnvId = null; + let showDeleteEnv = $state(false); + let pendingDeleteEnvId = $state(null); // #region loadSettings:Function [TYPE Function] // @ingroup Module @@ -129,18 +132,23 @@ * @post Environment is removed from backend and list is reloaded. * @param {string} id - The ID of the environment to delete. */ - async function handleDeleteEnv(id) { - if (confirm($t.settings?.env_delete_confirm)) { - try { - console.log(`[Settings.handleDeleteEnv][Action] Deleting environment: ${id}`); - await deleteEnvironment(id); - addToast($t.settings?.env_deleted, 'success'); - await loadSettings(); - console.log("[Settings.handleDeleteEnv][Coherence:OK] Environment deleted."); - } catch (error) { - console.error("[Settings.handleDeleteEnv][Coherence:Failed] Failed to delete environment:", error); - addToast($t.settings?.env_delete_failed, 'error'); - } + function handleDeleteEnv(id) { + pendingDeleteEnvId = id; + showDeleteEnv = true; + } + + async function confirmDeleteEnv() { + const id = pendingDeleteEnvId; + pendingDeleteEnvId = null; + try { + console.log(`[Settings.handleDeleteEnv][Action] Deleting environment: ${id}`); + await deleteEnvironment(id); + addToast($t.settings?.env_deleted, 'success'); + await loadSettings(); + console.log("[Settings.handleDeleteEnv][Coherence:OK] Environment deleted."); + } catch (error) { + console.error("[Settings.handleDeleteEnv][Coherence:Failed] Failed to delete environment:", error); + addToast($t.settings?.env_delete_failed, 'error'); } } // #endregion handleDeleteEnv:Function @@ -352,4 +360,15 @@ + { pendingDeleteEnvId = null; }} +/> + diff --git a/frontend/src/routes/settings/automation/+page.svelte b/frontend/src/routes/settings/automation/+page.svelte index ababa00f..0fc8e9c1 100644 --- a/frontend/src/routes/settings/automation/+page.svelte +++ b/frontend/src/routes/settings/automation/+page.svelte @@ -12,7 +12,7 @@
@@ -48,7 +64,7 @@ -
@@ -137,7 +153,7 @@
{repo.full_name || repo.name}
{repo.clone_url || repo.html_url}
- + {/each} @@ -147,4 +163,27 @@ + + { if (pendingDeleteConfigId) model.deleteConfig(pendingDeleteConfigId); }} + onCancel={() => { pendingDeleteConfigId = null; }} +/> + + { if (pendingDeleteRepo) model.deleteRemoteRepo(pendingDeleteRepo); }} + onCancel={() => { pendingDeleteRepo = null; }} +/> + diff --git a/frontend/src/routes/settings/git/__tests__/git_settings_page.ux.test.ts b/frontend/src/routes/settings/git/__tests__/git_settings_page.ux.test.ts index 87bc21c5..23a6c621 100644 --- a/frontend/src/routes/settings/git/__tests__/git_settings_page.ux.test.ts +++ b/frontend/src/routes/settings/git/__tests__/git_settings_page.ux.test.ts @@ -79,8 +79,6 @@ describe('GitSettingsPage UX Contracts', () => { beforeEach(() => { vi.clearAllMocks(); - // Default confirm to always accept for tests - global.confirm = vi.fn(() => true); }); // @UX_STATE: Initial Load @@ -164,12 +162,15 @@ describe('GitSettingsPage UX Contracts', () => { render(GitSettingsPage); await waitFor(() => expect(screen.getByText('Dev GitLab')).toBeTruthy()); - // Click delete button + // Click delete button to open ConfirmDialog const deleteButton = screen.getByTitle('Delete'); await fireEvent.click(deleteButton); + // Click the "Delete" button in the ConfirmDialog + await waitFor(() => expect(screen.getByText('Delete')).toBeTruthy()); + await fireEvent.click(screen.getByText('Delete')); + await waitFor(() => { - expect(global.confirm).toHaveBeenCalledWith('Are you sure?'); expect(gitService.deleteConfig).toHaveBeenCalledWith('conf-1'); expect(addToast).toHaveBeenCalledWith('Git configuration deleted', 'success'); expect(screen.queryByText('Dev GitLab')).toBeNull(); // Should be removed from DOM