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({
|
function AlertDialogAction({
|
||||||
className,
|
className,
|
||||||
|
variant = "default",
|
||||||
|
size = "default",
|
||||||
...props
|
...props
|
||||||
}: React.ComponentProps<typeof Button>) {
|
}: AlertDialogPrimitive.Close.Props &
|
||||||
|
Pick<React.ComponentProps<typeof Button>, "variant" | "size">) {
|
||||||
return (
|
return (
|
||||||
<Button
|
<AlertDialogPrimitive.Close
|
||||||
data-slot="alert-dialog-action"
|
data-slot="alert-dialog-action"
|
||||||
className={cn(className)}
|
className={cn(className)}
|
||||||
|
render={<Button variant={variant} size={size} />}
|
||||||
{...props}
|
{...props}
|
||||||
/>
|
/>
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user