Copilot commented on code in PR #18029:
URL:
https://github.com/apache/dolphinscheduler/pull/18029#discussion_r2902863078
##########
dolphinscheduler-ui/src/views/projects/workflow/definition/components/table-action.tsx:
##########
@@ -271,7 +271,19 @@ export default defineComponent({
onPositiveClick={this.handleDeleteWorkflow}
>
{{
- default: () => t('project.workflow.delete_confirm'),
+ default: () => (
+ <div style={{ maxWidth: 360 }}>
+ <div style={{ fontWeight: 600, marginBottom: 6 }}>
+ {`${t('project.workflow.delete_confirm')} "${
+ this.row?.name || ''
+ }"`}
+ </div>
+ <div style={{ color: '#ff4d4f' }}>
+ {t('project.workflow.delete_irreversible') ||
+ 'This action cannot be undone. The workflow and its
associated data will be permanently deleted.'}
Review 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.
```suggestion
{t('project.workflow.delete_irreversible')}
```
##########
dolphinscheduler-ui/src/views/projects/workflow/definition/components/table-action.tsx:
##########
@@ -271,7 +271,19 @@ export default defineComponent({
onPositiveClick={this.handleDeleteWorkflow}
>
{{
- default: () => t('project.workflow.delete_confirm'),
+ default: () => (
+ <div style={{ maxWidth: 360 }}>
+ <div style={{ fontWeight: 600, marginBottom: 6 }}>
+ {`${t('project.workflow.delete_confirm')} "${
+ this.row?.name || ''
+ }"`}
Review 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.
```suggestion
{t('project.workflow.delete_confirm_with_name', {
name: this.row?.name || ''
})}
```
##########
dolphinscheduler-ui/src/views/projects/workflow/definition/components/table-action.tsx:
##########
@@ -271,7 +271,19 @@ export default defineComponent({
onPositiveClick={this.handleDeleteWorkflow}
>
{{
- default: () => t('project.workflow.delete_confirm'),
+ default: () => (
+ <div style={{ maxWidth: 360 }}>
+ <div style={{ fontWeight: 600, marginBottom: 6 }}>
+ {`${t('project.workflow.delete_confirm')} "${
+ this.row?.name || ''
+ }"`}
+ </div>
+ <div style={{ color: '#ff4d4f' }}>
+ {t('project.workflow.delete_irreversible') ||
+ 'This action cannot be undone. The workflow and its
associated data will be permanently deleted.'}
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
