Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Optimization of modifying node names in workflow #2432

Merged
merged 1 commit into from
Feb 27, 2025
Merged

perf: Optimization of modifying node names in workflow #2432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ui/src/locales/lang/en-US/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,6 @@ export default {

inputPlaceholder: 'Please input',
title: 'Title',
content: 'Content'
content: 'Content',
rename: 'Rename'
}
1 change: 1 addition & 0 deletions ui/src/locales/lang/en-US/views/application-workflow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export default {
node: 'Node',
nodeName: 'Node Name',
baseComponent: 'Basic',
nodeSetting: 'Node Settings',
workflow: 'Workflow',
Expand Down
3 changes: 2 additions & 1 deletion ui/src/locales/lang/zh-CN/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,6 @@ export default {
param: {
outputParam: '输出参数',
inputParam:'输入参数'
}
},
rename:'重命名'
}
1 change: 1 addition & 0 deletions ui/src/locales/lang/zh-CN/views/application-workflow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export default {
node: '节点',
nodeName: '节点名称',
baseComponent: '基础组件',
nodeSetting: '节点设置',
workflow: '工作流',
Expand Down
5 changes: 3 additions & 2 deletions ui/src/locales/lang/zh-Hant/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export default {
content: '内容',
param: {
outputParam: '輸出參數',
inputParam:'輸入參數'
}
inputParam: '輸入參數'
},
rename: '重命名'
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided JavaScript export default object appears to be correctly formatted with no errors. However, since there is an additional comma after rename: '重命名', it might cause confusion or unexpected behavior in environments that enforce strict JSON formatting.

If possible, consider removing the trailing comma for clean syntax:

export default {
  content: '内容',
  param: {
    outputParam: '輸出參數',
    inputParam: '輸入參數'
  },
  rename: '重命名'
}

This adjustment ensures proper JSON compliance.

1 change: 1 addition & 0 deletions ui/src/locales/lang/zh-Hant/views/application-workflow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export default {
node: '節點',
nodeName: '節點名稱',
baseComponent: '基礎組件',
nodeSetting: '節點設置',
workflow: '工作流',
Expand Down
95 changes: 68 additions & 27 deletions ui/src/workflow/common/NodeContainer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,14 @@
>
<div v-resize="resizeStepContainer">
<div class="flex-between">
<div
class="flex align-center"
:style="{ maxWidth: node_status == 200 ? 'calc(100% - 85px)' : 'calc(100% - 85px)' }"
>
<div class="flex align-center" style="width: 70%;">
<component
:is="iconComponent(`${nodeModel.type}-icon`)"
class="mr-8"
:size="24"
:item="nodeModel?.properties.node_data"
/>
<h4 v-if="showOperate(nodeModel.type)" style="max-width: 90%">
<ReadWrite
@mousemove.stop
@mousedown.stop
@keydown.stop
@click.stop
@change="editName"
:data="nodeModel.properties.stepName"
trigger="dblclick"
/>
</h4>
<h4 v-else>{{ nodeModel.properties.stepName }}</h4>
<h4 class="ellipsis-1 break-all">{{ nodeModel.properties.stepName }}</h4>
</div>

<div @mousemove.stop @mousedown.stop @keydown.stop @click.stop>
Expand Down Expand Up @@ -70,6 +56,9 @@
</el-button>
<template #dropdown>
<el-dropdown-menu style="min-width: 80px">
<el-dropdown-item @click="renameNode" class="p-8">{{
$t('common.rename')
}}</el-dropdown-item>
<el-dropdown-item @click="copyNode" class="p-8">{{
$t('common.copy')
}}</el-dropdown-item>
Expand Down Expand Up @@ -138,6 +127,40 @@
@clickNodes="clickNodes"
/>
</el-collapse-transition>

<el-dialog
:title="$t('views.applicationWorkflow.nodeName')"
v-model="nodeNameDialogVisible"
:close-on-click-modal="false"
:close-on-press-escape="false"
:destroy-on-close="true"
append-to-body
>
<el-form label-position="top" ref="titleFormRef" :model="form">
<el-form-item
prop="title"
:rules="[
{
required: true,
message: $t('common.inputPlaceholder'),
trigger: 'blur'
}
]"
>
<el-input v-model="form.title" @blur="form.title = form.title.trim()" />
</el-form-item>
</el-form>
<template #footer>
<span class="dialog-footer">
<el-button @click.prevent="nodeNameDialogVisible = false">
{{ $t('common.cancel') }}
</el-button>
<el-button type="primary" @click="editName(titleFormRef)">
{{ $t('common.save') }}
</el-button>
</span>
</template>
</el-dialog>
</div>
</template>
<script setup lang="ts">
Expand All @@ -149,6 +172,7 @@ import { iconComponent } from '../icons/utils'
import { copyClick } from '@/utils/clipboard'
import { WorkflowType } from '@/enums/workflow'
import { MsgError, MsgConfirm } from '@/utils/message'
import type { FormInstance } from 'element-plus'
import { t } from '@/locales'
const {
params: { id }
Expand All @@ -165,6 +189,11 @@ const height = ref<{
})
const showAnchor = ref<boolean>(false)
const anchorData = ref<any>()
const titleFormRef = ref()
const nodeNameDialogVisible = ref<boolean>(false)
const form = ref<any>({
title: ''
})

const condition = computed({
set: (v) => {
Expand All @@ -190,6 +219,7 @@ const showNode = computed({
return true
}
})

const handleWheel = (event: any) => {
const isCombinationKeyPressed = event.ctrlKey || event.metaKey
if (!isCombinationKeyPressed) {
Expand All @@ -202,19 +232,30 @@ const node_status = computed(() => {
}
return 200
})
function editName(val: string) {
if (val.trim() && val.trim() !== props.nodeModel.properties.stepName) {
if (
!props.nodeModel.graphModel.nodes?.some(
(node: any) => node.properties.stepName === val.trim()
)
) {
set(props.nodeModel.properties, 'stepName', val.trim())
} else {
MsgError(t('views.applicationWorkflow.tip.repeatedNodeError'))

function renameNode() {
form.value.title = props.nodeModel.properties.stepName
nodeNameDialogVisible.value = true
}
const editName = async (formEl: FormInstance | undefined) => {
if (!formEl) return
await formEl.validate((valid) => {
if (valid) {
if (
!props.nodeModel.graphModel.nodes?.some(
(node: any) => node.properties.stepName === form.value.title
)
) {
set(props.nodeModel.properties, 'stepName', form.value.title)
nodeNameDialogVisible.value = false
formEl.resetFields()
} else {
MsgError(t('views.applicationWorkflow.tip.repeatedNodeError'))
}
}
}
})
}

const mousedown = () => {
props.nodeModel.graphModel.clearSelectElements()
set(props.nodeModel, 'isSelected', true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code looks mostly clean and well-structured. However, there are a few points of concern that could be addressed:

Code Improvement Suggestions

  1. Dynamic max-width Value:

    • The current usage of calc(100% - 85px) for both :style bindings on <div> elements seems redundant given their layout arrangement.

  2. Event Handling Redundancy:

    • In some places, multiple events like @mousemove.stop, @mousedown.stop, @keydown.stop, and @click.stop are added to an element without any logic behind them besides stopping propagation. This might lead to unused DOM event handlers.

  3. Dialog Enhancements:

    • Consider adding more validation rules or UI enhancements for the node renaming dialog such as allowing only alphanumeric characters with underscores (or other allowed special symbols).
    • Ensure proper focus handling in the dialog fields when it opens.

  4. Locale Translation Usage:

    • Use $t('common.rename') consistently throughout the component.
    • Ensure $t(...) calls support the correct context by passing the necessary variables if needed.

  5. Form Validation:

    • Add input masking capabilities to the input field to restrict valid characters based on the workflow type.

  6. Code Comments Improvements:

    • Provide detailed comments explaining complex logical flows or why particular choices were made.

Below is the revised version of the relevant section with some optimizations.

<template>
 <!-- ... -->
<div v-resize="resizeStepContainer">
 <div class="flex-between">
  <div class="flex align-center">
   <component :is="iconComponent(`${nodeModel.type}-icon`)"/>
   <h4 class="ellipsis-1 break-all">{{ nodeModel.properties.stepName }}</h4>
  </div>

  <div @mousedown="handleMouseDown"/>
</div>
<!-- ... -->

<script setup lang="ts">
import { iconComponent } from './icons/utils'
// ...

const handleMouseDown = () => {
 props.nodeModel.graphModel.clearSelectElements();
 set(props.nodeModel, 'isSelected', true);
}

<!-- ... -->

<el-dialog
 :title="$t('views.applicationWorkflow.nodeName')"
 v-model="nodeNameDialogVisible"
 :close-on-click-modal="false"
 :close-on-press-escape="false"
 :destroy-on-close="true"
 append-to-body
>
 <el-form label-position="top" ref="titleFormRef" :model="form">
  <el-form-item
   prop="title"
   :rules="[
    {
     validator: validateTitle,
     trigger: ['input', 'blur']
    }
   ]"
  >
   <el-input v-model="form.title" @blur="form.title = form.title.trim()" maxlength="255"/>
  </el-form-item>
 </el-form>
 <template #footer>
  <span class="dialog-footer">
   <el-button @click.prevent="nodeNameDialogVisible = false">{{ $t('common.cancel') }}</el-button>
   <el-button type="primary" @click="handleSave">{{ $t('common.save') }}</el-button>
  </span>
 </template>
</el-dialog>
</script>

By applying these improvements, the code becomes cleaner and more robust.

Expand Down
149 changes: 77 additions & 72 deletions ui/src/workflow/nodes/variable-assign-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
<div class="flex-between">
<div>
<span
>{{ $t('views.applicationWorkflow.nodes.variableAssignNode.assign')
>{{ $t('views.applicationWorkflow.nodes.variableAssignNode.assign')
}}<span class="danger">*</span></span
>
</div>
Expand All @@ -60,85 +60,90 @@
</el-select>
</div>
</template>
</el-form-item>
<div v-if="item.source === 'custom'" class="flex">
<el-row :gutter="8">
<el-col :span="8">
<el-select v-model="item.type" style="width: 130px;">
<el-option v-for="item in typeOptions" :key="item" :label="item"
:value="item" />
</el-select>
</el-col>
<el-col :span="16">
<el-form-item v-if="item.type === 'string'"
:prop="'variable_list.' + index + '.value'"
:rules="{
message: t('dynamicsForm.tip.requiredMessage'),
trigger: 'blur',
required: true
}"
>
<el-input
class="ml-4"
v-model="item.value"
:placeholder="$t('common.inputPlaceholder')"
show-word-limit
clearable
@wheel="wheel"
></el-input>
</el-form-item>
<el-form-item v-else-if="item.type ==='num'"
:prop="'variable_list.' + index + '.value'"
:rules="{
message: t('dynamicsForm.tip.requiredMessage'),
trigger: 'blur',
required: true
}"
>
<el-input-number
class="ml-4"
v-model="item.value"
></el-input-number>
</el-form-item>
<el-form-item v-else-if="item.type === 'json'"
:prop="'variable_list.' + index + '.value'"
:rules="[{
message: t('dynamicsForm.tip.requiredMessage'),
trigger: 'blur',
required: true
},
{
validator: (rule:any, value:any, callback:any) => {
try {
JSON.parse(value);
callback(); // Valid JSON
} catch (e) {
callback(new Error('Invalid JSON format'));
}
},
trigger: 'blur',
}]"
>
<el-input
class="ml-4"
v-model="item.value"
:placeholder="$t('common.inputPlaceholder')"
type="textarea"
></el-input>
</el-form-item>
</el-col>
</el-row>
</div>
<el-form-item v-else>
<div v-if="item.source === 'custom'" class="flex">
<el-row :gutter="8">
<el-col :span="8">
<el-select v-model="item.type" style="width: 85px">
<el-option
v-for="item in typeOptions"
:key="item"
:label="item"
:value="item"
/>
</el-select>
</el-col>
<el-col :span="16">
<el-form-item
v-if="item.type === 'string'"
:prop="'variable_list.' + index + '.value'"
:rules="{
message: t('dynamicsForm.tip.requiredMessage'),
trigger: 'blur',
required: true
}"
>
<el-input
class="ml-4"
v-model="item.value"
:placeholder="$t('common.inputPlaceholder')"
show-word-limit
clearable
@wheel="wheel"
></el-input>
</el-form-item>
<el-form-item
v-else-if="item.type === 'num'"
:prop="'variable_list.' + index + '.value'"
:rules="{
message: $t('common.inputPlaceholder'),
trigger: 'blur',
required: true
}"
>
<el-input-number class="ml-4" v-model="item.value"></el-input-number>
</el-form-item>
<el-form-item
v-else-if="item.type === 'json'"
:prop="'variable_list.' + index + '.value'"
:rules="[
{
message: $t('common.inputPlaceholder'),
trigger: 'blur',
required: true
},
{
validator: (rule: any, value: any, callback: any) => {
try {
JSON.parse(value)
callback() // Valid JSON
} catch (e) {
callback(new Error('Invalid JSON format'))
}
},
trigger: 'blur'
}
]"
>
<el-input
class="ml-4"
v-model="item.value"
:placeholder="$t('common.inputPlaceholder')"
type="textarea"
autosize
></el-input>
</el-form-item>
</el-col>
</el-row>
</div>
<NodeCascader
v-else
ref="nodeCascaderRef2"
:nodeModel="nodeModel"
class="w-full"
:placeholder="$t('views.applicationWorkflow.variable.placeholder')"
v-model="item.reference"
/>
</el-form-item>

</el-card>
</template>
<el-button link type="primary" @click="addVariable">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code snippet appears to be modifying Vue.js form data and rendering it using Element UI components within a dynamic workflow application. Some potential improvements or corrections could include:

Potential Issues:

  1. Translation Strings: The $t function is used for translations without checking if t is defined, which can lead to runtime errors.

  2. Autosizing Input:

    • Replace autosize with {minRows: 3} in the JSON input item, as autosize does not take an argument.
    <el-input
  class="ml-4"
  v-model="item.value"
  :placeholder="$t('common.inputPlaceholder')"
  type="textarea"
  autosize={{ minRows: 3 }}
></el-input>

  3. Conditional Rendering:

    • Ensure that the condition checks correctly between 'custom' and other cases. It seems like one case might need reevaluation.

  4. Type Options:

    • Use a more descriptive placeholder for string values instead of "common.inputPlaceholder" for clarity.

Optimizations:

  • Inline Styling: Keep styles inline where possible to reduce unnecessary CSS imports.
  • Event Listeners: Remove redundant event listeners; only necessary ones should be attached.
  • Validation Messages: Provide more informative validation messages depending on the context.

Here’s the revised version of the code snippet incorporating these suggestions:

<!-- ... template content ... -->

<template #default="{ index, nodeModel }">
  <el-cascader
    v-else
    ref="nodeCascaderRef2"
    :nodeModel="nodeModel"
    class="w-full"
    :placeholder="$t('views.applicationWorkflow.variable.placeholder', { count: item.type === 'json' ? undefined : item.options.length })"
    v-model="item.reference"
  />

</template>

<!-- ... script content ... -->
<script setup>
// ... component logic ...

function addVariable() {
  // Add variable logic here
}

// Example usage based on your existing conditions
let item;
const typeOptions = ['string', 'num', 'json'];
const defaultItemValue;

if (item) {
  let newItem;
  
  if (item.source === 'builtIn') {
    if (!newItem['name']) newItem['name'] = '';
    if (!newItem['description']) newItem['description'] = '';
    if (typeof newItem['type'] !== "undefined") {
      newItem['type'] = this.$clone(item["type"]);
    }
  } else if (item.source === 'constant') {
    newItem = {};
  } else if (item.source === 'custom') {
    newItem = {
      reference: [],
    };
    
    if (typeof item.type !== "undefined") {
      newItem.type = this.$clone(item["type"]);
      newItem.value = "";
      
      switch (newItem.type) {
        case "string":
          delete newItem.description;
          break;
        case "json":
          newItem.description = "";
          break;
        case "num":
          newItem.description = "";
          break;
      }

      if (!newItem.value) newItem.value = defaultItemValue[newItem.type];
    }
  }
}
</script>

<style scoped>
/* Inline styling */
.flex-between .el-form-item--medium .el-form-label,
.el-form-item--large .el-form-label {
  display: flex;
  align-items: center;
}

.danger span {
  color: red;
}
</style>

<!-- ... additional scripts/imports ... -->

Review the updated changes carefully before implementing them in your project to ensure they meet all requirements and do not have further unforeseen side effects or issues.

Expand Down