feat: Workflow node search

[shaohuzhang1]

feat: Workflow node search

[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.

Details

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-sigs/prow 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.

Details

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]

Here are some corrections and improvements to your Vue.js code:

1. Type Annotations: Ensure all props are properly annotated with their types.
2. Arrow Function Usage: Use consistent arrow function notation (() => {...}) throughout the template and script setup.
3. Computed Property: The selectedCount should be defined using computed.
4. Logical Operators: Simplify logical operators in the template where possible.
5. Comments: Add comments for better readability.
6. Style Updates: Update style properties as needed.

Revised Code

<template>
  <div class="workflow-search-container flex-between">
    <el-input
      ref="searchInputRef"
      v-model:value="searchText"
      @input="handleInput"
      :placeholder="$t('workflow.tip.searchPlaceholder')"
      clearable
      @keyup.enter="next"
      @keyup.esc="closeSearch"
    ></el-input>
    <span>
      <el-space :size="4">
        <!-- Conditional display of selected count, empty search results, or "No Results" -->
        <span class="lighter">{{ showSelectedIndicator }}</span>
        <el-divider direction="vertical" />

        <el-button text click="up"><ArrowUp /></el-button>
        <el-button text click="next"><ArrowDown /></el-button>
        <el-button text @click="closeSearch()">
          <el-icon><Close /></el-icon>
        </el-button>
      </el-space>
    </span>
  </div>
</template>

<script lang="ts" setup>
import { ref, watchEffect, onMounted, onUnmounted } from 'vue';
import { ArrowUp, Close } from '@element-plus/icons-vue';

// Define props type
interface Props {
  lf?: any; // Example prop
}

const props = defineProps<Props>();
const searchText = ref('');
let currentIdx = ref(0);
let selectedNodes = ref<Array<any>>();

// Computed property to determine indicator based on search criteria
const showSelectedIndicator = computed(() =>
  currentIdx > 0 ? (selectedNodes.value ? `${currentIdx}/` + selectedNodes.value.length : '') : ''
);

// Focuses on a given node by transforming its position and focusing in graph viewmodel
async function focusOn(node: any): Promise<void> {
  
}

watchEffect(async (oldVal, newVal) => {
  if (
    (!newVal && prevVal) ||
    (newVal.trim() !== oldVal.trim()) &&
    currentIdx == 0 &&
    selectedNodes.value != undefined
  ) {
    await focusOn(newVal);
  }
});

onMounted(async () => {
  // Initialize data here if necessary
});

const handleInput = async (value: string) => {
  if (props.lf) {
    const result = getSelectNodes(value);
    selectedNodes.value = result;

    if (result && result.length > 0) {
      selectNodes(result);
      focusOn(currentIdx);
    }
  }
};

function next(): void {
  if (selectedNodes.value && selectedNodes.value.length > 0) {
    if (currentIdx < selectedNodes.value.length - 1) {
      currentIdx++;
      focusOn(currentIdx);
    } else {
      currentIdx = 0;
    }
  }
}

function up(): void {
  if (selectedNodes.value && selectedNodes.value.length > 0) {
    if (currentIdx > 0) {
      currentIdx--;
      focusOn(currentIdx);
    } else {
      currentIdx = selectedNodes.value.length - 1;
    }
  }
}

const resetState = (): void => {
  currentIdx = 0;
  focusedIdMap = {};
  if (lf.graph.viewModels) {
    lf.graph.viewModels[graphData.key]!.focusedIds.clear();
  }
};
</script>

<style scoped>
.workflow-search-container {
  /* Your styles */
}
/* Rest of your CSS */
</style>

Changes Made:

1. Corrected type annotations for props.
2. Changed arrow functions in the template to match Vue's syntax.
3. Defined showSelectedIndicator as a computed property for cleaner logic.
4. Added comments for clarity.
5. Updated style properties where necessary without significant changes.

This revised version should address several issues found in the original code, making it more maintainable and robust. Make sure to update the rest of your components accordingly.

[shaohuzhang1]

Code Review

1. Logical Flow Initialization

<Control class="workflow-control" v-if="lf" :lf="lf"></Control>

There's no initialization of lf inside the <script setup> section.

Suggestion:
Add the initialiation line to ensure lf is not undefined before using it.

<script setup lang="ts">
import LogicFlow from '@logicflow/core'

const lf = ref(null)

// Rest of the code...
</script>

2. Node Search Functionality

The existing onSearch function attempts to iterate over all nodes and find one with a matching step name keyword. However, this implementation has several issues:

• It assumes that only one node matches the search criteria.
• If multiple nodes match, the first one selected will be lost due to resetting searchQueue.
• The focus behavior can lead to unexpected results if there are many nodes to select and they all have the same keyword.

Suggestions:

1. Update the logic to handle cases where multiple nodes might match the search criteria.
2. Adjust the focus method to prevent unintentional selection or movement.
3. Implement pagination or lazy loading to avoid performance bottlenecks with large numbers of nodes.

3. Node Importing (glob pattern)

const nodes: any = import.meta.glob('./nodes/**/index.ts', { eager: true })

This pattern imports all .ts files under directories named ./nodes. While convenient, it can increase the bundle size significantly depending on the number of files.

Suggestion:
Use dynamic imports (import(...).then(...)) instead of import.meta.glob() if you don't need to execute the imported modules immediately.

<script setup lang="ts">
import type { NodeComponent } from './types' // Assuming types exist in ./types
import * as nodeComponents from '@/components/nodes' //

const getNodes = async () => {
  const loadedNodeComponents: Record<string, NodeComponent> = {}
  await Promise.all(
    Object.entries(nodeComponents).map(async ([key, module]) => {
      try {
        const component = await module.default?.default ?? module.default
        if (component) {
          loadedNodeComponents[key] = component
        }
      } catch (err) {
        console.error(`Failed to load node ${key}`, err)
      }
    }),
  )

  return loadedNodeComponents
}

getNodes().then(nodes => {})
</script>

4. Template Errors

Replace deprecated tags:

<TeleportContainer :flow-id="flowId" />
<NodeSearch class="workflow-search" :on-search="onSearch"></NodeSearch>

with their updated versions:

<TeleportContainer :flow-id="flowId" />

<NodeSearch class="workflow-search" :lf="lf" @select-node="handleSelectNode"></NodeSearch>

Make sure to define handleSelectNode in your script.

Final Suggested Changes

1. Initialize Logic Flow Instance

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

<Control class="workflow-control" v-if="lf" :lf="lf"></Control>

<!-- ... -->
</template>

<script setup lang="ts">
import LogicFlow from '@logicflow/core';
import Control from '@/components/Common/Control.vue';

const lf = ref();
...</script>

2. Update Node Search Method

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

<NodeSearch class="workflow-search" :lf="lf" @select-node="handleSelectNode"></NodeSearch>

<!-- ... -->
</template>

<script setup lang="ts">
import logicFlowInstance from '@/path/to/logic-flow-instance'; // Replace with actual path

const handleSelectNode = (selectedNodesIds: string[], selectedNodeData: object[]) => {
  // Handle selected node actions here
};

const renderGraphDta = (data?:any) => {
  // ...
}
</script>

These changes should help make the code more robust, efficient, and maintainable. Always test after making these changes thoroughly to ensure everything functions correctly.

[shaohuzhang1]

Your code appears to be quite complex and includes several functions that seem to manipulate graph data using an unknown library (lf). However, I can make some general observations based on what you've provided:

Potential Issues and Improvements

1. Type Safety:

   • The selectOn and focusOn methods accept generic types but use them inconsistently across different parameters.

2. Readability:

   • Some function names could be clearer. For example, getSelectNodes is good for retrieving nodes with a specific keyword but lacks clarity about its purpose within this context.

3. Event Handling:

   • The way events are handled seems a bit redundant and could be simplified by directly passing necessary arguments rather than wrapping everything in another wrapper object like event.

4. Optimization Considerations:

   • Directly manipulating DOM properties such as clientWidth and clientHeight might not always reflect the actual content size if there are no layout adjustments happening at those points.
   • Ensure that all function calls are correctly bound or passed with the correct dependencies.

Suggestions for Refinement

1. Function Renaming and Clarification:

   const filterByKeyword = (nodes: any[], kw: string): any[] =>
     // Return nodes whose stepName field contains kw
     nodes.filter(node => node.properties.stepName.includes(kw));
   
   const findNodeById = (id: string): any|null =>
     // Retrieve node by id from graph data
     lf.value.getGraphData().nodes.find(node => node.id === id);
   
   const selectAndFocus = (node: any, keyword: string): void => {
     const model = findNodeById(node.id);
     if (model) {
       model.selectOn(keyword); // Select on given key word
       model.focusOn({ x: model.x, y: model.y }, { width: lf.value.container.clientWidth || 800, height: lf.value.container.clientHeight || 600 }); // Focus on center of node
     }
   };

2. Remove Event Wrapping:
   Instead of having dedicated wrapper functions (on, selectOn, etc.), bind these operations directly to event targets without unnecessary abstraction.

3. Consistent Typing:
   Use more precise type annotations where possible. For instance, instead of relying on generics for everything, ensure type safety wherever feasible.

---

These suggestions aim to make the code more readable, maintainable, and potentially more efficient by simplifying logic and improving readability while maintaining consistency throughout the codebase.