fix(FR-2819): use BAIConfirmModalWithInput for irreversible delete actions#7370
Merged
Merged
Conversation
Contributor
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
|
|
|
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes irreversible delete confirmations by replacing simple confirm dialogs with BAIConfirmModalWithInput, requiring users to type an identifier before destructive actions proceed (aligning with the project’s destructive-action UX pattern).
Changes:
- Updated keypair deletion in
UserCredentialListto useBAIConfirmModalWithInputinstead ofmodal.confirm. - Updated auto-scaling rule deletion in
AutoScalingRuleListLegacyto useBAIConfirmModalWithInputand replacedCardwithBAICard. - Refactored delete triggers to set “currently deleting” state and render the typed-confirm modal at the component level.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| react/src/components/UserCredentialList.tsx | Switches keypair deletion confirmation from modal.confirm to BAIConfirmModalWithInput using a selected-row state. |
| react/src/components/AutoScalingRuleListLegacy.tsx | Switches rule deletion from Popconfirm to BAIConfirmModalWithInput, introduces delete-target state, and uses BAICard. |
ironAiken2
force-pushed
the
fix/FR-2819-use-bai-confirm-modal
branch
from
58235a5 to
f1a3556
Compare
github-actions
Bot
added
area:ux
Contributor
Coverage Report for backend-ai-ui-coverage (./packages/backend.ai-ui)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This was referenced May 14, 2026
nowgnuesLee
requested changes
May 14, 2026
Contributor
nowgnuesLee
left a comment
nowgnuesLee
left a comment
There was a problem hiding this comment.
Please also update the Storybook case for when target is included.
Contributor
Author
|
Addressed feedback from @nowgnuesLee: added a |
ironAiken2
force-pushed
the
fix/FR-2819-use-bai-confirm-modal
branch
from
72c5872 to
f942af5
Compare
nowgnuesLee
requested changes
May 15, 2026
Contributor
nowgnuesLee
left a comment
nowgnuesLee
left a comment
There was a problem hiding this comment.
please fill the pr description.
5 tasks
ironAiken2
force-pushed
the
fix/FR-2819-use-bai-confirm-modal
branch
from
f942af5 to
1839b25
Compare
Merge activity
|
…tions (#7370) Resolves #7253(FR-2819) ## Summary Sweep all permanent-delete flows that still used `modal.confirm` / `Popconfirm` / ad-hoc dialogs and move them onto the project's typed-confirmation pattern (`.claude/rules/destructive-confirmation.md`). In the process, **consolidate two overlapping components** — `BAIConfirmModalWithInput` and `BAIDeleteConfirmModal` — into a single API so every irreversible delete in the app looks and behaves the same. ## What changed ### Component consolidation - **Remove `BAIConfirmModalWithInput`** (component, stories, exports). All 16 call sites are migrated to `BAIDeleteConfirmModal`. - **Extend `BAIDeleteConfirmModal`**: - Add `requireConfirmInput` to force typed confirmation for single-item deletes. - Add `target` prop — when set (and no `description` is provided) the modal renders the localized "Are you sure you want to permanently delete {target}?" copy. Centralizes the most common irreversible-delete sentence so call sites don't redeclare it. - Reinstall-confirm flows (`ManageImageResourceLimitModal`, `ManageAppsModal`) — which are confirmations but **not deletes** — are moved to plain `BAIModal` instead of a delete-shaped modal. They were only on `BAIConfirmModalWithInput` for the typed-input UX, which they don't actually need. ### Bug fixes (the issue's scope) - `UserCredentialList.tsx` — permanent keypair deletion was a single-click `modal.confirm`. Now opens `BAIDeleteConfirmModal` with `requireConfirmInput` and the keypair's `access_key` as the confirm string. - `AutoScalingRuleListLegacy.tsx` — permanent rule deletion was a `Popconfirm`. Now opens `BAIDeleteConfirmModal` requiring the rule's identifier to be typed. Card is migrated to `BAICard` in the same change per project convention. ### Sweep — other irreversible flows migrated Single-click confirms / ad-hoc patterns replaced with `BAIDeleteConfirmModal` (+ `requireConfirmInput` where the item is sensitive): - `AutoScalingRuleList.tsx` (V2) - `ContainerRegistryList.tsx` - `CustomizedImageList.tsx` - `DeleteForeverVFolderModalV2.tsx` - `DeploymentAccessTokensTab.tsx`, `DeploymentConfigurationSection.tsx`, `DeploymentList.tsx`, `EndpointList.tsx` - `KeypairResourcePolicyList.tsx`, `ProjectResourcePolicyList.tsx`, `UserResourcePolicyList.tsx` - `MyKeypairManagementModal.tsx` - `PrometheusPresetTab.tsx` - `PurgeUsersModal.tsx` - `ResourceGroupList.tsx`, `ResourcePresetList.tsx` - `RoleAssignmentTab.tsx`, `RolePermissionTab.tsx`, `RBACManagementPage.tsx` - `ShellScriptEditModal.tsx` - `UserCredentialList.tsx` - `VFolderNodes.tsx` - `AIAgentPage.tsx` - `FileExplorer/DeleteSelectedItemsModal.tsx` - `fragments/BAIProjectTable.tsx` ### `confirmText` policy Standardized so each call site picks the **safer** of two options: - **User-readable IDs** (project name, vfolder name, keypair `access_key`, role name, …) → use the resource's name as the confirm string. The user has it on screen and can copy-type it unambiguously. - **Cryptic / long IDs** (image full path, autoscaling metric name, bulk purge across N users) → use the literal `"Permanently Delete"` string. Asking the user to type a 90-char Docker reference is worse UX, not better. Activate / deactivate (reversible) flows are intentionally **left as-is** — those belong to FR-2825. ### i18n - New keys across all 22 locales: - `general.*` resource-type labels (9 new entries) for the `target` prop. - `dialog.title.DeleteSomething` - `comp:BAIDeleteConfirmModal.AreYouSureToPermanentlyDeleteTarget` - Filled in previously missing keys across non-English locales (translations were already drifting; this PR brings them back in sync). - Fix one Korean typo: `environment.ModifyImageResourceLimitReinstallRequired`. ### Storybook - Remove `BAIConfirmModalWithInput.stories.tsx`. - Add a Storybook case for the new `target` prop on `BAIDeleteConfirmModal`. ## Why one API, not two Before: `BAIConfirmModalWithInput` (typed-input shape) and `BAIDeleteConfirmModal` (item-list shape) covered overlapping use cases — the typed-input modal was being used for deletes, and call sites were duplicating copy and layout that already existed in `BAIDeleteConfirmModal`. After: a single component covers both the single-item and N-item delete cases, with `requireConfirmInput` and `target` handling the variants. Less surface area, less drift, fewer translation keys to keep aligned. ## Checklist - [x] Documentation — N/A (no user-facing manual change; convention rule unchanged) - [x] Minimum required manager version — N/A - [x] Specific setting for review — none - [x] Minimum requirements to check during review — `bash scripts/verify.sh` passes; manually exercise a few migrated flows (keypair delete, autoscaling rule delete, vfolder permanent delete, bulk user purge) to confirm the OK button stays disabled until the right string is typed - [x] Test case(s) — see Storybook stories and the migrated call sites; behavior across all 16 sites is now driven by one component
graphite-app
Bot
force-pushed
the
fix/FR-2819-use-bai-confirm-modal
branch
from
1839b25 to
0ca1f8b
Compare
This was referenced May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #7253(FR-2819)
Summary
Sweep all permanent-delete flows that still used
modal.confirm/Popconfirm/ ad-hoc dialogs and move them onto the project's typed-confirmation pattern (.claude/rules/destructive-confirmation.md). In the process, consolidate two overlapping components —BAIConfirmModalWithInputandBAIDeleteConfirmModal— into a single API so every irreversible delete in the app looks and behaves the same.What changed
Component consolidation
BAIConfirmModalWithInput(component, stories, exports). All 16 call sites are migrated toBAIDeleteConfirmModal.BAIDeleteConfirmModal:requireConfirmInputto force typed confirmation for single-item deletes.targetprop — when set (and nodescriptionis provided) the modal renders the localized "Are you sure you want to permanently delete {target}?" copy. Centralizes the most common irreversible-delete sentence so call sites don't redeclare it.ManageImageResourceLimitModal,ManageAppsModal) — which are confirmations but not deletes — are moved to plainBAIModalinstead of a delete-shaped modal. They were only onBAIConfirmModalWithInputfor the typed-input UX, which they don't actually need.Bug fixes (the issue's scope)
UserCredentialList.tsx— permanent keypair deletion was a single-clickmodal.confirm. Now opensBAIDeleteConfirmModalwithrequireConfirmInputand the keypair'saccess_keyas the confirm string.AutoScalingRuleListLegacy.tsx— permanent rule deletion was aPopconfirm. Now opensBAIDeleteConfirmModalrequiring the rule's identifier to be typed. Card is migrated toBAICardin the same change per project convention.Sweep — other irreversible flows migrated
Single-click confirms / ad-hoc patterns replaced with
BAIDeleteConfirmModal(+requireConfirmInputwhere the item is sensitive):AutoScalingRuleList.tsx(V2)ContainerRegistryList.tsxCustomizedImageList.tsxDeleteForeverVFolderModalV2.tsxDeploymentAccessTokensTab.tsx,DeploymentConfigurationSection.tsx,DeploymentList.tsx,EndpointList.tsxKeypairResourcePolicyList.tsx,ProjectResourcePolicyList.tsx,UserResourcePolicyList.tsxMyKeypairManagementModal.tsxPrometheusPresetTab.tsxPurgeUsersModal.tsxResourceGroupList.tsx,ResourcePresetList.tsxRoleAssignmentTab.tsx,RolePermissionTab.tsx,RBACManagementPage.tsxShellScriptEditModal.tsxUserCredentialList.tsxVFolderNodes.tsxAIAgentPage.tsxFileExplorer/DeleteSelectedItemsModal.tsxfragments/BAIProjectTable.tsxconfirmTextpolicyStandardized so each call site picks the safer of two options:
access_key, role name, …) → use the resource's name as the confirm string. The user has it on screen and can copy-type it unambiguously."Permanently Delete"string. Asking the user to type a 90-char Docker reference is worse UX, not better.Activate / deactivate (reversible) flows are intentionally left as-is — those belong to FR-2825.
i18n
general.*resource-type labels (9 new entries) for thetargetprop.dialog.title.DeleteSomethingcomp:BAIDeleteConfirmModal.AreYouSureToPermanentlyDeleteTargetenvironment.ModifyImageResourceLimitReinstallRequired.Storybook
BAIConfirmModalWithInput.stories.tsx.targetprop onBAIDeleteConfirmModal.Why one API, not two
Before:
BAIConfirmModalWithInput(typed-input shape) andBAIDeleteConfirmModal(item-list shape) covered overlapping use cases — the typed-input modal was being used for deletes, and call sites were duplicating copy and layout that already existed inBAIDeleteConfirmModal. After: a single component covers both the single-item and N-item delete cases, withrequireConfirmInputandtargethandling the variants. Less surface area, less drift, fewer translation keys to keep aligned.Checklist
bash scripts/verify.shpasses; manually exercise a few migrated flows (keypair delete, autoscaling rule delete, vfolder permanent delete, bulk user purge) to confirm the OK button stays disabled until the right string is typed