ui: confirming an alert dialog should close it
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>
This commit is contained in:
67
app/components/ui/alert-dialog.test.tsx
Normal file
67
app/components/ui/alert-dialog.test.tsx
Normal file
@@ -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()
|
||||
})
|
||||
})
|
||||
@@ -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}
|
||||
/>
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user