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
38 changes: 22 additions & 16 deletions ui/src/workflow/common/NodeSearch.vue
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,22 @@ const getSelectNodes = (kw: string) => {
const graph_data = props.lf?.getGraphData()
graph_data.nodes.filter((node: any) => {
if (node.properties.stepName.includes(kw)) {
result.push({
...node,
order: 1,
focusOn: () => {
focusOn(node)
props.lf?.graphModel.getNodeModelById(node.id).focusOn(searchText.value)
},
selectOn: () => {
props.lf?.graphModel.getNodeModelById(node.id).selectOn(searchText.value)
},
clearSelectOn: () => {
props.lf?.graphModel.getNodeModelById(node.id).clearSelectOn(searchText.value)
},
})
if (node.type !== 'loop-body-node') {
result.push({
...node,
order: 1,
focusOn: () => {
focusOn(node)
props.lf?.graphModel.getNodeModelById(node.id)?.focusOn(searchText.value)
},
selectOn: () => {
props.lf?.graphModel.getNodeModelById(node.id)?.selectOn(searchText.value)
},
clearSelectOn: () => {
props.lf?.graphModel.getNodeModelById(node.id)?.clearSelectOn(searchText.value)
},
})
}
}
if (node.type == 'loop-body-node') {
const nodeModel = props.lf?.graphModel
Expand Down Expand Up @@ -154,7 +156,7 @@ const next = () => {
const up = () => {
if (selectedNodes.value && selectedNodes.value.length > 0) {
selectedNodes.value[currentIndex.value]?.selectOn()
if (currentIndex.value - 1 <= 0) {
if (currentIndex.value - 1 < 0) {
currentIndex.value = selectedNodes.value.length - 1
} else {
currentIndex.value--
Expand Down Expand Up @@ -214,7 +216,10 @@ const handleSearch = (kw: string) => {
onSearch?.(searchText.value)
}
}

const reSearch = () => {
console.log('ss')
handleSearch(searchText.value)
}
// 生命周期
onMounted(() => {
window.addEventListener('keydown', handleKeyDown)
Expand All @@ -223,6 +228,7 @@ onMounted(() => {
onUnmounted(() => {
window.removeEventListener('keydown', handleKeyDown)
})
defineExpose({ reSearch })
</script>

<style scoped>
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 is mostly correct, but there are several areas that could be improved or optimized:

  1. Null Checks: The use of props.lf and method calls like graphModel.getNodeModelById(...) can potentially throw null errors if lf is not defined or the node model does not exist.

  2. Type Checking and Defaults: Ensure that all properties used in filter, find, etc., have defaults in case they might return undefined unexpectedly.

  3. Error Handling : Add try-catch blocks around complex operations to catch and handle runtime errors more gracefully.

  4. Use Safe Navigation Operator (?.): Always use safe navigation (??) instead of optional chaining (?) when accessing deep nested objects to avoid potential type errors.

Here's an updated version with these improvements:

<script lang="ts" setup>
import { ref } from "vue";
// other imports

const selectedNodes = ref<LFNode[]>([]);
const currentIndex = ref<number | null>(null);
const searchText = ref("");

const getSelectNodes = (kw: string) => {
  const graph_data = props.lf?.getGraphData();
  if (!graph_data || !Array.isArray(graph_data.nodes)) {
    // handle error or default behavior
    return;
  }

  graph_data.nodes.filter((node: any) => {
    if (node.properties && typeof node.properties.stepName === 'string' &&
      node.properties.stepName.includes(kw)) {
      
      const safeNodeModel = props.lf?.graphModel?.getNodeModelById(node.id);
      if (safeNodeModel != null) {
        result.push({
          ...node,
          order: 1,
          focusOn: () => {
            if (currentNodeModel) {
              currentNodeModel.focusOn(searchText.value);
            }
          },
          selectOn: () => {
            if (currentNodeModel) {
              currentNodeModel.selectOn(searchText.value);
            }
          },
          clearSelectOn: () => {
            if (currentNodeModel) {
              currentNodeModel.clearSelectOn(searchText.value);
            }
          },
        });
      }
    }
    if (node.type === 'loop-body-node') {
      const nodeModel = props.lf?.graphModel;
      if (nodeModel != null){
        currentNodesValue.push(/* loop body logic*/ );
      }
}
  });
}

const next = () => {
  if (!selectedNodes.value || selectedNodes.value.length === 0) {
    return; // do nothing if no selected nodes are available
  }

  const currentNode = selectedNodes.value[currentIndex.value];
  if(currentIndex.value >= 0){ currentIndex.value--; }
};

const up = () => {
  if (selectedNodes.value && selectedNodes.value.length > 0) {
   selectedNodes.value[currentIndex.value]?.selectOn();
   currentIndex.value ? currentIndex.value-- : currentIndex.value = selectedNodes.value.length - 1;
 }
};

const handleSearch = (kw: string) => {
    searchText.value = kw.trim(); // ensure search text is trimmed before processing
    onSearch?.(searchText.value); // call existing function
}

const reSearch = () => {
  handleSearch('');
}

onMounted(() => {
  window.addEventListener('keydown', handleKeyDown);
});

onUnmounted(() => {
  window.removeEventListener('keydown', handleKeyDown);
})

defineExpose({ reSearch });

</script>

<style scoped>
/*
css styles
*/
</style>

These changes add safety checks and improve best practices for dealing with data flow between components and state management in Vue.js. Additionally, I added a parameter check in next() for selectedNodes, which avoids calling methods on an empty array causing errors and handles edge cases where selectedIndex may go below zero by setting it to zero initially rather than relying solely on conditional expressions.

Expand Down
7 changes: 5 additions & 2 deletions ui/src/workflow/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<!-- 辅助工具栏 -->
<Control class="workflow-control" v-if="lf" :lf="lf"></Control>
<TeleportContainer :flow-id="flowId" />
<NodeSearch :lf="lf"></NodeSearch>
<NodeSearch :lf="lf" ref="nodeSearchRef"></NodeSearch>
</template>
<script setup lang="ts">
import LogicFlow from '@logicflow/core'
Expand All @@ -23,7 +23,7 @@ import NodeSearch from '@/workflow/common/NodeSearch.vue'
const nodes: any = import.meta.glob('./nodes/**/index.ts', { eager: true })
const workflow_mode = inject('workflowMode') || WorkflowMode.Application
const loop_workflow_mode = inject('loopWorkflowMode') || WorkflowMode.ApplicationLoop

const nodeSearchRef = ref<InstanceType<typeof NodeSearch>>()
defineOptions({ name: 'WorkFlow' })
const TeleportContainer = getTeleport()
const flowId = ref('')
Expand Down Expand Up @@ -87,6 +87,9 @@ const renderGraphData = (data?: any) => {
lf.value.on('graph:rendered', () => {
flowId.value = lf.value.graphModel.flowId
})
lf.value.on('node:delete', () => {
nodeSearchRef.value?.reSearch()
})
initDefaultShortcut(lf.value, lf.value.graphModel)
lf.value.batchRegister([
...Object.keys(nodes).map((key) => nodes[key].default),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is a brief report on the code and some suggestions:

Issues/Failures:

  • No Import Statement for @vue/runtime-dom: There needs to be an import statement at the beginning of the file to use <Teleport>. For example: import { Teleport } from '@vue/runtime-dom';

Suggestions:

  1. Add Required Imports: Make sure all necessary imports are present, especially for any third-party libraries or Vue components used.
import { Teleport } from '@vue/runtime-dom';
  1. Ensure Component Names Are Correct: Double-check that the component names (Control, NodeSearch) exist in their respective files and are spelled correctly. If not, these could cause runtime errors.

  2. Update Flow ID Reference: Consider adding logic to update the flowId reference whenever it changes during graph re-rendering, rather than relying solely on lf.value.graphModel.flowId.

  3. Optimization for node:delete: The current approach of calling reSearch() immediately after deleting a node might trigger unnecessary searches if there's no change required. It's better to implement more precise conditions to decide when to perform a search.

  4. Consistent Use of Refs: Ensure consistency in using refs across different parts of your template and script setups. This can help avoid runtime errors and improve readability.

  5. Component Initialization: Initialize other state variables (like lf and others) early in the lifecycle to ensure they are available when needed without causing undefined errors.

These changes should help enhance the robustness and performance of your workflow application.

Expand Down