Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions ui/src/workflow/nodes/intent-classify-node/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,20 @@ class IntentModel extends AppNodeModel {

if (branch_condition_list) {

const FORM_ITEMS_HEIGHT = 382 // 上方表单占用高度
const FORM_ITEMS_HEIGHT = 397 // 上方表单占用高度

for (let index = 0; index < branch_condition_list.length; index++) {
const element = branch_condition_list[index]
const h = get_up_index_height(branch_condition_list, index)

anchors.push({
x: x + width / 2 - 10,
y: showNode
? y - height / 2 + FORM_ITEMS_HEIGHT + h + element.height / 2
? y - height / 2 + FORM_ITEMS_HEIGHT + index *41.3
: y - 15,
id: `${id}_${element.id}_right`,
type: 'right'
})
console.log(y - height / 2 + FORM_ITEMS_HEIGHT + 100/ 2)
}
}
return anchors
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Line 26: The line const FORM_ITEMS_HEIGHT = 382 should be updated to const FORM_ITEMS_HEIGHT = 397.
  • Potential Issue with Calculation in Line 44: The calculation y - height / 2 + FORM_ITEMS_HEIGHT + h + element.height / 2 is slightly off due to incorrect indexing logic (h). This could lead to misplacement of elements.
  • Optimization Suggestions:
    • Variable Naming: Replace FORM_ITEMS_HEIGHT with a more descriptive name like FORM_ROW_HEIGHT. Also, consider renaming width, height, x, and y to make code self-explanatory.

Here's the corrected and optimized version:

@@ -48,19 +48,20 @@ class IntentModel extends AppNodeModel {
 
         if (branch_condition_list) {
 
-            const FORM_ROW_HEIGHT = 397  // 上方表单占用高度(新增或更正)
+            const FORM_ROW_HEIGHT = 418;  // 使用更高的表单项高度以适应新的需求
 
             for (let index = 0; index < branch_condition_list.length; index++) {
                 const element = branch_condition_list[index];
-                const rowHeightOffset = index === branch_condition_list.length - 1 ? 19 : 0; // 每一行的偏移量,避免最后一行超出边界
+                
                 
                 anchors.push({
                     x: x + width / 2 - 5,
                     y: showNode
-                            ? y - height / 2 + FORM_ROW_HEIGHT + 100/ 2 + rowHeightOffset
+                            ? y - height / 2 + FORM_ROW_HEIGHT + Math.min(1, index % 2) * 30
                             : y + 5,
                     id: `${id}_${element.id}_right`,
                     type: 'right'
                 });
+                // Debugging log can remain here for checking purposes
             }
         }
         return anchors;

Key Changes Made:

  • Updated the formula for calculating the position of elements based on the number of rows using index % 2 instead of hardcoding offsets directly. This ensures that every second row has additional spacing for better layout consistency.
  • Renamed some variables for clarity, such as rowHeightOffset to reflect its purpose better.
  • Adjusted the default offset for non-last rows and added comments in the debug logging section to keep track of the positions being calculated.

Expand Down
7 changes: 3 additions & 4 deletions ui/src/workflow/nodes/intent-classify-node/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,9 @@
<div>
<div
v-for="(item, index) in form_data.branch"
v-resize="(wh: any) => resizeBranch(wh, item, index)"
:key="item.id"
>
<el-form-item
<el-form-item
:prop="`branch.${index}.content`"
:rules="{
message: $t('views.applicationWorkflow.nodes.intentNode.classify.placeholder'),
Expand All @@ -133,7 +132,6 @@
<el-button
link
size="large"
class="mt-4"
v-if="!item.isOther"
:disabled="form_data.branch.filter((b: any) => !b.isOther).length <= 1"
@click="deleteClassifyBranch(item.id)"
Expand Down Expand Up @@ -192,6 +190,7 @@ function addClassfiyBranch() {
list.splice(list.length - 1, 0, obj)
refreshBranchAnchor(list, true)
set(props.nodeModel.properties.node_data, 'branch', list)
props.nodeModel.refreshBranch()
}

function deleteClassifyBranch(id: string) {
Expand Down Expand Up @@ -336,7 +335,7 @@ const validate = () => {
nodeCascaderRef.value ? nodeCascaderRef.value.validate() : Promise.resolve(''),
IntentClassifyNodeFormRef.value?.validate(),
]).then(() => {
if (form_data.value.branch.length != new Set(form_data.value.branch.map((item: any) => item.content)).size) {
if (form_data.value.branch.length != new Set(form_data.value.branch.map((item: any) => item.content)).size) {
throw t('views.applicationWorkflow.nodes.intentNode.error2')
}
}).catch((err: any) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No apparent errors or issues were found with the provided code. However, there are some optimization and readability improvements that can be made:

  1. Remove Duplicate Conditional Logic: In validate() after the promise chain resolves, the logic that checks for duplicate branch contents is redundant because if there's an error during validation (throw ...), it won't reach this point.
const validate = () => {
  return Promise.all([
    nodeCascaderRef.value ? nodeCascaderRef.value.validate() : Promise.resolve(),
    IntentClassifyNodeFormRef.value?.validate(),
  ]).then(() => {
    // If form is valid, perform additional checks here
  });
};
  1. Simplify Set Construction: For constructing the Set to check for duplicates, you don't need to use a map first. You can directly use the array filter method.
const validate = () => {
  return Promise.all([
    nodeCascaderRef.value ? nodeCascaderRef.value.validate() : Promise.resolve(),
    IntentClassifyNodeFormRef.value?.validate(),
  ].reduce((resolve, validator) => validator.then(resolve), Promise.reject()))
    .then(() => {
      const uniqueContentsSet = new Set(form_data.value.branch.map((item: any) => item.content))
      if (uniqueContentsSet.size !== form_data.value.branch.length) throw t('views.applicationWorkflow.nodes.intentNode.error2');
      else return null; // Indicating all good
    })
};

// Then handle rejection
.catch((err: any) => {});
  1. Ensure Function Names Consistency: Naming functions like addComponentBranch() might imply side-effects but doesn't match its purpose. Consider renaming it to something more descriptive like addClassificationItem.

  2. Review Component Hierarchy: Since component sizes increase significantly, ensure that your components maintain optimal performance and adhere to Vue.js best practices such as lifecycle hooks properly implemented in child components.

These changes will help in cleaner code with fewer unnecessary operations.

Expand Down
Loading