diff --git a/apps/web/app/settings/plugins/page.test.tsx b/apps/web/app/settings/plugins/page.test.tsx index 5de5f39cfc..58145ab053 100644 --- a/apps/web/app/settings/plugins/page.test.tsx +++ b/apps/web/app/settings/plugins/page.test.tsx @@ -45,8 +45,7 @@ vi.mock("@/lib/api/domains/plugins-api", () => ({ return Promise.resolve({ disabled: true }); }, uninstallPlugin: (...args: [string]) => { - uninstallPluginSpy(...args); - return Promise.resolve({ deleted: true }); + return uninstallPluginSpy(...args); }, installPluginFromUrl: (...args: [string]) => installPluginFromUrlSpy(...args), installPluginUpload: (...args: [File]) => installPluginUploadSpy(...args), @@ -150,6 +149,7 @@ function emptySyncResult(overrides: Partial = {}): SyncResult { } beforeEach(() => { + uninstallPluginSpy.mockResolvedValue({ deleted: true }); installPluginFromUrlSpy.mockResolvedValue({ plugin: installedPlugin() }); installPluginUploadSpy.mockResolvedValue({ plugin: installedPlugin() }); syncPluginsSpy.mockResolvedValue(emptySyncResult()); @@ -235,7 +235,8 @@ describe("PluginsSettingsPage", () => { render(); fireEvent.click(screen.getByRole("button", { name: /uninstall/i })); - fireEvent.click(screen.getByRole("button", { name: /confirm uninstall/i })); + expect(screen.getByTestId("plugin-uninstall-confirm-popover").textContent).toContain("Acme"); + fireEvent.click(screen.getByTestId("plugin-uninstall-confirm")); await vi.waitFor(() => expect(uninstallPluginSpy).toHaveBeenCalledWith(PLUGIN_ID)); expect(unloadPluginSpy).toHaveBeenCalledWith(PLUGIN_ID); @@ -252,6 +253,30 @@ describe("PluginsSettingsPage", () => { expect(uninstallPluginSpy).not.toHaveBeenCalled(); }); + + it("disables auto-update while an uninstall request is pending", async () => { + let resolveUninstall!: (result: { deleted: boolean }) => void; + uninstallPluginSpy.mockReturnValueOnce( + new Promise((resolve) => { + resolveUninstall = resolve; + }), + ); + setStoreState([activePlugin({ auto_update: null })]); + + render(); + fireEvent.click(screen.getByRole("button", { name: /uninstall/i })); + fireEvent.click(screen.getByTestId("plugin-uninstall-confirm")); + + await vi.waitFor(() => expect(uninstallPluginSpy).toHaveBeenCalledWith(PLUGIN_ID)); + const autoUpdate = screen.getByTestId(`plugin-auto-update-${PLUGIN_ID}`); + try { + expect((autoUpdate as HTMLButtonElement).disabled).toBe(true); + } finally { + resolveUninstall({ deleted: true }); + const removePlugin = storeState.removePlugin as ReturnType; + await vi.waitFor(() => expect(removePlugin).toHaveBeenCalledWith(PLUGIN_ID)); + } + }); }); describe("PluginsSettingsPage install dialog", () => { diff --git a/apps/web/components/settings/plugins/plugin-detail.tsx b/apps/web/components/settings/plugins/plugin-detail.tsx index 4c44e30afb..4f06375251 100644 --- a/apps/web/components/settings/plugins/plugin-detail.tsx +++ b/apps/web/components/settings/plugins/plugin-detail.tsx @@ -1,5 +1,6 @@ "use client"; +import { useRef, useState, type RefObject } from "react"; import { IconArrowLeft } from "@tabler/icons-react"; import { Trans, useTranslation } from "react-i18next"; import { Badge } from "@kandev/ui/badge"; @@ -9,6 +10,7 @@ import { Separator } from "@kandev/ui/separator"; import { PluginSlot } from "@/components/plugins/plugin-slot"; import Link from "@/components/routing/app-link"; import { useRouter } from "@/lib/routing/client-router"; +import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; import { usePlugins } from "@/hooks/domains/plugins/use-plugins"; import { SettingsCard } from "@/components/settings/settings-card"; import { useSettingsSaveContributor } from "@/components/settings/settings-save-provider"; @@ -17,7 +19,7 @@ import { PluginManifestCard } from "./plugin-manifest-card"; import { PluginRepoLink } from "./plugin-repo-link"; import { PluginStatusBadge } from "./plugin-status-badge"; import { PluginErrorDiagnostic } from "./plugin-error-diagnostic"; -import { UninstallPluginDialog } from "./uninstall-plugin-dialog"; +import { PluginUninstallConfirmation } from "./uninstall-plugin-dialog"; import { usePluginActions } from "./use-plugin-actions"; import { usePluginConfigForm } from "./use-plugin-config-form"; import type { PluginRecord } from "@/lib/types/plugins"; @@ -35,8 +37,11 @@ const PLUGINS_SETTINGS_HREF = "/settings/plugins"; export function PluginDetail({ pluginId }: { pluginId: string }) { const { items, loaded } = usePlugins(); const router = useRouter(); + const { isFinePointer } = useResponsiveBreakpoint(); const actions = usePluginActions(); const plugin = items.find((p) => p.id === pluginId) ?? null; + const [confirmingUninstall, setConfirmingUninstall] = useState(false); + const uninstallAnchorRef = useRef(null); const form = usePluginConfigForm(plugin); useSettingsSaveContributor({ id: `plugin-config:${pluginId}`, @@ -63,17 +68,35 @@ export function PluginDetail({ pluginId }: { pluginId: string }) { ownerPluginId={plugin.id} slotProps={{ pluginId: plugin.id, status: plugin.status }} /> - + - - { + setConfirmingUninstall(true); + }} + /> + { + setConfirmingUninstall(false); + }} onConfirm={async () => { - await actions.confirmUninstall(); - router.push(PLUGINS_SETTINGS_HREF); + const uninstalled = await actions.confirmUninstall(plugin); + if (uninstalled) router.push(PLUGINS_SETTINGS_HREF); }} /> @@ -185,11 +208,22 @@ function PluginSettingsBody({ plugin, form, busy }: PluginSettingsCardProps) { type PluginDangerZoneProps = { plugin: PluginRecord; actions: ReturnType; + isFinePointer: boolean; + confirmingUninstall: boolean; + uninstallAnchorRef: RefObject; + onUninstall: () => void; }; -function PluginDangerZone({ plugin, actions }: PluginDangerZoneProps) { +function PluginDangerZone({ + plugin, + actions, + isFinePointer, + confirmingUninstall, + uninstallAnchorRef, + onUninstall, +}: PluginDangerZoneProps) { const { t } = useTranslation(); - const busy = actions.busyId === plugin.id; + const busy = actions.busyId === plugin.id || actions.uninstallBusy; const canEnable = plugin.status === "disabled" || plugin.status === "registered" || plugin.status === "error"; const canDisable = plugin.status === "active" || plugin.status === "error"; @@ -218,15 +252,18 @@ function PluginDangerZone({ plugin, actions }: PluginDangerZoneProps) { {t("plugins:disable")} )} - + {(isFinePointer || !confirmingUninstall) && ( + + )} ); } diff --git a/apps/web/components/settings/plugins/plugin-row.test.tsx b/apps/web/components/settings/plugins/plugin-row.test.tsx index 87eb8f7e96..13d161d91c 100644 --- a/apps/web/components/settings/plugins/plugin-row.test.tsx +++ b/apps/web/components/settings/plugins/plugin-row.test.tsx @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { PluginRow } from "./plugin-row"; import type { MarketplaceEntry, PluginRecord } from "@/lib/types/plugins"; @@ -55,7 +55,6 @@ const baseProps = { autoUpdateBusy: false, onEnable: noop, onDisable: noop, - onUninstall: noop, onSetAutoUpdate: noop, }; @@ -217,3 +216,51 @@ describe("PluginRow auto-update toggle", () => { expect(onSetAutoUpdate).toHaveBeenCalledWith(p, null); }); }); + +describe("PluginRow uninstall confirmation", () => { + const pluginName = "Acme Tools"; + + it("anchors fine-pointer confirmation to the row action and names the target", () => { + const p = plugin({ display_name: pluginName }); + const onConfirmUninstall = vi.fn(); + + render( + , + ); + + fireEvent.click(screen.getByRole("button", { name: /^Uninstall$/i })); + + expect(screen.getByTestId("plugin-uninstall-confirm-popover").textContent).toContain( + pluginName, + ); + expect(document.querySelector('[data-slot="dialog-overlay"]')).toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: /^Cancel$/i })); + expect(screen.queryByTestId("plugin-uninstall-confirm-popover")).toBeNull(); + }); + + it("uses inline touch actions on coarse pointers and passes the row target on confirm", async () => { + const p = plugin({ display_name: pluginName }); + const onConfirmUninstall = vi.fn(); + + render( + , + ); + + fireEvent.click(screen.getByRole("button", { name: /^Uninstall$/i })); + + const confirmation = screen.getByTestId("plugin-uninstall-inline-confirmation"); + expect(confirmation.textContent).toContain(pluginName); + expect(screen.queryByRole("button", { name: /^Uninstall$/i })).toBeNull(); + expect(screen.getByTestId("plugin-uninstall-confirm").className).toContain("h-11"); + expect(screen.getByTestId("plugin-uninstall-confirm").className).toContain("min-w-11"); + + fireEvent.click(screen.getByTestId("plugin-uninstall-confirm")); + await waitFor(() => expect(onConfirmUninstall).toHaveBeenCalledWith(p)); + }); +}); diff --git a/apps/web/components/settings/plugins/plugin-row.tsx b/apps/web/components/settings/plugins/plugin-row.tsx index d12b9bb5cf..f1b69aa5d8 100644 --- a/apps/web/components/settings/plugins/plugin-row.tsx +++ b/apps/web/components/settings/plugins/plugin-row.tsx @@ -1,5 +1,6 @@ "use client"; +import { useRef, useState, type RefObject } from "react"; import { IconArrowUpCircle, IconChevronRight } from "@tabler/icons-react"; import { useTranslation } from "react-i18next"; import { Badge } from "@kandev/ui/badge"; @@ -9,6 +10,7 @@ import Link from "@/components/routing/app-link"; import { PluginRepoLink } from "./plugin-repo-link"; import { PluginStatusBadge } from "./plugin-status-badge"; import { PluginErrorDiagnostic } from "./plugin-error-diagnostic"; +import { PluginUninstallConfirmation } from "./uninstall-plugin-dialog"; import type { MarketplaceEntry, PluginRecord } from "@/lib/types/plugins"; import { SETTINGS_TYPOGRAPHY } from "@/components/settings/settings-typography"; @@ -23,9 +25,13 @@ type PluginRowProps = { autoUpdateBusy: boolean; /** True when the plugin declares required settings the operator has not filled in. */ needsSetup?: boolean; + /** Fine pointers use an anchored popover; coarse pointers use inline row actions. */ + isFinePointer?: boolean; + /** True while this plugin's uninstall request is in flight. */ + uninstallBusy?: boolean; onEnable: (plugin: PluginRecord) => void; onDisable: (plugin: PluginRecord) => void; - onUninstall: (plugin: PluginRecord) => void; + onConfirmUninstall?: (plugin: PluginRecord) => void | Promise; onUpdate?: (entry: MarketplaceEntry) => void; onSetAutoUpdate: (plugin: PluginRecord, value: boolean | null) => void; }; @@ -49,9 +55,11 @@ export function PluginRow({ autoUpdateDefault, autoUpdateBusy, needsSetup = false, + isFinePointer = true, + uninstallBusy = false, onEnable, onDisable, - onUninstall, + onConfirmUninstall, onUpdate, onSetAutoUpdate, }: PluginRowProps) { @@ -59,6 +67,15 @@ export function PluginRow({ const canEnable = plugin.status === "disabled" || plugin.status === "registered" || plugin.status === "error"; const canDisable = plugin.status === "active" || plugin.status === "error"; + const mutationBusy = busy || autoUpdateBusy || uninstallBusy; + const { + confirmingUninstall, + setConfirmingUninstall, + uninstallAnchorRef, + requestUninstall, + cancelUninstall, + confirmUninstall, + } = usePluginUninstallConfirmation(plugin, onConfirmUninstall); return (
+
+ +
); } +function usePluginUninstallConfirmation( + plugin: PluginRecord, + onConfirmUninstall?: (plugin: PluginRecord) => void | Promise, +) { + const [confirmingUninstall, setConfirmingUninstall] = useState(false); + const uninstallAnchorRef = useRef(null); + + const requestUninstall = () => { + setConfirmingUninstall(true); + }; + + const cancelUninstall = () => { + setConfirmingUninstall(false); + }; + + const confirmUninstall = () => { + setConfirmingUninstall(false); + return onConfirmUninstall?.(plugin); + }; + + return { + confirmingUninstall, + setConfirmingUninstall, + uninstallAnchorRef, + requestUninstall, + cancelUninstall, + confirmUninstall, + }; +} + /** * The row's name, state badges and identifiers. Everything here is inert and * sits under the card's overlay link, except the repo link, which raises @@ -237,9 +298,14 @@ type PluginRowActionsProps = Omit< | "autoUpdateBusy" | "needsSetup" | "onSetAutoUpdate" + | "onConfirmUninstall" + | "uninstallBusy" > & { canEnable: boolean; canDisable: boolean; + isFinePointer: boolean; + confirmingUninstall: boolean; + uninstallAnchorRef: RefObject; onEnable: (plugin: PluginRecord) => void; onDisable: (plugin: PluginRecord) => void; onUninstall: (plugin: PluginRecord) => void; @@ -251,6 +317,9 @@ function PluginRowActions({ update, canEnable, canDisable, + isFinePointer, + confirmingUninstall, + uninstallAnchorRef, onEnable, onDisable, onUninstall, @@ -294,15 +363,18 @@ function PluginRowActions({ {t("plugins:disable")} )} - + {(isFinePointer || !confirmingUninstall) && ( + + )} ); } diff --git a/apps/web/components/settings/plugins/plugins-settings.tsx b/apps/web/components/settings/plugins/plugins-settings.tsx index 21e8420ed3..60ba7f9c15 100644 --- a/apps/web/components/settings/plugins/plugins-settings.tsx +++ b/apps/web/components/settings/plugins/plugins-settings.tsx @@ -7,6 +7,7 @@ import { Button } from "@kandev/ui/button"; import { Switch } from "@kandev/ui/switch"; import { Tabs, TabsContent, TabsList, TabsTrigger } from "@kandev/ui/tabs"; import { SettingsPageTemplate } from "@/components/settings/settings-page-template"; +import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; import { useAutoUpdateSettings } from "@/hooks/domains/plugins/use-auto-update-settings"; import { usePlugins } from "@/hooks/domains/plugins/use-plugins"; import { usePluginSetupStatus } from "@/hooks/domains/plugins/use-plugin-setup-status"; @@ -15,7 +16,6 @@ import type { MarketplaceEntry } from "@/lib/types/plugins"; import { InstallPluginDialog } from "./install-plugin-dialog"; import { MarketplaceBrowser } from "./marketplace-browser"; import { PluginRow } from "./plugin-row"; -import { UninstallPluginDialog } from "./uninstall-plugin-dialog"; import { usePluginActions } from "./use-plugin-actions"; import { settingsActionClassName } from "@/components/settings/settings-control"; @@ -26,6 +26,7 @@ import { settingsActionClassName } from "@/components/settings/settings-control" */ export function PluginsSettings() { const { t } = useTranslation(); + const { isFinePointer } = useResponsiveBreakpoint(); const list = usePlugins(); const actions = usePluginActions(); const autoUpdate = useAutoUpdateSettings(); @@ -75,6 +76,7 @@ export function PluginsSettings() { autoUpdate={autoUpdate} updates={updates} updatingId={updatingId} + isFinePointer={isFinePointer} onUpdate={handleUpdate} /> @@ -84,12 +86,6 @@ export function PluginsSettings() { - ; updates: Map; updatingId: string | null; + isFinePointer: boolean; onUpdate: (entry: MarketplaceEntry) => void; }; @@ -118,6 +115,7 @@ function InstalledTab({ autoUpdate, updates, updatingId, + isFinePointer, onUpdate, }: InstalledTabProps) { const { t } = useTranslation(); @@ -167,6 +165,7 @@ function InstalledTab({ autoUpdateDefault={autoUpdate.autoUpdateDefault} updates={updates} updatingId={updatingId} + isFinePointer={isFinePointer} onUpdate={onUpdate} /> @@ -213,6 +212,7 @@ type PluginListProps = { autoUpdateDefault: boolean; updates: Map; updatingId: string | null; + isFinePointer: boolean; onUpdate: (entry: MarketplaceEntry) => void; }; @@ -222,6 +222,7 @@ function PluginList({ autoUpdateDefault, updates, updatingId, + isFinePointer, onUpdate, }: PluginListProps) { const { t } = useTranslation(); @@ -263,9 +264,13 @@ function PluginList({ autoUpdateDefault={autoUpdateDefault} autoUpdateBusy={actions.autoUpdateBusyId === plugin.id} needsSetup={needsSetup.has(plugin.id)} + isFinePointer={isFinePointer} + uninstallBusy={actions.uninstallBusy} onEnable={actions.handleEnable} onDisable={actions.handleDisable} - onUninstall={actions.openUninstall} + onConfirmUninstall={async (target) => { + await actions.confirmUninstall(target); + }} onUpdate={onUpdate} onSetAutoUpdate={actions.handleSetAutoUpdate} /> diff --git a/apps/web/components/settings/plugins/uninstall-plugin-dialog.test.tsx b/apps/web/components/settings/plugins/uninstall-plugin-dialog.test.tsx new file mode 100644 index 0000000000..83a70c610d --- /dev/null +++ b/apps/web/components/settings/plugins/uninstall-plugin-dialog.test.tsx @@ -0,0 +1,70 @@ +import { useRef, type ComponentProps } from "react"; +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { PluginUninstallConfirmation } from "./uninstall-plugin-dialog"; +import type { PluginRecord } from "@/lib/types/plugins"; + +afterEach(cleanup); + +function plugin(): PluginRecord { + return { + id: "acme-tools", + api_version: 1, + version: "1.0.0", + display_name: "Acme Tools", + description: "", + author: "acme", + categories: [], + capabilities: {}, + status: "active", + install_path: "/plugins/acme-tools", + signed: true, + installed_at: "2026-01-01T00:00:00Z", + restart_count: 0, + }; +} + +function Confirmation({ isFinePointer }: { isFinePointer: boolean }) { + const anchorRef = useRef(null); + const props = { + target: plugin(), + open: true, + isFinePointer, + anchorRef, + onOpenChange: vi.fn(), + onCancel: vi.fn(), + onConfirm: vi.fn(), + } satisfies ComponentProps; + + return ( + <> + + + + ); +} + +describe("plugin uninstall confirmation", () => { + it("uses an anchored non-modal confirmation on fine pointers", () => { + render(); + + expect(screen.getByTestId("plugin-uninstall-confirm-popover")).toBeTruthy(); + expect(document.querySelector('[data-slot="dialog-overlay"]')).toBeNull(); + expect(screen.getByRole("dialog").textContent).toContain("Acme Tools"); + expect(screen.getByTestId("plugin-uninstall-confirm").getAttribute("aria-label")).toBe( + "Confirm uninstall", + ); + }); + + it("keeps phone confirmation inline with touch-sized actions", () => { + render(); + + expect(screen.getByTestId("plugin-uninstall-inline-confirmation")).toBeTruthy(); + expect(document.querySelector('[data-slot="dialog-overlay"]')).toBeNull(); + expect(screen.getByTestId("plugin-uninstall-confirm").className).toContain("h-11"); + expect(screen.getByTestId("plugin-uninstall-confirm").className).toContain("min-w-11"); + }); +}); diff --git a/apps/web/components/settings/plugins/uninstall-plugin-dialog.tsx b/apps/web/components/settings/plugins/uninstall-plugin-dialog.tsx index 72304aa019..3ac36eabe9 100644 --- a/apps/web/components/settings/plugins/uninstall-plugin-dialog.tsx +++ b/apps/web/components/settings/plugins/uninstall-plugin-dialog.tsx @@ -1,69 +1,78 @@ "use client"; +import type { RefObject } from "react"; import { Trans, useTranslation } from "react-i18next"; -import { Button } from "@kandev/ui/button"; -import { - Dialog, - DialogContent, - DialogDescription, - DialogFooter, - DialogHeader, - DialogTitle, -} from "@kandev/ui/dialog"; + +import { ActionConfirmPopover } from "@/components/confirmation/action-confirm-popover"; +import { InlineConfirmActions } from "@/components/confirmation/inline-confirm-actions"; import type { PluginRecord } from "@/lib/types/plugins"; -type UninstallPluginDialogProps = { - target: PluginRecord | null; - busy: boolean; - onClose: () => void; - onConfirm: () => void; +type UninstallPluginConfirmationProps = { + target: PluginRecord; + open: boolean; + isFinePointer: boolean; + anchorRef: RefObject; + onOpenChange: (open: boolean) => void; + onCancel: () => void; + onConfirm: () => void | Promise; }; -export function UninstallPluginDialog({ +/** + * Keeps plugin uninstall confirmation at its initiating control. The action + * hook still owns the target mutation, pending state, and failure feedback. + */ +export function PluginUninstallConfirmation({ target, - busy, - onClose, + open, + isFinePointer, + anchorRef, + onOpenChange, + onCancel, onConfirm, -}: UninstallPluginDialogProps) { +}: UninstallPluginConfirmationProps) { const { t } = useTranslation(); + const description = ( + + This will permanently remove{" "} + {target.display_name} and revoke its API + key. This action cannot be undone. + + ); + const confirmAriaLabel = t("plugins:confirmUninstall"); + + if (!isFinePointer) { + if (!open) return null; + return ( + onOpenChange(false)} + onConfirm={onConfirm} + /> + ); + } + return ( - { - if (!open) onClose(); - }} - > - - - {t("plugins:uninstallPlugin")} - - {/* The display name comes from the plugin's manifest, so it is - third-party data rather than our copy. */} - - This will permanently remove{" "} - {target?.display_name} and revoke - its API key. This action cannot be undone. - - - - - - - - - + ); } diff --git a/apps/web/components/settings/plugins/use-plugin-actions.test.tsx b/apps/web/components/settings/plugins/use-plugin-actions.test.tsx index 92db560d1b..852e87f8d7 100644 --- a/apps/web/components/settings/plugins/use-plugin-actions.test.tsx +++ b/apps/web/components/settings/plugins/use-plugin-actions.test.tsx @@ -11,6 +11,12 @@ const installPluginFromUrl = vi.fn<() => Promise>(); const installPluginUpload = vi.fn<() => Promise>(); const enablePlugin = vi.fn(async () => ({ enabled: true })); const getPlugin = vi.fn<() => Promise>(); +const uninstallPlugin = vi.fn<() => Promise<{ deleted: boolean }>>(); +const { toastError } = vi.hoisted(() => ({ toastError: vi.fn() })); + +vi.mock("@/lib/toast/sonner", () => ({ + toast: { error: (...args: unknown[]) => toastError(...args) }, +})); vi.mock("@/lib/plugins/host", () => ({ loadPlugins: (...args: unknown[]) => loadPlugins(...(args as [])), @@ -27,6 +33,7 @@ vi.mock("@/lib/api/domains/plugins-api", async () => { installPluginUpload: (...args: unknown[]) => installPluginUpload(...(args as [])), enablePlugin: (...args: unknown[]) => enablePlugin(...(args as [])), getPlugin: (...args: unknown[]) => getPlugin(...(args as [])), + uninstallPlugin: (...args: unknown[]) => uninstallPlugin(...(args as [])), }; }); @@ -63,6 +70,8 @@ beforeEach(() => { installPluginUpload.mockReset(); enablePlugin.mockClear(); getPlugin.mockReset(); + uninstallPlugin.mockReset(); + toastError.mockReset(); }); describe("usePluginActions — install/update", () => { @@ -161,3 +170,52 @@ describe("usePluginActions — enable", () => { expect(getPlugin).toHaveBeenCalledWith(plugin.id, { cache: "no-store" }); }); }); + +describe("usePluginActions — uninstall", () => { + it("confirms an explicitly supplied target and keeps busy state until API cleanup settles", async () => { + const plugin = activeRecord(); + let resolveUninstall!: (result: { deleted: boolean }) => void; + uninstallPlugin.mockReturnValueOnce( + new Promise<{ deleted: boolean }>((resolve) => { + resolveUninstall = resolve; + }), + ); + + const { result } = renderHook(() => usePluginActions(), { wrapper }); + let confirmation!: Promise; + act(() => { + confirmation = result.current.confirmUninstall(plugin); + }); + + await waitFor(() => expect(result.current.uninstallBusy).toBe(true)); + expect(uninstallPlugin).toHaveBeenCalledWith(plugin.id); + expect(unloadPlugin).not.toHaveBeenCalled(); + + resolveUninstall({ deleted: true }); + await act(async () => { + await expect(confirmation).resolves.toBe(true); + }); + + expect(unloadPlugin).toHaveBeenCalledWith(plugin.id); + await waitFor(() => expect(result.current.uninstallBusy).toBe(false)); + }); + + it("keeps localized failure feedback and releases busy state when uninstall fails", async () => { + const plugin = activeRecord(); + uninstallPlugin.mockRejectedValueOnce(new Error("API key revoke failed")); + + const { result } = renderHook(() => usePluginActions(), { wrapper }); + let confirmation!: Promise; + act(() => { + confirmation = result.current.confirmUninstall(plugin); + }); + + await act(async () => { + await expect(confirmation).resolves.toBe(false); + }); + + expect(toastError).toHaveBeenCalledWith("API key revoke failed"); + expect(unloadPlugin).not.toHaveBeenCalled(); + expect(result.current.uninstallBusy).toBe(false); + }); +}); diff --git a/apps/web/components/settings/plugins/use-plugin-actions.ts b/apps/web/components/settings/plugins/use-plugin-actions.ts index 78779e72a7..1ba40b7eb3 100644 --- a/apps/web/components/settings/plugins/use-plugin-actions.ts +++ b/apps/web/components/settings/plugins/use-plugin-actions.ts @@ -153,34 +153,29 @@ function useAutoUpdateAction(upsertPlugin: (p: PluginRecord) => void) { } function useUninstallAction(removePlugin: (id: string) => void) { - const [uninstallTarget, setUninstallTarget] = useState(null); const [uninstallBusy, setUninstallBusy] = useState(false); - const confirmUninstall = async () => { - if (!uninstallTarget) return; - const target = uninstallTarget; + const confirmUninstall = async (target: PluginRecord) => { setUninstallBusy(true); try { await uninstallPlugin(target.id); unloadPlugin(target.id); removePlugin(target.id); - setUninstallTarget(null); + return true; } catch (err) { toast.error( err instanceof Error ? err.message : t("plugins:failedToUninstall", { name: target.display_name }), ); + return false; } finally { setUninstallBusy(false); } }; return { - uninstallTarget, uninstallBusy, - openUninstall: setUninstallTarget, - closeUninstall: () => setUninstallTarget(null), confirmUninstall, }; } diff --git a/apps/web/e2e/tests/plugins/mobile-plugin-settings-row.spec.ts b/apps/web/e2e/tests/plugins/mobile-plugin-settings-row.spec.ts index 3a42dd78e0..464df75954 100644 --- a/apps/web/e2e/tests/plugins/mobile-plugin-settings-row.spec.ts +++ b/apps/web/e2e/tests/plugins/mobile-plugin-settings-row.spec.ts @@ -39,6 +39,21 @@ test("mobile plugin row: whole card opens settings, controls still act", async ( expect(linkBox.width).toBeGreaterThanOrEqual(rowBox.width - 2); expect(linkBox.height).toBeGreaterThanOrEqual(rowBox.height - 2); + // Uninstall keeps its confirmation inside the row on a coarse pointer. + // Cancel first so the same fixture can cover the existing disable action + // and the detail-surface confirmation below. + const rowUninstall = pluginRow.getByRole("button", { name: "Uninstall" }); + expect((await rowUninstall.boundingBox())?.height).toBeGreaterThanOrEqual(44); + await rowUninstall.tap(); + const rowConfirmation = pluginRow.getByTestId("plugin-uninstall-inline-confirmation"); + await expect(rowConfirmation).toBeVisible(); + await expect(testPage.locator('[data-slot="dialog-overlay"]')).toHaveCount(0); + expect( + (await rowConfirmation.getByTestId("plugin-uninstall-confirm").boundingBox())?.height, + ).toBeGreaterThanOrEqual(44); + await rowConfirmation.getByRole("button", { name: "Cancel" }).tap(); + await expect(rowConfirmation).toHaveCount(0); + // Every control sits above the overlay. If one slipped below it the tap would // navigate instead of acting, and on a phone there is no hover state to // reveal which is which. @@ -52,5 +67,21 @@ test("mobile plugin row: whole card opens settings, controls still act", async ( // change; keep clear of the row's own controls. await pluginRow.tap({ position: { x: 12, y: 12 } }); await expect(testPage).toHaveURL(new RegExp(`/settings/plugins/${PLUGIN_ID}$`)); - await expect(testPage.getByTestId(`plugin-detail-${PLUGIN_ID}`)).toBeVisible(); + const detail = testPage.getByTestId(`plugin-detail-${PLUGIN_ID}`); + await expect(detail).toBeVisible(); + + // The detail danger zone uses the same phone-native inline confirmation and + // keeps both actions at the 44px touch target minimum. + const detailUninstall = detail.getByRole("button", { name: "Uninstall" }); + expect((await detailUninstall.boundingBox())?.height).toBeGreaterThanOrEqual(44); + await detailUninstall.tap(); + const detailConfirmation = testPage.getByTestId("plugin-uninstall-inline-confirmation"); + await expect(detailConfirmation).toBeVisible(); + await expect(testPage.locator('[data-slot="dialog-overlay"]')).toHaveCount(0); + expect( + (await detailConfirmation.getByTestId("plugin-uninstall-confirm").boundingBox())?.height, + ).toBeGreaterThanOrEqual(44); + await detailConfirmation.getByTestId("plugin-uninstall-confirm").tap(); + await expect(testPage).toHaveURL(/\/settings\/plugins$/); + await expect(testPage.getByTestId(`plugin-row-${PLUGIN_ID}`)).toHaveCount(0); }); diff --git a/apps/web/e2e/tests/plugins/plugins.spec.ts b/apps/web/e2e/tests/plugins/plugins.spec.ts index 8560eb34e0..06b7e8009c 100644 --- a/apps/web/e2e/tests/plugins/plugins.spec.ts +++ b/apps/web/e2e/tests/plugins/plugins.spec.ts @@ -325,7 +325,9 @@ test.describe("Plugins — gRPC plugin install/load/live-update/uninstall", () = // --- 7. Uninstall via UI (with confirmation): row disappears, package // directory tree is removed from disk. --- await pluginRow.getByRole("button", { name: "Uninstall" }).click(); - await testPage.getByRole("button", { name: "Confirm uninstall" }).click(); + await expect(testPage.getByTestId("plugin-uninstall-confirm-popover")).toContainText("E2E"); + await expect(testPage.locator('[data-slot="dialog-overlay"]')).toHaveCount(0); + await testPage.getByTestId("plugin-uninstall-confirm").click(); await expect(pluginRow).toHaveCount(0, { timeout: 10_000 }); const pluginDir = path.join(pluginsDir, PLUGIN_ID); @@ -383,6 +385,37 @@ test.describe("Plugins — gRPC plugin install/load/live-update/uninstall", () = await expect(globalToggle).toHaveAttribute("aria-checked", "false"); }); + test("row and detail uninstall confirmations stay local to their initiating controls", async ({ + testPage, + }) => { + test.setTimeout(60_000); + + await openInstallDialog(testPage); + await uploadPackage(testPage, PACKAGE_PATH); + const pluginRow = testPage.getByTestId(`plugin-row-${PLUGIN_ID}`); + await expect(pluginRow).toBeVisible({ timeout: 15_000 }); + + await pluginRow.getByRole("button", { name: "Uninstall" }).click(); + const rowConfirmation = testPage.getByTestId("plugin-uninstall-confirm-popover"); + await expect(rowConfirmation).toContainText("Kandev E2E Fixture Plugin"); + await expect(testPage.locator('[data-slot="dialog-overlay"]')).toHaveCount(0); + await rowConfirmation.getByRole("button", { name: "Cancel" }).click(); + await expect(rowConfirmation).toHaveCount(0); + + await pluginRow.click({ position: { x: 600, y: 12 } }); + await expect(testPage).toHaveURL(new RegExp(`/settings/plugins/${PLUGIN_ID}$`)); + const detail = testPage.getByTestId(`plugin-detail-${PLUGIN_ID}`); + await expect(detail).toBeVisible(); + await detail.getByRole("button", { name: "Uninstall" }).click(); + + const detailConfirmation = testPage.getByTestId("plugin-uninstall-confirm-popover"); + await expect(detailConfirmation).toContainText("Kandev E2E Fixture Plugin"); + await expect(testPage.locator('[data-slot="dialog-overlay"]')).toHaveCount(0); + await testPage.getByTestId("plugin-uninstall-confirm").click(); + await expect(testPage).toHaveURL(/\/settings\/plugins$/); + await expect(testPage.getByTestId(`plugin-row-${PLUGIN_ID}`)).toHaveCount(0); + }); + test("shows boot failure diagnostics and retries an errored plugin", async ({ testPage, apiClient, @@ -735,6 +768,18 @@ test.describe("Plugins — gRPC plugin install/load/live-update/uninstall", () = { timeout: 15_000, intervals: [250, 500, 1000] }, ) .toBe("s3cret-roundtrip"); + + // --- Uninstall from the detail danger zone: the confirmation stays at + // the initiating control and only successful cleanup returns to the list. --- + await testPage.goto(`/settings/plugins/${PLUGIN_ID}`); + const detail = testPage.getByTestId(`plugin-detail-${PLUGIN_ID}`); + await expect(detail).toBeVisible(); + await detail.getByRole("button", { name: "Uninstall" }).click(); + await expect(testPage.getByTestId("plugin-uninstall-confirm-popover")).toContainText("E2E"); + await expect(testPage.locator('[data-slot="dialog-overlay"]')).toHaveCount(0); + await testPage.getByTestId("plugin-uninstall-confirm").click(); + await expect(testPage).toHaveURL(/\/settings\/plugins$/); + await expect(testPage.getByTestId(`plugin-row-${PLUGIN_ID}`)).toHaveCount(0); }); test("uploading a corrupted package surfaces install-plugin-error", async ({ testPage }) => {