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
47 changes: 24 additions & 23 deletions ui/src/workflow/nodes/loop-body-node/LoopBodyContainer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
:class="{ isSelected: props.nodeModel.isSelected, error: node_status !== 200 }"
style="overflow: visible; background: #fff"
>
<div v-resize="resizeStepContainer">
<div>
<div class="flex-between">
<div class="flex align-center">
<component
Expand All @@ -17,14 +17,14 @@
<h4 class="ellipsis-1 break-all">{{ nodeModel.properties.stepName }}</h4>
</div>
<!-- 放大缩小按钮 -->
<!-- <el-button link @click="enlargeHandle">
<el-button link @click="enlargeHandle">
<AppIcon
:iconName="enlarge ? 'app-minify' : 'app-magnify'"
class="color-secondary"
style="font-size: 20px"
>
</AppIcon>
</el-button> -->
</el-button>
</div>
<el-collapse-transition>
<div @mousedown.stop @keydown.stop @click.stop v-show="showNode" class="mt-16">
Expand All @@ -40,7 +40,10 @@
show-icon
:closable="false"
/>
<slot></slot>
<div :style="`height:${height}px`">
<slot></slot>
</div>

<template v-if="nodeFields.length > 0">
<h5 class="title-decoration-1 mb-8 mt-8">
{{ $t('common.param.outputParam') }}
Expand Down Expand Up @@ -115,16 +118,8 @@ import { ElMessage } from 'element-plus'
import type { FormInstance } from 'element-plus'
import { t } from '@/locales'
import { WorkflowMode } from '@/enums/application'

provide('workflowMode', WorkflowMode.ApplicationLoop)
const height = ref<{
stepContainerHeight: number
inputContainerHeight: number
outputContainerHeight: number
}>({
stepContainerHeight: 0,
inputContainerHeight: 0,
outputContainerHeight: 0,
})

const titleFormRef = ref()
const nodeNameDialogVisible = ref<boolean>(false)
Expand Down Expand Up @@ -179,14 +174,7 @@ const mousedown = () => {
}
const showicon = ref<number | null>(null)

const resizeStepContainer = (wh: any) => {
if (wh.height) {
if (!props.nodeModel.virtual) {
height.value.stepContainerHeight = wh.height
props.nodeModel.setHeight(height.value.stepContainerHeight)
}
}
}
const height = ref<number>(600)

const props = defineProps<{
nodeModel: any
Expand All @@ -211,9 +199,22 @@ const enlarge = ref(false)
function enlargeHandle() {
enlarge.value = !enlarge.value
if (enlarge.value) {
// ...
props.nodeModel.graphModel.transformModel.focusOn(
props.nodeModel.x,
props.nodeModel.y,
props.nodeModel.width + 470,
props.nodeModel.height - 30,
)
height.value =
(props.nodeModel.graphModel.height - 100) / props.nodeModel.graphModel.transformModel.SCALE_Y
const width = window.innerWidth / props.nodeModel.graphModel.transformModel.SCALE_X
props.nodeModel.width = width
props.nodeModel.setHeight(height.value)
} else {
// ...
height.value = 600
const width = 1920
props.nodeModel.width = width
props.nodeModel.setHeight(height.value)
}
}
</script>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. The div element added at line 4 should have a closing tag (</div>).
  2. Remove the commented-out showicon ref definition since it's not used in the code snippet provided.
  3. Ensure that stepContainerHeight, inputContainerHeight, and outputContainerHeight properties of props.nodeModel.graphModel are set before using them to change height value.
  4. Consider replacing hardcoded values with dynamic calculations based on screen dimensions and node properties.
  5. Use TypeScript interfaces instead of plain objects when defining reactive prop types for better typing support.
  6. Replace the placeholder comment inside if (enlarge.value) with actual logic for animating the step container resizing effect.

Expand Down
11 changes: 11 additions & 0 deletions ui/src/workflow/nodes/loop-body-node/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ class LoopBodyModel extends AppNodeModel {

return anchors
}
setHeight(height: number) {
this.properties['height'] = height
this.outgoing.edges.forEach((edge: any) => {
// 调用自定义的更新方案
edge.updatePathByAnchor()
})
this.incoming.edges.forEach((edge: any) => {
// 调用自定义的更新方案
edge.updatePathByAnchor()
})
}
}
export default {
type: 'loop-body-node',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code looks largely valid and well-organized. It defines a LoopBodyModel class that extends an AppNodeModel. The method for setting height (setHeight) updates the node's properties and propagates the change to the relevant edges. However, there are a few minor suggestions:

  1. Type Annotations: Ensure you have appropriate TypeScript type annotations, especially for array variables like anchors, outgoing.edges, and incoming.edges.

  2. Variable Naming Consistency: While not strictly necessary, it might be helpful to maintain consistent naming conventions or documentation on what each property represents.

  3. Error Handling: Consider adding error handling if the updatePathByAnchor() functions could throw exceptions, particularly given they modify edges directly.

  4. Comments: Adding comments explaining complex logic or decisions can help future developers (or perhaps你自己 in the distant future) understand the purpose of certain parts of the codebase more easily.

Here is the updated code with some suggested improvements:

@@ -36,6 +36,18 @@ class LoopBodyModel extends AppNodeModel {
   getAnchors(): any[] {
     return this.anchors;
   }

+  setHeight(height: number): void {
+    if (!Number.isInteger(height)) {
+      throw new Error('Height must be an integer');
+    }
+
+    this.properties['height'] = height;
+    
+    this.outgoing.edges.forEach((edge: any) => {
+      edge.updatePathByAnchor();
+    });
+    
+    this.incoming.edges.forEach((edge: any) => {
+      edge.updatePathByAnchor();
+    });
+  }
}

In summary, the code itself seems functional, but improving readability and robustness would provide better overall quality and maintenance.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/workflow/nodes/loop-body-node/index.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<LoopBodyContainer :nodeModel="nodeModel">
<div ref="containerRef" @wheel.stop style="height: 600px"></div>
<div ref="containerRef" @wheel.stop style="height: 100%; width: 100%"></div>
</LoopBodyContainer>
</template>
<script setup lang="ts">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The updated code changes the width of the .containerRef to 100%. This is generally appropriate for making the container take up its full available space, especially if it's inside another element with flexible sizing.

Overall, this change seems reasonable within typical use cases, but there aren't any noticeable issues that would prevent the component from functioning correctly as intended. If you anticipate scrolling in different directions or need additional customization based on specific context, further adjustments might be necessary. Otherwise, your changes should work well.

Expand Down
Loading