mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
fix(site): replace modal delete confirmation with inline UI in agents admin (#22587)
## Problem
The agents admin panel (`/agents` → Admin button) is rendered inside a
Radix Dialog (`ConfigureAgentsDialog`). Deleting a model or provider
previously opened a MUI `DeleteDialog` on top, creating a modal-on-modal
situation. The two dialog systems (Radix and MUI) don't coordinate focus
trapping, scroll locking, or backdrop behavior, so the delete
confirmation was broken.
## Solution
Replace the modal `DeleteDialog` in both `ModelForm` and `ProviderForm`
with an inline confirmation strip rendered in the footer area. Clicking
"Delete" now swaps the footer to show:
- A warning message ("Are you sure? This action is irreversible.")
- Cancel and a destructive confirm button with loading spinner
This keeps everything within the existing Radix Dialog content pane — no
layering issues, no second modal.
## Changes
| File | Change |
|---|---|
| `ModelForm.tsx` | Added `isDeleting` prop, changed `onDeleteModel`
signature to async, added `confirmingDelete` state, inline confirmation
footer |
| `ProviderForm.tsx` | Removed `DeleteDialog` import/usage, replaced
with inline confirmation footer |
| `ModelsSection.tsx` | Removed `DeleteDialog` import/usage, removed
`modelToDelete` state, passes new props to `ModelForm` |
This commit is contained in:
@@ -621,6 +621,256 @@ export const ModelFormBedrock: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
export const ModelDeleteConfirmation: Story = {
|
||||
args: { section: "models" as ChatModelAdminSection },
|
||||
beforeEach: () => {
|
||||
setupChatSpies({
|
||||
providerConfigs: [
|
||||
createProviderConfig({
|
||||
id: "provider-openai",
|
||||
provider: "openai",
|
||||
display_name: "OpenAI",
|
||||
source: "database",
|
||||
has_api_key: true,
|
||||
}),
|
||||
],
|
||||
modelConfigs: [
|
||||
createModelConfig({
|
||||
id: "model-1",
|
||||
provider: "openai",
|
||||
model: "gpt-4o",
|
||||
display_name: "GPT-4o",
|
||||
}),
|
||||
],
|
||||
modelCatalog: { providers: [] },
|
||||
});
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const body = within(canvasElement.ownerDocument.body);
|
||||
|
||||
// Click the model row to open the edit form.
|
||||
await userEvent.click(await body.findByText("GPT-4o"));
|
||||
|
||||
// The Delete button should be visible in the footer.
|
||||
const deleteButton = await body.findByRole("button", { name: "Delete" });
|
||||
await expect(deleteButton).toBeInTheDocument();
|
||||
|
||||
// Click Delete to show the inline confirmation.
|
||||
await userEvent.click(deleteButton);
|
||||
|
||||
// The confirmation strip should appear — leave it visible
|
||||
// so the Chromatic snapshot captures this state.
|
||||
await expect(
|
||||
await body.findByText(/Are you sure\? This action is irreversible/i),
|
||||
).toBeInTheDocument();
|
||||
await expect(
|
||||
body.getByRole("button", { name: "Delete model" }),
|
||||
).toBeInTheDocument();
|
||||
await expect(
|
||||
body.getByRole("button", { name: "Cancel" }),
|
||||
).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const ModelDeleteCancelled: Story = {
|
||||
args: { section: "models" as ChatModelAdminSection },
|
||||
beforeEach: () => {
|
||||
setupChatSpies({
|
||||
providerConfigs: [
|
||||
createProviderConfig({
|
||||
id: "provider-openai",
|
||||
provider: "openai",
|
||||
display_name: "OpenAI",
|
||||
source: "database",
|
||||
has_api_key: true,
|
||||
}),
|
||||
],
|
||||
modelConfigs: [
|
||||
createModelConfig({
|
||||
id: "model-1",
|
||||
provider: "openai",
|
||||
model: "gpt-4o",
|
||||
display_name: "GPT-4o",
|
||||
}),
|
||||
],
|
||||
modelCatalog: { providers: [] },
|
||||
});
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const body = within(canvasElement.ownerDocument.body);
|
||||
|
||||
// Navigate to edit form, trigger confirmation, then cancel.
|
||||
await userEvent.click(await body.findByText("GPT-4o"));
|
||||
await userEvent.click(await body.findByRole("button", { name: "Delete" }));
|
||||
await body.findByText(/Are you sure/i);
|
||||
await userEvent.click(body.getByRole("button", { name: "Cancel" }));
|
||||
|
||||
// Normal footer should be restored.
|
||||
await expect(
|
||||
await body.findByRole("button", { name: "Delete" }),
|
||||
).toBeInTheDocument();
|
||||
await expect(
|
||||
await body.findByRole("button", { name: "Save" }),
|
||||
).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const ModelDeleteConfirmed: Story = {
|
||||
args: { section: "models" as ChatModelAdminSection },
|
||||
beforeEach: () => {
|
||||
setupChatSpies({
|
||||
providerConfigs: [
|
||||
createProviderConfig({
|
||||
id: "provider-openai",
|
||||
provider: "openai",
|
||||
display_name: "OpenAI",
|
||||
source: "database",
|
||||
has_api_key: true,
|
||||
}),
|
||||
],
|
||||
modelConfigs: [
|
||||
createModelConfig({
|
||||
id: "model-1",
|
||||
provider: "openai",
|
||||
model: "gpt-4o",
|
||||
display_name: "GPT-4o",
|
||||
}),
|
||||
],
|
||||
modelCatalog: { providers: [] },
|
||||
});
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const body = within(canvasElement.ownerDocument.body);
|
||||
|
||||
// Navigate to edit form, trigger delete confirmation, then confirm.
|
||||
await userEvent.click(await body.findByText("GPT-4o"));
|
||||
await userEvent.click(await body.findByRole("button", { name: "Delete" }));
|
||||
await userEvent.click(
|
||||
await body.findByRole("button", { name: "Delete model" }),
|
||||
);
|
||||
|
||||
// The delete API should have been called.
|
||||
await waitFor(() => {
|
||||
expect(API.deleteChatModelConfig).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
expect(API.deleteChatModelConfig).toHaveBeenCalledWith("model-1");
|
||||
},
|
||||
};
|
||||
|
||||
export const ProviderDeleteConfirmation: Story = {
|
||||
args: { section: "providers" as ChatModelAdminSection },
|
||||
beforeEach: () => {
|
||||
setupChatSpies({
|
||||
providerConfigs: [
|
||||
createProviderConfig({
|
||||
id: "provider-openai",
|
||||
provider: "openai",
|
||||
display_name: "OpenAI",
|
||||
source: "database",
|
||||
has_api_key: true,
|
||||
}),
|
||||
],
|
||||
modelConfigs: [],
|
||||
modelCatalog: { providers: [] },
|
||||
});
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const body = within(canvasElement.ownerDocument.body);
|
||||
|
||||
// Navigate to the provider detail view.
|
||||
await userEvent.click(await body.findByRole("button", { name: /OpenAI/i }));
|
||||
|
||||
// Click Delete to show the inline confirmation.
|
||||
const deleteButton = await body.findByRole("button", { name: "Delete" });
|
||||
await userEvent.click(deleteButton);
|
||||
|
||||
// The confirmation strip should appear — leave it visible
|
||||
// so the Chromatic snapshot captures this state.
|
||||
await expect(
|
||||
await body.findByText(/Are you sure\? This action is irreversible/i),
|
||||
).toBeInTheDocument();
|
||||
await expect(
|
||||
body.getByRole("button", { name: "Delete provider" }),
|
||||
).toBeInTheDocument();
|
||||
await expect(
|
||||
body.getByRole("button", { name: "Cancel" }),
|
||||
).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const ProviderDeleteCancelled: Story = {
|
||||
args: { section: "providers" as ChatModelAdminSection },
|
||||
beforeEach: () => {
|
||||
setupChatSpies({
|
||||
providerConfigs: [
|
||||
createProviderConfig({
|
||||
id: "provider-openai",
|
||||
provider: "openai",
|
||||
display_name: "OpenAI",
|
||||
source: "database",
|
||||
has_api_key: true,
|
||||
}),
|
||||
],
|
||||
modelConfigs: [],
|
||||
modelCatalog: { providers: [] },
|
||||
});
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const body = within(canvasElement.ownerDocument.body);
|
||||
|
||||
// Navigate to provider detail, trigger confirmation, then cancel.
|
||||
await userEvent.click(await body.findByRole("button", { name: /OpenAI/i }));
|
||||
await userEvent.click(await body.findByRole("button", { name: "Delete" }));
|
||||
await body.findByText(/Are you sure/i);
|
||||
await userEvent.click(body.getByRole("button", { name: "Cancel" }));
|
||||
|
||||
// Normal footer should be restored.
|
||||
await expect(
|
||||
await body.findByRole("button", { name: "Delete" }),
|
||||
).toBeInTheDocument();
|
||||
await expect(
|
||||
await body.findByRole("button", { name: "Save changes" }),
|
||||
).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const ProviderDeleteConfirmed: Story = {
|
||||
args: { section: "providers" as ChatModelAdminSection },
|
||||
beforeEach: () => {
|
||||
setupChatSpies({
|
||||
providerConfigs: [
|
||||
createProviderConfig({
|
||||
id: "provider-openai",
|
||||
provider: "openai",
|
||||
display_name: "OpenAI",
|
||||
source: "database",
|
||||
has_api_key: true,
|
||||
}),
|
||||
],
|
||||
modelConfigs: [],
|
||||
modelCatalog: { providers: [] },
|
||||
});
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const body = within(canvasElement.ownerDocument.body);
|
||||
|
||||
// Navigate to provider detail, trigger delete, then confirm.
|
||||
await userEvent.click(await body.findByRole("button", { name: /OpenAI/i }));
|
||||
await userEvent.click(await body.findByRole("button", { name: "Delete" }));
|
||||
await userEvent.click(
|
||||
await body.findByRole("button", { name: "Delete provider" }),
|
||||
);
|
||||
|
||||
// The delete API should have been called.
|
||||
await waitFor(() => {
|
||||
expect(API.deleteChatProviderConfig).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
expect(API.deleteChatProviderConfig).toHaveBeenCalledWith(
|
||||
"provider-openai",
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
export const ValidatesModelConfigFields: Story = {
|
||||
args: { section: "models" as ChatModelAdminSection },
|
||||
beforeEach: () => {
|
||||
|
||||
@@ -64,6 +64,7 @@ type ModelFormProps = {
|
||||
onSelectedProviderChange: (provider: string) => void;
|
||||
modelConfigsUnavailable: boolean;
|
||||
isSaving: boolean;
|
||||
isDeleting: boolean;
|
||||
onCreateModel: (
|
||||
req: TypesGen.CreateChatModelConfigRequest,
|
||||
) => Promise<unknown>;
|
||||
@@ -72,7 +73,7 @@ type ModelFormProps = {
|
||||
req: TypesGen.UpdateChatModelConfigRequest,
|
||||
) => Promise<unknown>;
|
||||
onCancel: () => void;
|
||||
onDeleteModel?: () => void;
|
||||
onDeleteModel?: (modelConfigId: string) => Promise<void>;
|
||||
};
|
||||
|
||||
export const ModelForm: FC<ModelFormProps> = ({
|
||||
@@ -83,6 +84,7 @@ export const ModelForm: FC<ModelFormProps> = ({
|
||||
onSelectedProviderChange,
|
||||
modelConfigsUnavailable,
|
||||
isSaving,
|
||||
isDeleting,
|
||||
onCreateModel,
|
||||
onUpdateModel,
|
||||
onCancel,
|
||||
@@ -90,6 +92,7 @@ export const ModelForm: FC<ModelFormProps> = ({
|
||||
}) => {
|
||||
const isEditing = Boolean(editingModel);
|
||||
const [showAdvanced, setShowAdvanced] = useState(false);
|
||||
const [confirmingDelete, setConfirmingDelete] = useState(false);
|
||||
|
||||
const canManageModels = Boolean(
|
||||
selectedProviderState?.providerConfig &&
|
||||
@@ -462,37 +465,68 @@ export const ModelForm: FC<ModelFormProps> = ({
|
||||
{/* Footer — pushed to bottom */}
|
||||
<div className="mt-auto pt-6">
|
||||
<hr className="mb-4 border-0 border-t border-solid border-border" />
|
||||
<div className="flex items-center justify-between">
|
||||
{isEditing && editingModel && onDeleteModel ? (
|
||||
{confirmingDelete && onDeleteModel && editingModel ? (
|
||||
<div className="flex items-center gap-3">
|
||||
<p className="m-0 flex-1 text-sm text-content-secondary">
|
||||
Are you sure? This action is irreversible.
|
||||
</p>
|
||||
<div className="flex shrink-0 items-center gap-2">
|
||||
<Button
|
||||
variant="outline"
|
||||
size="lg"
|
||||
type="button"
|
||||
onClick={() => setConfirmingDelete(false)}
|
||||
disabled={isDeleting}
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
<Button
|
||||
variant="destructive"
|
||||
size="lg"
|
||||
type="button"
|
||||
disabled={isDeleting}
|
||||
onClick={() => void onDeleteModel(editingModel.id)}
|
||||
>
|
||||
{isDeleting && (
|
||||
<Loader2Icon className="h-4 w-4 animate-spin" />
|
||||
)}
|
||||
Delete model
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex items-center justify-between">
|
||||
{isEditing && editingModel && onDeleteModel ? (
|
||||
<Button
|
||||
variant="outline"
|
||||
size="lg"
|
||||
type="button"
|
||||
className="text-content-secondary hover:text-content-destructive hover:border-border-destructive"
|
||||
disabled={isSaving}
|
||||
onClick={() => setConfirmingDelete(true)}
|
||||
>
|
||||
Delete
|
||||
</Button>
|
||||
) : (
|
||||
<Button
|
||||
variant="outline"
|
||||
size="lg"
|
||||
type="button"
|
||||
onClick={onCancel}
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
)}{" "}
|
||||
<Button
|
||||
variant="outline"
|
||||
size="lg"
|
||||
type="button"
|
||||
className="text-content-secondary hover:text-content-destructive hover:border-border-destructive"
|
||||
disabled={isSaving}
|
||||
onClick={() => onDeleteModel()}
|
||||
type="submit"
|
||||
disabled={isSaving || !form.isValid || hasFieldErrors}
|
||||
>
|
||||
Delete
|
||||
{isSaving && <Loader2Icon className="h-4 w-4 animate-spin" />}
|
||||
{isEditing ? "Save" : "Add model"}{" "}
|
||||
</Button>
|
||||
) : (
|
||||
<Button
|
||||
variant="outline"
|
||||
size="lg"
|
||||
type="button"
|
||||
onClick={onCancel}
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
)}{" "}
|
||||
<Button
|
||||
size="lg"
|
||||
type="submit"
|
||||
disabled={isSaving || !form.isValid || hasFieldErrors}
|
||||
>
|
||||
{isSaving && <Loader2Icon className="h-4 w-4 animate-spin" />}
|
||||
{isEditing ? "Save" : "Add model"}{" "}
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</form>
|
||||
</div>
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import type * as TypesGen from "api/typesGenerated";
|
||||
import { Badge } from "components/Badge/Badge";
|
||||
import { Button } from "components/Button/Button";
|
||||
import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog";
|
||||
import {
|
||||
DropdownMenu,
|
||||
DropdownMenuContent,
|
||||
@@ -68,8 +67,6 @@ export const ModelsSection: FC<ModelsSectionProps> = ({
|
||||
onDeleteModel,
|
||||
}) => {
|
||||
const [view, setView] = useState<ModelView>({ mode: "list" });
|
||||
const [modelToDelete, setModelToDelete] =
|
||||
useState<TypesGen.ChatModelConfig | null>(null);
|
||||
|
||||
// When the form is open it takes over the full panel.
|
||||
if (view.mode === "add" || view.mode === "edit") {
|
||||
@@ -100,6 +97,7 @@ export const ModelsSection: FC<ModelsSectionProps> = ({
|
||||
onSelectedProviderChange={onSelectedProviderChange}
|
||||
modelConfigsUnavailable={modelConfigsUnavailable}
|
||||
isSaving={isCreating || isUpdating}
|
||||
isDeleting={isDeleting}
|
||||
onCreateModel={async (req) => {
|
||||
await onCreateModel(req);
|
||||
setView({ mode: "list" });
|
||||
@@ -111,8 +109,8 @@ export const ModelsSection: FC<ModelsSectionProps> = ({
|
||||
onCancel={() => setView({ mode: "list" })}
|
||||
onDeleteModel={
|
||||
editingModel
|
||||
? () => {
|
||||
setModelToDelete(editingModel);
|
||||
? async (id) => {
|
||||
await onDeleteModel(id);
|
||||
setView({ mode: "list" });
|
||||
}
|
||||
: undefined
|
||||
@@ -255,21 +253,6 @@ export const ModelsSection: FC<ModelsSectionProps> = ({
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
|
||||
<DeleteDialog
|
||||
isOpen={modelToDelete !== null}
|
||||
onCancel={() => setModelToDelete(null)}
|
||||
onConfirm={() => {
|
||||
if (modelToDelete) {
|
||||
void onDeleteModel(modelToDelete.id).finally(() =>
|
||||
setModelToDelete(null),
|
||||
);
|
||||
}
|
||||
}}
|
||||
entity="model"
|
||||
name={modelToDelete?.display_name || modelToDelete?.model || ""}
|
||||
confirmLoading={isDeleting}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import type * as TypesGen from "api/typesGenerated";
|
||||
import { Alert, AlertDetail, AlertTitle } from "components/Alert/Alert";
|
||||
import { Button } from "components/Button/Button";
|
||||
import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog";
|
||||
import { Input } from "components/Input/Input";
|
||||
import {
|
||||
Tooltip,
|
||||
@@ -62,7 +61,7 @@ export const ProviderForm: FC<ProviderFormProps> = ({
|
||||
);
|
||||
const [apiKeyTouched, setApiKeyTouched] = useState(false);
|
||||
const [baseURLValue, setBaseURLValue] = useState(initialValues.baseURL);
|
||||
const [showDeleteDialog, setShowDeleteDialog] = useState(false);
|
||||
const [confirmingDelete, setConfirmingDelete] = useState(false);
|
||||
|
||||
const isAPIKeyEnvManaged = isEnvPreset && !providerConfig;
|
||||
const requiresAPIKey = !providerConfig && !isAPIKeyEnvManaged;
|
||||
@@ -253,46 +252,62 @@ export const ProviderForm: FC<ProviderFormProps> = ({
|
||||
{/* Footer — pushed to bottom */}
|
||||
<div className="mt-auto pt-6">
|
||||
<hr className="mb-4 border-0 border-t border-solid border-border" />
|
||||
<div className="flex items-center justify-between">
|
||||
{providerConfig ? (
|
||||
<Button
|
||||
variant="outline"
|
||||
size="lg"
|
||||
type="button"
|
||||
className="text-content-secondary hover:text-content-destructive hover:border-border-destructive"
|
||||
disabled={isDisabled}
|
||||
onClick={() => setShowDeleteDialog(true)}
|
||||
>
|
||||
Delete
|
||||
</Button>
|
||||
) : (
|
||||
<div />
|
||||
)}
|
||||
<Button size="lg" type="submit" disabled={!canSave}>
|
||||
{isProviderMutationPending && (
|
||||
<Loader2Icon className="h-4 w-4 animate-spin" />
|
||||
{confirmingDelete && providerConfig ? (
|
||||
<div className="flex items-center gap-3">
|
||||
<p className="m-0 flex-1 text-sm text-content-secondary">
|
||||
Are you sure? This action is irreversible.
|
||||
</p>
|
||||
<div className="flex shrink-0 items-center gap-2">
|
||||
<Button
|
||||
variant="outline"
|
||||
size="lg"
|
||||
type="button"
|
||||
onClick={() => setConfirmingDelete(false)}
|
||||
disabled={isProviderMutationPending}
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
<Button
|
||||
variant="destructive"
|
||||
size="lg"
|
||||
type="button"
|
||||
disabled={isProviderMutationPending}
|
||||
onClick={() => void onDeleteProvider(providerConfig.id)}
|
||||
>
|
||||
{isProviderMutationPending && (
|
||||
<Loader2Icon className="h-4 w-4 animate-spin" />
|
||||
)}
|
||||
Delete provider
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex items-center justify-between">
|
||||
{providerConfig ? (
|
||||
<Button
|
||||
variant="outline"
|
||||
size="lg"
|
||||
type="button"
|
||||
className="text-content-secondary hover:text-content-destructive hover:border-border-destructive"
|
||||
disabled={isDisabled}
|
||||
onClick={() => setConfirmingDelete(true)}
|
||||
>
|
||||
Delete
|
||||
</Button>
|
||||
) : (
|
||||
<div />
|
||||
)}
|
||||
{providerConfig ? "Save changes" : "Create provider config"}
|
||||
</Button>
|
||||
</div>
|
||||
<Button size="lg" type="submit" disabled={!canSave}>
|
||||
{isProviderMutationPending && (
|
||||
<Loader2Icon className="h-4 w-4 animate-spin" />
|
||||
)}
|
||||
{providerConfig ? "Save changes" : "Create provider config"}
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</form>
|
||||
)}
|
||||
|
||||
<DeleteDialog
|
||||
isOpen={showDeleteDialog}
|
||||
onCancel={() => setShowDeleteDialog(false)}
|
||||
onConfirm={() => {
|
||||
if (providerConfig) {
|
||||
void onDeleteProvider(providerConfig.id).finally(() =>
|
||||
setShowDeleteDialog(false),
|
||||
);
|
||||
}
|
||||
}}
|
||||
entity="provider"
|
||||
name={providerState.label}
|
||||
confirmLoading={isProviderMutationPending}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user