Skip to content

Commit 91cef6f

Browse files
committed
refactor(FR-2479): standardize confirmation UX with BAIConfirmModalWithInput and Popconfirm
1 parent 4892c49 commit 91cef6f

31 files changed

Lines changed: 436 additions & 295 deletions
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
@@ -386,7 +386,7 @@
386386
"modal": {
387387
"DeleteForeverDesc": "경고: 선택한 항목은 완전히 삭제되며 복원할 수 없습니다.",
388388
"ItemSelectedWithCount": "{{count}}개 선택됨.",
389-
"PleaseTypeToConfirm": "{{ confirmText }}를 입력하십시오."
389+
"PleaseTypeToConfirm": "{{ confirmText }}를 입력해주세요."
390390
},
391391
"validation": {
392392
"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

@@ -73,10 +75,13 @@ const EndpointList: React.FC<EndpointListProps> = ({
7375
}) => {
7476
const { t } = useTranslation();
7577
const { token } = theme.useToken();
76-
const { message, modal } = App.useApp();
78+
const { message } = App.useApp();
7779
const [currentUser] = useCurrentUserInfo();
7880
const baiClient = useSuspendedBackendaiClient();
7981
const webuiNavigate = useWebUINavigate();
82+
const [deletingEndpoint, setDeletingEndpoint] = useState<Endpoint | null>(
83+
null,
84+
);
8085

8186
const endpoints = useFragment(
8287
graphql`
@@ -144,46 +149,7 @@ const EndpointList: React.FC<EndpointListProps> = ({
144149
type: 'danger',
145150
disabled: isEndpointInDestroyingCategory(row),
146151
onClick: () => {
147-
modal.confirm({
148-
title: t('dialog.ask.DoYouWantToDeleteSomething', {
149-
name: row.name,
150-
}),
151-
content: t('dialog.warning.CannotBeUndone'),
152-
okText: t('button.Delete'),
153-
okButtonProps: {
154-
danger: true,
155-
type: 'primary',
156-
},
157-
onOk: () => {
158-
if (row.endpoint_id) {
159-
return new Promise<void>((resolve) => {
160-
terminateModelServiceMutation.mutate(row.endpoint_id!, {
161-
onSuccess: (res) => {
162-
onDeleted?.(row);
163-
if (res.success) {
164-
message.success(
165-
t('modelService.ServiceTerminated', {
166-
name: row?.name,
167-
}),
168-
);
169-
} else {
170-
message.error(
171-
t('modelService.FailedToTerminateService'),
172-
);
173-
}
174-
resolve();
175-
},
176-
onError: () => {
177-
message.error(
178-
t('modelService.FailedToTerminateService'),
179-
);
180-
resolve();
181-
},
182-
});
183-
});
184-
}
185-
},
186-
});
152+
setDeletingEndpoint(row);
187153
},
188154
},
189155
]}
@@ -267,16 +233,63 @@ const EndpointList: React.FC<EndpointListProps> = ({
267233
]);
268234

269235
return (
270-
<BAITable
271-
size="small"
272-
loading={loading}
273-
scroll={{ x: 'max-content' }}
274-
rowKey={'endpoint_id'}
275-
dataSource={filterOutNullAndUndefined(endpoints)}
276-
columns={columns}
277-
pagination={pagination}
278-
{...tableProps}
279-
/>
236+
<>
237+
<BAITable
238+
size="small"
239+
loading={loading}
240+
scroll={{ x: 'max-content' }}
241+
rowKey={'endpoint_id'}
242+
dataSource={filterOutNullAndUndefined(endpoints)}
243+
columns={columns}
244+
pagination={pagination}
245+
{...tableProps}
246+
/>
247+
<BAIConfirmModalWithInput
248+
open={!!deletingEndpoint}
249+
title={t('dialog.ask.DoYouWantToDeleteSomething', {
250+
name: deletingEndpoint?.name,
251+
})}
252+
content={
253+
<BAIFlex direction="column" gap="md" align="stretch">
254+
<Alert type="warning" title={t('dialog.warning.CannotBeUndone')} />
255+
<BAIFlex>
256+
<Typography.Text style={{ marginRight: token.marginXXS }}>
257+
{t('dialog.TypeNameToConfirmDeletion')}
258+
</Typography.Text>
259+
(<Typography.Text code>{deletingEndpoint?.name}</Typography.Text>)
260+
</BAIFlex>
261+
</BAIFlex>
262+
}
263+
confirmText={deletingEndpoint?.name ?? ''}
264+
inputProps={{ placeholder: deletingEndpoint?.name ?? '' }}
265+
okText={t('button.Delete')}
266+
okButtonProps={{ loading: terminateModelServiceMutation.isPending }}
267+
onOk={() => {
268+
if (deletingEndpoint?.endpoint_id) {
269+
terminateModelServiceMutation.mutate(deletingEndpoint.endpoint_id, {
270+
onSuccess: (res) => {
271+
onDeleted?.(deletingEndpoint);
272+
if (res.success) {
273+
message.success(
274+
t('modelService.ServiceTerminated', {
275+
name: deletingEndpoint?.name,
276+
}),
277+
);
278+
} else {
279+
message.error(t('modelService.FailedToTerminateService'));
280+
}
281+
setDeletingEndpoint(null);
282+
},
283+
onError: () => {
284+
message.error(t('modelService.FailedToTerminateService'));
285+
setDeletingEndpoint(null);
286+
},
287+
});
288+
}
289+
}}
290+
onCancel={() => setDeletingEndpoint(null)}
291+
/>
292+
</>
280293
);
281294
};
282295

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)