[Improvement-17947][ui] Improve delete confirmation to show workflow name#18029
[Improvement-17947][ui] Improve delete confirmation to show workflow name#18029asadjan4611 wants to merge 2 commits intoapache:devfrom
Conversation
…on (apache#17947) Show the workflow name and an irreversible-action warning inside the existing NPopconfirm to reduce accidental deletions.
asadjan4611
changed the title
jieguangzhou
left a comment
jieguangzhou
left a comment
There was a problem hiding this comment.
LGTM. This change only updates the workflow delete popconfirm copy and locale entries. No blocking issues found.
|
Thanks for the review @jieguangzhou . Thank you! |
There was a problem hiding this comment.
Pull request overview
Enhances the workflow definition delete confirmation popover to provide more context (workflow name) and a clear irreversibility warning to reduce accidental deletions.
Changes:
- Updated the workflow definition table delete
NPopconfirmcontent to include the workflow name and an irreversible-action warning. - Added a new i18n key
delete_irreversibleto both English and Chinese locale bundles.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| dolphinscheduler-ui/src/views/projects/workflow/definition/components/table-action.tsx | Renders a richer delete confirmation message (workflow name + warning). |
| dolphinscheduler-ui/src/locales/en_US/project.ts | Adds English translation for the irreversible delete warning. |
| dolphinscheduler-ui/src/locales/zh_CN/project.ts | Adds Chinese translation for the irreversible delete warning. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| {`${t('project.workflow.delete_confirm')} "${ | ||
| this.row?.name || '' | ||
| }"`} |
There was a problem hiding this comment.
The confirmation title is built by concatenating a translated fragment (delete_confirm) with the workflow name. This is hard to localize correctly (word order/punctuation differ by language) and can read awkwardly (e.g., Delete? "name"). Consider introducing a dedicated i18n key that includes a placeholder for the name (e.g., delete_confirm_with_name) and render using that single translation.
| {`${t('project.workflow.delete_confirm')} "${ | |
| this.row?.name || '' | |
| }"`} | |
| {t('project.workflow.delete_confirm_with_name', { | |
| name: this.row?.name || '' | |
| })} |
| <div style={{ color: '#ff4d4f' }}> | ||
| {t('project.workflow.delete_irreversible') || | ||
| 'This action cannot be undone. The workflow and its associated data will be permanently deleted.'} |
There was a problem hiding this comment.
Avoid hard-coding #ff4d4f for the warning text color. The app defines theme-specific errorColor (e.g., dark theme uses a different value), so this will look wrong in dark mode. Prefer using a theme token (CSS variable) or a Naive UI component like NText with type='error' so it adapts to the active theme.
| {t('project.workflow.delete_irreversible') || | ||
| 'This action cannot be undone. The workflow and its associated data will be permanently deleted.'} |
There was a problem hiding this comment.
t('project.workflow.delete_irreversible') || '...' is effectively dead code: t() returns a non-empty string even when the key is missing (typically the key itself), so the hard-coded English fallback will never be used. Also, embedding a raw English sentence here bypasses i18n. Prefer relying on the i18n key only (and ensure it exists in all locale files) or handle missing keys via the i18n layer.
| {t('project.workflow.delete_irreversible') || | |
| 'This action cannot be undone. The workflow and its associated data will be permanently deleted.'} | |
| {t('project.workflow.delete_irreversible')} |
|
asadjan4611
force-pushed
the
Feature-17947
branch
from
7498af7 to
51e5048
Compare
Description
Enhance the workflow delete confirmation to display the workflow name and an irreversible-action warning to reduce accidental deletions.
Changes
dolphinscheduler-ui/src/views/projects/workflow/definition/components/table-action.tsxNPopconfirmdolphinscheduler-ui/src/locales/en_US/project.tsdelete_irreversiblekeydolphinscheduler-ui/src/locales/zh_CN/project.tsdelete_irreversiblekeyWhy
Users couldn’t see which workflow they'd delete; adding the name and clear warning improves safety and UX for destructive actions.
How to test
Related
Closes #17947