From d61d50167776d45d4ab0a99b9abbaf1b8c0d3ac5 Mon Sep 17 00:00:00 2001
From: jules <cloudtech@juleslive.net>
Date: Tue, 14 Jul 2026 14:58:57 +1000
Subject: [PATCH] ui: confirming an alert dialog should close it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

AlertDialogAction was a plain <Button> — only AlertDialogCancel wrapped the
primitive's Close — so confirming ran the action but never dismissed the
dialog. Found in Trippy, where "Clear conversation" cleared the chat and left
the modal sitting on screen; the same component is in every Crema app.

Delete-style dialogs hide it by accident: the row they act on unmounts out from
under the open dialog and takes it with it. Any confirm that leaves its target
on screen (a rewrite, a regenerate, a clear) just gets stuck.

Fixed at the component, so every call site gets it. The action still runs —
Base UI merges its own close handler with the caller's onClick — and async work
carries on behind the closed dialog.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 app/components/ui/alert-dialog.test.tsx | 67 +++++++++++++++++++++++++
 app/components/ui/alert-dialog.tsx      | 14 +++++-
 2 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 app/components/ui/alert-dialog.test.tsx

diff --git a/app/components/ui/alert-dialog.test.tsx b/app/components/ui/alert-dialog.test.tsx
new file mode 100644
index 0000000..204e709
--- /dev/null
+++ b/app/components/ui/alert-dialog.test.tsx
@@ -0,0 +1,67 @@
+// Confirming an AlertDialog must close it AND run the action.
+//
+// AlertDialogAction used to be a plain <Button>, so the dialog just sat there
+// after you confirmed ("Clear conversation" cleared the chat and left the modal
+// on screen). Other call sites only looked right because the row they acted on
+// unmounted underneath the open dialog. Both halves are asserted here so a
+// future refactor can't quietly drop one.
+
+import { act } from "react"
+import { render, screen } from "@testing-library/react"
+import { afterEach, describe, expect, it, vi } from "vitest"
+
+import {
+  AlertDialog,
+  AlertDialogAction,
+  AlertDialogCancel,
+  AlertDialogContent,
+  AlertDialogFooter,
+  AlertDialogHeader,
+  AlertDialogTitle,
+} from "./alert-dialog"
+
+function Harness({ onConfirm }: { onConfirm: () => void }) {
+  return (
+    <AlertDialog defaultOpen>
+      <AlertDialogContent>
+        <AlertDialogHeader>
+          <AlertDialogTitle>Clear this conversation?</AlertDialogTitle>
+        </AlertDialogHeader>
+        <AlertDialogFooter>
+          <AlertDialogCancel>Keep it</AlertDialogCancel>
+          <AlertDialogAction onClick={onConfirm}>Clear</AlertDialogAction>
+        </AlertDialogFooter>
+      </AlertDialogContent>
+    </AlertDialog>
+  )
+}
+
+afterEach(() => vi.restoreAllMocks())
+
+describe("AlertDialogAction", () => {
+  it("runs the action and closes the dialog", async () => {
+    const onConfirm = vi.fn()
+    render(<Harness onConfirm={onConfirm} />)
+
+    expect(screen.getByText("Clear this conversation?")).toBeTruthy()
+
+    await act(async () => {
+      screen.getByRole("button", { name: "Clear" }).click()
+    })
+
+    expect(onConfirm).toHaveBeenCalledTimes(1)
+    expect(screen.queryByText("Clear this conversation?")).toBeNull()
+  })
+
+  it("cancelling closes without running the action", async () => {
+    const onConfirm = vi.fn()
+    render(<Harness onConfirm={onConfirm} />)
+
+    await act(async () => {
+      screen.getByRole("button", { name: "Keep it" }).click()
+    })
+
+    expect(onConfirm).not.toHaveBeenCalled()
+    expect(screen.queryByText("Clear this conversation?")).toBeNull()
+  })
+})
diff --git a/app/components/ui/alert-dialog.tsx b/app/components/ui/alert-dialog.tsx
index 600d38d..19ea595 100644
--- a/app/components/ui/alert-dialog.tsx
+++ b/app/components/ui/alert-dialog.tsx
@@ -141,14 +141,24 @@ function AlertDialogDescription({
   )
 }
 
+// Confirming closes the dialog, exactly as cancelling does. This was a plain
+// <Button>, so the dialog stayed open after you confirmed — every call site
+// already assumed otherwise, and the ones that looked fine only did because
+// the thing they acted on (a row, a card) unmounted underneath them. The
+// handler still runs: Base UI merges its close handler with the caller's
+// onClick, so async work carries on behind the closed dialog.
 function AlertDialogAction({
   className,
+  variant = "default",
+  size = "default",
   ...props
-}: React.ComponentProps<typeof Button>) {
+}: AlertDialogPrimitive.Close.Props &
+  Pick<React.ComponentProps<typeof Button>, "variant" | "size">) {
   return (
-    <Button
+    <AlertDialogPrimitive.Close
       data-slot="alert-dialog-action"
       className={cn(className)}
+      render={<Button variant={variant} size={size} />}
       {...props}
     />
   )
-- 
2.48.1