Skip to content

Commit 262823e

Browse files
ironAiken2ironAiken2
committed
refactor(FR-2479): standardize confirmation UX with BAIConfirmModalWithInput and Popconfirm (#6471)
Resolves #6437 (FR-2479) ## Summary - **Phase 1**: Upgrade 6 destructive actions from `modal.confirm()` to `BAIConfirmModalWithInput` (typed-input confirmation) - `ResourcePresetList.tsx` — preset deletion - `EndpointList.tsx` — endpoint deletion - `ShellScriptEditModal.tsx` — script deletion - `AIAgentPage.tsx` — agent deletion and reset - `ManageAppsModal.tsx` — image reinstallation confirmation - `ManageImageResourceLimitModal.tsx` — image reinstallation confirmation - **Phase 2**: Convert 2 TOTP toggle confirmations from `modal.confirm()` to `Popconfirm` - `UserSettingModal.tsx` — TOTP deactivation toggle - `UserProfileSettingModal.tsx` — TOTP deactivation toggle - **Phase 3**: Remove orphaned `PopConfirmWithInput.tsx` (0 imports) ## Test plan - [ ] Delete a resource preset — typed confirmation modal appears, must type preset name - [ ] Delete an endpoint — typed confirmation modal appears, must type endpoint name - [ ] Delete a shell script — typed confirmation modal appears - [ ] Delete/reset an AI agent — typed confirmation modal appears - [ ] Modify apps on installed image — typed confirmation modal appears - [ ] Modify resource limits on installed image — typed confirmation modal appears - [ ] Toggle TOTP off (UserSettingModal) — Popconfirm appears inline - [ ] Toggle TOTP off (UserProfileSettingModal) — Popconfirm appears inline - [ ] Verify `PopConfirmWithInput.tsx` is removed
1 parent 2e0a5e1 commit 262823e

31 files changed

Lines changed: 459 additions & 316 deletions

‎.claude/rules/destructive-confirmation.md‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
# Destructive Action Confirmation Rule
2+
3+
For **irreversible destructive actions** (permanent deletion, purge, force termination, data wipe), confirmation must be collected through a **modal that requires the user to type a confirmation string** — not a `Popconfirm`, not a plain `Modal.confirm`, not a single-click dialog.
4+
5+
## Why
6+
7+
Popconfirm and one-click confirmation dialogs are appropriate for **reversible or low-impact** actions (inactivating, hiding, unassigning, canceling a draft). For actions the user cannot undo, a single misclick has permanent consequences. Requiring the user to type a specific string (typically the resource's name) forces a deliberate pause and prevents accidental destruction.
8+
9+
This convention was applied project-wide in FR-2479 ("standardize confirmation UX"), which replaced the legacy `PopConfirmWithInput.tsx` with the shared `BAIConfirmModalWithInput` component.
10+
11+
## Rules
12+
13+
1. **Irreversible actions → `BAIConfirmModalWithInput`** (from `backend.ai-ui`). The user must type a confirmation string (usually the resource's name) before the OK button enables. Examples: permanently delete a VFolder, terminate a model service endpoint, purge a user, delete an image, delete a resource preset, remove a shell script.
14+
2. **Reversible / low-impact actions → `Popconfirm`** or `App.useApp().modal.confirm({ ... })`. Examples: deactivating (not deleting) a user, canceling an in-progress action, hiding an item, marking inactive.
15+
3. **Never use `Popconfirm` for permanent deletion**, even when the action is guarded server-side. The UX contract is about *user intent*, not backend safety.
16+
4. Do **not** reintroduce `PopConfirmWithInput` or any ad-hoc "modal with a text input" for destructive flows — use the shared `BAIConfirmModalWithInput`. This keeps the copy, layout, danger styling, and accessibility consistent.
17+
5. The confirmation string should be something the user sees on screen and can copy unambiguously (e.g., the resource's `name` or `id`). Avoid opaque tokens.
18+
19+
## Pattern
20+
21+
### ❌ Wrong — Popconfirm for permanent deletion
22+
23+
```tsx
24+
<Popconfirm
25+
title={t('dialog.ask.DoYouWantToDeleteSomething', { name: row.name })}
26+
onConfirm={() => deleteForever(row.id)}
27+
>
28+
<Button danger icon={<DeleteOutlined />} />
29+
</Popconfirm>
30+
```
31+
32+
### ❌ Wrong — single-click `modal.confirm` for permanent deletion
33+
34+
```tsx
35+
modal.confirm({
36+
title: t('dialog.ask.DoYouWantToDeleteSomething', { name: row.name }),
37+
okButtonProps: { danger: true },
38+
onOk: () => deleteForever(row.id),
39+
});
40+
```
41+
42+
### ✅ Correct — typed confirmation for permanent deletion
43+
44+
```tsx
45+
import { BAIConfirmModalWithInput } from 'backend.ai-ui';
46+
47+
const [deletingTarget, setDeletingTarget] = useState<Row | null>(null);
48+
49+
// Trigger
50+
<Button
51+
danger
52+
icon={<DeleteOutlined />}
53+
onClick={() => setDeletingTarget(row)}
54+
/>
55+
56+
// Modal
57+
<BAIConfirmModalWithInput
58+
open={!!deletingTarget}
59+
title={t('dialog.ask.PermanentlyDeleteSomething', { name: deletingTarget?.name })}
60+
content={t('dialog.warning.CannotBeUndone')}
61+
confirmText={deletingTarget?.name ?? ''}
62+
okText={t('button.Delete')}
63+
onOk={async () => {
64+
if (deletingTarget) await deleteForever(deletingTarget.id);
65+
setDeletingTarget(null);
66+
}}
67+
onCancel={() => setDeletingTarget(null)}
68+
/>
69+
```
70+
71+
### ✅ Correct — `Popconfirm` for reversible actions
72+
73+
```tsx
74+
<Popconfirm
75+
title={t('dialog.ask.DoYouWantToInactivateSomething', { name: row.name })}
76+
onConfirm={() => setInactive(row.id)}
77+
>
78+
<Button icon={<StopOutlined />} />
79+
</Popconfirm>
80+
```
81+
82+
## How to decide
83+
84+
Ask: *"If the user clicks OK by accident, can they recover the state in <30 seconds without contacting support?"*
85+
86+
- **Yes** → `Popconfirm` / `modal.confirm` is fine.
87+
- **No** → `BAIConfirmModalWithInput`.
88+
89+
Soft-delete / trash-bin flows count as reversible **only if** the UI actually exposes a restore path the user can reach on their own. If restoration requires admin intervention or database access, treat it as irreversible.
90+
91+
## Related
92+
93+
- `BAIConfirmModalWithInput` — `packages/backend.ai-ui/src/components/BAIConfirmModalWithInput.tsx`
94+
- `BAIDeleteConfirmModal` — higher-level convenience wrapper for common delete flows
95+
- FR-2479 — the refactor that standardized this convention across the project

‎packages/backend.ai-ui/src/locale/ko.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@
388388
"modal": {
389389
"DeleteForeverDesc": "경고: 선택한 항목은 완전히 삭제되며 복원할 수 없습니다.",
390390
"ItemSelectedWithCount": "{{count}}개 선택됨.",
391-
"PleaseTypeToConfirm": "{{ confirmText }}를 입력하십시오."
391+
"PleaseTypeToConfirm": "{{ confirmText }}를 입력해주세요."
392392
},
393393
"validation": {
394394
"LetterNumber-_dot": "문자, 숫자, '-', '_', '.'만 입력할 수 있습니다.",

‎react/src/components/EndpointList.tsx‎

Lines changed: 66 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,20 @@ import {
1818
DeleteOutlined,
1919
SettingOutlined,
2020
} from '@ant-design/icons';
21-
import { Typography, theme, App, TablePaginationConfig } from 'antd';
21+
import { Typography, theme, App, TablePaginationConfig, Alert } from 'antd';
2222
import type { ColumnType } from 'antd/lib/table';
2323
import {
2424
filterOutEmpty,
2525
filterOutNullAndUndefined,
2626
BAITable,
2727
BAITableProps,
2828
BAINameActionCell,
29+
BAIConfirmModalWithInput,
30+
BAIFlex,
2931
} from 'backend.ai-ui';
3032
import dayjs from 'dayjs';
3133
import * as _ from 'lodash-es';
32-
import React from 'react';
34+
import React, { useState } from 'react';
3335
import { useTranslation } from 'react-i18next';
3436
import { graphql, useFragment } from 'react-relay';
3537

@@ -76,10 +78,13 @@ const EndpointList: React.FC<EndpointListProps> = ({
7678
}) => {
7779
const { t } = useTranslation();
7880
const { token } = theme.useToken();
79-
const { message, modal } = App.useApp();
81+
const { message } = App.useApp();
8082
const [currentUser] = useCurrentUserInfo();
8183
const baiClient = useSuspendedBackendaiClient();
8284
const webuiNavigate = useWebUINavigate();
85+
const [deletingEndpoint, setDeletingEndpoint] = useState<Endpoint | null>(
86+
null,
87+
);
8388

8489
const endpoints = useFragment(
8590
graphql`
@@ -151,46 +156,7 @@ const EndpointList: React.FC<EndpointListProps> = ({
151156
type: 'danger',
152157
disabled: isEndpointInDestroyingCategory(row),
153158
onClick: () => {
154-
modal.confirm({
155-
title: t('dialog.ask.DoYouWantToDeleteSomething', {
156-
name: row.name,
157-
}),
158-
content: t('dialog.warning.CannotBeUndone'),
159-
okText: t('button.Delete'),
160-
okButtonProps: {
161-
danger: true,
162-
type: 'primary',
163-
},
164-
onOk: () => {
165-
if (row.endpoint_id) {
166-
return new Promise<void>((resolve) => {
167-
terminateModelServiceMutation.mutate(row.endpoint_id!, {
168-
onSuccess: (res) => {
169-
onDeleted?.(row);
170-
if (res.success) {
171-
message.success(
172-
t('modelService.ServiceTerminated', {
173-
name: row?.name,
174-
}),
175-
);
176-
} else {
177-
message.error(
178-
t('modelService.FailedToTerminateService'),
179-
);
180-
}
181-
resolve();
182-
},
183-
onError: () => {
184-
message.error(
185-
t('modelService.FailedToTerminateService'),
186-
);
187-
resolve();
188-
},
189-
});
190-
});
191-
}
192-
},
193-
});
159+
setDeletingEndpoint(row);
194160
},
195161
},
196162
]}
@@ -282,16 +248,63 @@ const EndpointList: React.FC<EndpointListProps> = ({
282248
]);
283249

284250
return (
285-
<BAITable
286-
size="small"
287-
loading={loading}
288-
scroll={{ x: 'max-content' }}
289-
rowKey={'endpoint_id'}
290-
dataSource={filterOutNullAndUndefined(endpoints)}
291-
columns={columns}
292-
pagination={pagination}
293-
{...tableProps}
294-
/>
251+
<>
252+
<BAITable
253+
size="small"
254+
loading={loading}
255+
scroll={{ x: 'max-content' }}
256+
rowKey={'endpoint_id'}
257+
dataSource={filterOutNullAndUndefined(endpoints)}
258+
columns={columns}
259+
pagination={pagination}
260+
{...tableProps}
261+
/>
262+
<BAIConfirmModalWithInput
263+
open={!!deletingEndpoint}
264+
title={t('dialog.ask.DoYouWantToDeleteSomething', {
265+
name: deletingEndpoint?.name,
266+
})}
267+
content={
268+
<BAIFlex direction="column" gap="md" align="stretch">
269+
<Alert type="warning" title={t('dialog.warning.CannotBeUndone')} />
270+
<BAIFlex>
271+
<Typography.Text style={{ marginRight: token.marginXXS }}>
272+
{t('dialog.TypeNameToConfirmDeletion')}
273+
</Typography.Text>
274+
(<Typography.Text code>{deletingEndpoint?.name}</Typography.Text>)
275+
</BAIFlex>
276+
</BAIFlex>
277+
}
278+
confirmText={deletingEndpoint?.name ?? ''}
279+
inputProps={{ placeholder: deletingEndpoint?.name ?? '' }}
280+
okText={t('button.Delete')}
281+
okButtonProps={{ loading: terminateModelServiceMutation.isPending }}
282+
onOk={() => {
283+
if (deletingEndpoint?.endpoint_id) {
284+
terminateModelServiceMutation.mutate(deletingEndpoint.endpoint_id, {
285+
onSuccess: (res) => {
286+
onDeleted?.(deletingEndpoint);
287+
if (res.success) {
288+
message.success(
289+
t('modelService.ServiceTerminated', {
290+
name: deletingEndpoint?.name,
291+
}),
292+
);
293+
} else {
294+
message.error(t('modelService.FailedToTerminateService'));
295+
}
296+
setDeletingEndpoint(null);
297+
},
298+
onError: () => {
299+
message.error(t('modelService.FailedToTerminateService'));
300+
setDeletingEndpoint(null);
301+
},
302+
});
303+
}
304+
}}
305+
onCancel={() => setDeletingEndpoint(null)}
306+
/>
307+
</>
295308
);
296309
};
297310

‎react/src/components/ManageAppsModal.tsx‎

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,17 @@ import {
1111
Form,
1212
message,
1313
Typography,
14-
App,
1514
FormInstance,
1615
theme,
1716
} from 'antd';
18-
import { BAIFlex, BAIModal, BAIModalProps } from 'backend.ai-ui';
17+
import {
18+
BAIFlex,
19+
BAIModal,
20+
BAIModalProps,
21+
BAIConfirmModalWithInput,
22+
} from 'backend.ai-ui';
1923
import * as _ from 'lodash-es';
20-
import React from 'react';
24+
import React, { useState } from 'react';
2125
import { Trans, useTranslation } from 'react-i18next';
2226
import { graphql, useFragment, useMutation } from 'react-relay';
2327

@@ -37,7 +41,10 @@ const ManageAppsModal: React.FC<ManageAppsModalProps> = ({
3741
}) => {
3842
const { t } = useTranslation();
3943
const formRef = React.useRef<FormInstance>(null);
40-
const app = App.useApp();
44+
const [isReinstallConfirmOpen, setIsReinstallConfirmOpen] = useState(false);
45+
const [pendingCommitRequest, setPendingCommitRequest] = useState<
46+
(() => void) | null
47+
>(null);
4148

4249
const { token } = theme.useToken();
4350

@@ -157,21 +164,8 @@ const ManageAppsModal: React.FC<ManageAppsModalProps> = ({
157164
});
158165

159166
if (image?.installed) {
160-
app.modal.confirm({
161-
title: 'Image reinstallation required',
162-
content: (
163-
<>
164-
<Trans
165-
i18nKey={
166-
'environment.ModifyImageResourceLimitReinstallRequired'
167-
}
168-
/>
169-
</>
170-
),
171-
onOk: commitRequest,
172-
getContainer: () => document.body,
173-
closable: true,
174-
});
167+
setPendingCommitRequest(() => commitRequest);
168+
setIsReinstallConfirmOpen(true);
175169
} else {
176170
commitRequest();
177171
}
@@ -329,6 +323,25 @@ const ManageAppsModal: React.FC<ManageAppsModalProps> = ({
329323
</Form.List>
330324
</BAIFlex>
331325
</Form>
326+
<BAIConfirmModalWithInput
327+
open={isReinstallConfirmOpen}
328+
title={t('environment.ImageReinstallationRequired')}
329+
content={
330+
<Trans
331+
i18nKey={'environment.ModifyImageResourceLimitReinstallRequired'}
332+
/>
333+
}
334+
confirmText={image?.name ?? image?.namespace ?? ''}
335+
onOk={() => {
336+
setIsReinstallConfirmOpen(false);
337+
pendingCommitRequest?.();
338+
setPendingCommitRequest(null);
339+
}}
340+
onCancel={() => {
341+
setIsReinstallConfirmOpen(false);
342+
setPendingCommitRequest(null);
343+
}}
344+
/>
332345
</BAIModal>
333346
);
334347
};

0 commit comments

Comments
 (0)