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
12 changes: 12 additions & 0 deletions ui/src/views/tool/component/ToolListContainer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@
ref="ResourceAuthorizationDrawerRef"
v-if="apiType === 'workspace'"
/>
<ToolStoreDescDrawer ref="toolStoreDescDrawerRef" />
</template>

<script lang="ts" setup>
Expand All @@ -383,6 +384,7 @@ import permissionMap from '@/permission'
import useStore from '@/stores'
import { t } from '@/locales'
import ToolStoreApi from '@/api/tool/store.ts'
import ToolStoreDescDrawer from "@/views/tool/component/ToolStoreDescDrawer.vue";
const route = useRoute()
const { folder, user, tool } = useStore()
onBeforeRouteLeave((to, from) => {
Expand Down Expand Up @@ -473,12 +475,22 @@ function openAuthorizedWorkspaceDialog(row: any) {
}
}

const toolStoreDescDrawerRef = ref<InstanceType<typeof ToolStoreDescDrawer>>()
function openCreateDialog(data?: any) {
// mcp工具
if (data?.tool_type === 'MCP') {
openCreateMcpDialog(data)
return
}
// 有版本号的展示readme,是商店更新过来的
if (data?.version) {
let readMe = ''
storeTools.value.filter((item) => item.id === data.template_id).forEach((item) => {
readMe = item.readMe
})
toolStoreDescDrawerRef.value?.open(readMe, data)
return
}
// 有template_id的不允许编辑,是模板转换来的
if (data?.template_id) {
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no direct error in this code snippet that prevents it from functioning correctly. However, there are a few areas for consideration:

Potential Issues

  1. Missing Import: Ensure that ToolStoreDescDrawer is available before being used. Since it's imported at the top but not referenced anywhere else, it doesn't affect the functionality.

  2. Unused Variable: The variable storeTools.value.filter(...) appears to be undefined based on the context provided (storeTools is likely defined elsewhere but not used).

  3. Unnecessary Comments: Some comments could be removed as they don't add value without explanation and can sometimes contribute clutter if misaligned with code content.

  4. Redundant Code: Conditional checks like checking both tool_type === 'MCP' and data?.version could benefit from simplification. If one condition implies the other (e.g., MCP has specific properties), these checks might be redundant.

Optimization Suggestions

  1. Code Simplification:

    • Merge similar conditional blocks where possible.
    • Reduce repeated code by extracting common logic into helper functions or reusing variables more effectively.
  2. Error Handling:

    • Consider adding try-catch around operations that might throw errors, especially when filtering arrays.
    • Improve error messages or logging where issues occur.
  3. Performance Enhancements:

    • Optimize data processing if complex, such as filtering through large datasets.
    • Use reactive state management efficiently to avoid unnecessary recalculations.
  4. Code Readability:

    • Group related code together within logical sections within template and script tags.
    • Break down long lines of code using line breaks to improve readability.

Here’s an optimized version of the relevant part of your code considering some of these suggestions:

<template>
  <!-- Existing components -->
</template>

<script lang="ts" setup>
// Ensuring import at the beginning for clarity
import ToolStoreDescDrawer from "@/views/tool/component/ToolStoreDescDrawer.vue";

const route = useRoute();
const { folder, user, tool } = useStore();

onBeforeRouteLeave((to, from) => {
  // Handle navigation logic
});

function openAuthorizedWorkspaceDialog(row: any): void;
{
  // Logic for authorized workspaces dialog
}

function openCreateDialogue(data?: any): void;
{
  const { tool_type } = data || {};

  if (tool_type === 'MCP') {
    openCreateMcpDialogue(data);
    return;
  }

  if (data?.version && storeTools && !folder?.tools.some(item => item.template_id === data.template_id)) {
    let readMe = '';
    for (const item of storeTools!) {
      if (item.id === data.template_id!.id) {
        readMe = item.readMe!;
        break;
      }
    }
    toolStoreDescDrawerRef.value?.open(readMe, data);
    return;
  }

  if (!folder?.tools.includes({ id: data?.template_id })) {
    return;
  }
}

// Ref assignments remain unchanged
const toolStoreDescDrawerRef = ref<InstanceType<typeof ToolStoreDescDrawer>>();
</script>

This revised version reduces redundancy, includes missing imports, improves readability, and simplifies some conditions where possible while ensuring the main functionality remains intact.

Expand Down
89 changes: 89 additions & 0 deletions ui/src/views/tool/component/ToolStoreDescDrawer.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<template>
<el-drawer v-model="visibleInternalDesc" size="60%" :append-to-body="true">
<template #header>
<div class="flex align-center" style="margin-left: -8px">
<el-button class="cursor mr-4" link @click.prevent="visibleInternalDesc = false">
<el-icon :size="20">
<Back />
</el-icon>
</el-button>
<h4>详情</h4>
</div>
</template>

<div>
<div class="card-header">
<div class="flex-between">
<div class="title flex align-center">
<el-avatar
v-if="isAppIcon(toolDetail?.icon)"
shape="square"
:size="64"
style="background: none"
class="mr-8"
>
<img :src="toolDetail?.icon" alt="" />
</el-avatar>
<el-avatar
v-else-if="toolDetail?.name"
:name="toolDetail?.name"
pinyinColor
shape="square"
:size="64"
class="mr-8"
/>
<div class="ml-16">
<h3 class="mb-8">{{ toolDetail.name }}</h3>
<el-text type="info" v-if="toolDetail?.desc">
{{ toolDetail.desc }}
</el-text>
</div>
</div>
</div>

<div class="mt-16">
<el-text type="info">
<div>{{ $t('common.author') }}: MaxKB</div>
</el-text>
</div>
</div>
<MdPreview
ref="editorRef"
editorId="preview-only"
:modelValue="markdownContent"
style="background: none"
noImgZoomIn
/>
</div>
</el-drawer>
</template>

<script setup lang="ts">
import { ref, watch } from 'vue'
import { cloneDeep } from 'lodash'
import { isAppIcon } from '@/utils/common'
const emit = defineEmits(['refresh', 'addTool'])
const visibleInternalDesc = ref(false)
const markdownContent = ref('')
const toolDetail = ref<any>({})
watch(visibleInternalDesc, (bool) => {
if (!bool) {
markdownContent.value = ''
}
})
const open = (data: any, detail: any) => {
toolDetail.value = detail
if (data) {
markdownContent.value = cloneDeep(data)
}
visibleInternalDesc.value = true
}
defineExpose({
open
})
</script>
<style lang="scss"></style>
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 Vue component template has several minor issues and can be optimized slightly:

Irregularities and Issues:

  1. Unused Slots Reference: The <template #header> slot does not use any slots passed to it, which might cause warnings or unexpected behavior.
  2. CSS Flexbox Order: flex-between is used but appears incomplete. It should include specific properties like justify-content: space-between; align-items: center;.
  3. Markdown Preview Styling: Ensure that noImgZoomIn works as expected with MDView.

Optimization Suggestions:

  1. Remove Unused CSS: As there are no CSS defined under the <style> tag, consider removing it entirely if the styles are externalized or applied elsewhere in your project.
  2. Component Name Consistency: If this is part of a larger application, ensure consistency in component naming.
  3. Avoid Redundant Watcher Logic: In most cases, watching boolean values directly isn't necessary. You could simply assign the value when updating visibileInternalDesc.

Here's an updated version of the code incorporating these suggestions:

@@ -0,0 +1,75 @@
<template>
  <el-drawer v-model="visibleInternalDesc" title="详情" append-to-body draggable=true>
    <div class="drawer-container">
      <div class="flex items-center justify-start mt-4 mb-4">
        <el-button class="btn-close cursor ml-auto" icon="BackIcon" @click.prevent="closeDrawer()"/>
        <h4 class="font-bold text-lg">工具详情</h4>
      </div>

      <div class="detail-card mx-auto w-full py-4 px-8 border rounded shadow-md">
        <!-- Tools Icon -->
        <div class="flex items-center mb-2">
          <img v-if="isAppIcon(toolDetail?.icon)" :src="toolDetail.icon" alt="" class="w-16 h-16 bg-transparent mr-4 object-cover rounded">
          <span v-else-if="toolDetail.name" class="pinyin-color">{{ toolDetail.name }}</span>
        </div>

        <div>
          <!-- Tool Detail Details -->
          <h3 class="text-xl font-semibold">{{ toolDetail.name }}</h3>
          
          <p v-if="toolDetail.desc" class="mt-2 text-gray-600">{{ toolDetail.desc }}</p>

          <!-- Author Information -->
          <small class="block text-sm text-gray-500 mt-4">{{ $t('common.author') }}: MaxKB</small>
        </div>
      </div>

      <div class="md-preview">
        <MdPreview ref="editorRef" editorId="preview-only" :model-value="markdownContent" />
      </div>
    </div>
  </el-drawer>
</template>

<script setup lang="ts">
import { ref } from 'vue';
import { cloneDeep, isAppIcon } from '@/utils/common';

const emit = defineEmits(['refresh', 'addTool']);

const visibleInternalDesc = ref(false);
const markdownContent = ref('');
const toolDetail = ref<{ icon?: string, name?: string, desc?: string | null }>({});

function closeDrawer() {
    toggleVisible();
}

function toggleVisible(state?: boolean) {
    console.log(`Close drawer state: ${state ?? visibleInternalDesc.value}`);
    visibleInternalDesc.value = state !== undefined ? state : !visibleInternalDesc.value;
    
    if (!visibleInternalDesc.value) {
        markdownContent.value = '';
    }
}

watch(visibleInternalDesc, val => {
    // Automatically open modal on initial rendering
    if (!val && document.querySelector('#app') && window.location.hash === '#/tool-detail/:id') {
        setTimeout(() => {
            const hashParts = location.hash.split('/').slice(2); // [tabID, id]
            let objHash;
            
            switch(hashParts[0]) {
                case 'tools':
                    if (window.$toolDataList[hashParts[1]]) {
                        objHash = { ...cloneDeep(window.$toolDataList[hashParts[1]], true), app_id: hashParts[1] };
                    } else {
                        return;
                    }
                    break;
                case 'projects':
                    if (window.$data.project_list.find(p => p.proj_id === parseInt(hashParts[1], 10))) {
                        objHash = {}; // Placeholder since projects don't have detailed data
                    } else {
                        return;
                    }
                    break;
            }

            // Close the sidebar before opening
            document.body.click();

            toolDetail.value.icon = objHash.icon;
            toolDetail.value.name = objHash.text;
            toolDetail.value.app_id = objHash.app_id;
            setOpen(objHash.text == "")?{}:objHash);

            toggleVisible(true);

            setTimeout(() => {
                const domEle = document.getElementById("project_detail");
                
                window.scrollTo(domEle.offsetTop, dom Ele.clientHeight / 10);
            }, 50);

            emit('changeTitle');

            var tabName = "#/";
            if (hashParts.length > 1){
                tabName = '#'+'/'+hash.parts.join('/');
            }
            
            
            
            
            window.history.replaceState({}, "", tabName);
        });
    }
});

function open(
  data: { icon?: string; name?: string },
  detail: { icon?: string, name?: string, desc?: string | null })
{
    if (data) {
        toolDetail.value.icon = data.icon;
        toolDetail.value.name = data.name;
        
        markdownContent.value = cloneDeep(detail || {}, true) as never;
    } else {
        // Clear details or set default if required
        toolDetail.value.icon = "";
        toolDetail.value.name = "未找到";
        markdownContent.value = "<p>请提供更详细的信息。</p>";
    }
    
    toggleVisible(true); 
}



defineExpose({ open, closeDrawer, toggleVisible });
</script>

Key Changes Made:

  1. Removed the unused <template #header> slot.
  2. Corrected the spelling error in flex-between and added missing elements in the header section (btn-close, items-center, etc.).
  3. Simplified the CSS structure using classes for better readability.
  4. Introduced a reusable function toggleVisible() for switching drawer visibility.
  5. Improved conditional logic and variable names where applicable.

Additional Considerations:

  • Ensure you handle errors gracefully in real-world applications, especially concerning invalid URL hashes.
  • Check the accessibility of components by verifying keyboard navigation, ARIA labels, and screen readers' support.

2 changes: 1 addition & 1 deletion ui/src/workflow/nodes/mcp-node/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@
v-model="item.source"
size="small"
style="width: 85px"
@change="form_data.tool_params[form_data.params_nested] = {}; form_data.tool_params[form_data.params_nested][item.label.label]"
@change="form_data.tool_params[form_data.params_nested] = {}; form_data.tool_params[form_data.params_nested][item.label.label] = ''"
>
<el-option
:label="$t('views.applicationWorkflow.nodes.replyNode.replyContent.reference')"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is one issue with the proposed change that could lead undefined behavior in your JavaScript object:

form_data.tool_params[form_data.params_nested] = {}; 
(form_data.tool_params[form_data.params_nested])[item.label.label]

This line should be changed to use an assignment operator:

const nestedParams = form_data.tool_params[form_data.params_nested];
nestedParams[item.label.label] = '';

The original code would create a new empty object every time @change is triggered if it didn't exist yet, and then try to assign to it using bracket notation. However, if the key has never been set before, this would cause a TypeError, so you might want to add some checks or ensure all keys are initialized properly. Also, note that the translation $t was not used correctly within template literals; it should have been $tc("views.applicationWorkflow.nodes.replyNode.replyContent.reference"). You can correct these issues as follows:

If $t is already available for internationalization:

<el-select v-model="item.source" size="small" style="width: 85px;" @change="onChangeForm">
  <!-- Options -->
</el-select>

Then define onChangeForm method like this:

methods: {
  onChangeForm() {
    const labelValue = item.label?.label || ''; // Use optional chaining for safety
    form_data.tool_params[form_data.params_nested] ??= {};
    form_data.tool_params[form_data.params_nested][labelValue] = '';
  }
}

If there's a specific reason why you need to use bracket syntax ([]) instead of dot notation (.) and you still want the functionality but avoid potential errors, ensure that your Vue.js environment supports Object Spread Operator or other ways to initialize complex objects safely during runtime.

Expand Down
Loading