perf: Optimization of modifying node names in workflow

[wangdan-fit2cloud]

What this PR does / why we need it?

Summary of your change

Please indicate you've done the following:

• Made sure tests are passing and test coverage is added if needed.
• Made sure commit message follow the rule of Conventional Commits specification.
• Considered the docs impact and opened a new docs issue or PR with docs changes if needed.

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

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.

[shaohuzhang1]

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.

[shaohuzhang1]

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.