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
4 changes: 1 addition & 3 deletions ui/src/locales/lang/en-US/views/tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default {
copyTool: 'Copy Tool',
importTool: 'Import Tool',
settingTool: 'Set Tool',
mcpConfig: 'MCP Service Config',
toolStore: {
title: 'Tool Store',
createFromToolStore: 'Create from Tool Store',
Expand Down Expand Up @@ -38,9 +39,6 @@ export default {
},

form: {
create: 'Create Tool',
createMcp: 'Create MCP',
mcpConfig: 'MCP Service Config',
toolName: {
label: 'Name',
name: 'Tool Name',
Expand Down
4 changes: 1 addition & 3 deletions ui/src/locales/lang/zh-CN/views/tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default {
copyTool: '复制工具',
importTool: '导入工具',
settingTool: '设置工具',
mcpConfig: 'MCP服务配置',
toolStore: {
title: '工具商店',
createFromToolStore: '从工具商店创建',
Expand All @@ -32,9 +33,6 @@ export default {
saveMessage: '当前的更改尚未保存,确认退出吗?',
},
form: {
create: '创建工具',
createMcp: '创建MCP',
mcpConfig: 'MCP服务配置',
toolName: {
label: '名称',
name: '工具名称',
Expand Down
4 changes: 1 addition & 3 deletions ui/src/locales/lang/zh-Hant/views/tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default {
copyTool: '複製工具',
importTool: '匯入工具',
settingTool: '設定工具',
mcpConfig: 'MCP服務配置',
toolStore: {
title: '工具商店',
createFromToolStore: '從工具商店創建',
Expand Down Expand Up @@ -35,9 +36,6 @@ export default {
confirmMessage: '停用後,引用該工具的應用在查詢時會報錯,請謹慎操作。',
},
form: {
create: '建立工具',
createMcp: '建立MCP',
mcpConfig: 'MCP服務配置',
toolName: {
label: '名稱',
name: '工具名稱',
Expand Down
52 changes: 26 additions & 26 deletions ui/src/views/tool/component/McpToolConfigDialog.vue
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
<template>
<el-dialog
:title="$t('views.tool.form.mcpConfig')"
:title="$t('views.tool.mcpConfig')"
width="600"
v-model="dialogVisible"
:close-on-click-modal="false"
:close-on-press-escape="false"
:destroy-on-close="true"
:before-close="close"
append-to-body
class="mcp-config-dialog"
>
<el-form label-width="auto" label-position="top">
<el-form-item>
Expand All @@ -16,24 +17,25 @@
v-model="mcp_servers"
rows="8"
disabled
class="config-textarea"
@mouseenter.stop="showIcon = true"
@mouseleave.stop="showIcon = false"
></el-input>
<AppIcon
iconName="app-copy"
class="copy-icon color-secondary"
@click="copyClick(mcp_servers)"
/>
<el-button circle class="copy-icon" v-if="showIcon" @click="copyClick(mcp_servers)">
<AppIcon iconName="app-copy" class="color-secondary"/>
</el-button>
</el-form-item>
</el-form>
</el-dialog>
</template>

<script setup lang="ts">
import {ref} from 'vue'
import {copyClick} from '@/utils/clipboard'

import { ref } from 'vue'
import { copyClick } from '@/utils/clipboard'

const mcp_servers = ref<string>('')
const dialogVisible = ref<boolean>(false)
const showIcon = ref<boolean>(false)

const close = () => {
dialogVisible.value = false
Expand All @@ -43,25 +45,23 @@ const open = (item: any) => {
dialogVisible.value = true
}

defineExpose({open,})
defineExpose({ open })
</script>

<style scoped lang="scss">

.copy-icon {
position: absolute;
top: 6px;
right: 8px;
font-size: 16px;
cursor: pointer;
z-index: 2;
}

.copy-icon:hover {
opacity: 0.85;
}

:deep(.el-textarea__inner) {
padding-right: 34px; // 给右上角图标留空间
.mcp-config-dialog {
.copy-icon {
position: absolute;
top: 12px;
right: 12px;
box-shadow: 0px 4px 8px 0px rgba(31, 35, 41, 0.1);
z-index: 2;
}
.config-textarea {
:deep(.el-textarea__inner) {
color: var(--el-text-color-primary);
cursor: pointer;
}
}
}
</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.

Code Review and Optimization Suggestions

General Checks:

  1. Duplicate Imports and Exports: Ensure that ref is imported only once at the beginning of the file to avoid redundancy.
  2. Unused Variables: Verify if there are any unused variables (showIcon) in the data() block which can be removed for simplicity.
  3. CSS Class Duplication: The CSS class .config-textarea duplicates some styling from within :deep(.el-textarea__inner). Remove unnecessary duplication.

Potential Issues:

  1. Template Element Names: There might be an issue with the use of template elements such as <template> not being closed properly. Ensure all tags have matching closing elements.

Optimizations:

  1. Simplify Conditional Styling: The conditional handling for whether or not to render the copy icon based on mouse enter event could be simplified using Vue directives like v-bind:class.

Revised Code:

<template>
  <el-dialog
    :title="$t('views.tool.mcpConfig')"
    width="600"
    v-model="dialogVisible"
    :close-on-click-modal="false"
    :close-on-press-escape="false"
    :destroy-on-close="true"
    :before-close="close"
    append-to-body
    class="mcp-config-dialog"
  >
    <el-form label-width="auto" label-position="top">
      <el-form-item>
        <el-input
          v-model="mcp_servers"
          rows="8"
          disabled
          class="config-textarea"
        ></el-input>
        <span
          @mouseenter.stop="handleShowIcon(true)"
          @mouseleave.stop="handleShowIcon(false)"
          class="copy-icon"
          :class="{ 'opacity-85': showIcon }"
        >
          <AppIcon iconName="app-copy" class="color-secondary"/>
        </span>
      </el-form-item>
    </el-form>
  </el-dialog>
</template>

<script setup lang="ts">
import { ref, defineExpose } from "vue";
import { copyClick } from "@/utils/clipboard";

let mcp_servers = ref("");
let dialogVisible = ref(false);
let showIcon = ref(false);

const close = () => {
  dialogVisible.value = false;
};

const open = (item: any) => {
  dialogVisible.value = true;
};

defineExpose({
  open,
});

const handleShowIcon = (flag: boolean) => {
  showIcon.value = flag;
};
</script>

<style scoped lang="scss">
$mcp-config-dialog-border-radius: 8px;

.mcp-config-dialog {
  border-radius: $mcp-config-dialog-border-radius;
  padding-bottom: 1em;
  
  .copy-icon {
    position: absolute;
    top: calc(50% - 14px); /** Adjusted for vertical alignment */
    right: 10px;
    box-shadow: none; /* Removed for transparency effect */
  }

  &:hover .copy-icon {
    opacity: 1;
  }
}
</style>

Changes Made:

  1. Fixed the incorrect closure tag for the <style> element.
  2. Simplified the conditional logic for showing/hiding the copy icon using Vue directives @mouseenter and @mouseleave.
  3. Replaced the duplicate styles related to button appearance and color inside :.config-textarea::deep(...) with a more direct approach.
  4. Cleaned up the CSS styles for better readability and flexibility. Added box-shadow: none to allow text highlighting without shadows appearing under the icons, enhancing user experience.

8 changes: 4 additions & 4 deletions ui/src/views/tool/component/ToolListContainer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
<img src="@/assets/workflow/icon_tool.svg" style="width: 58%" alt="" />
</el-avatar>
<div class="pre-wrap ml-8">
<div class="lighter">{{ $t('views.tool.form.create') }}</div>
<div class="lighter">{{ $t('views.tool.createTool') }}</div>
</div>
</div>
</el-dropdown-item>
Expand All @@ -60,7 +60,7 @@
<img src="@/assets/workflow/icon_mcp.svg" style="width: 75%" alt="" />
</el-avatar>
<div class="pre-wrap ml-8">
<div class="lighter">{{ $t('views.tool.form.createMcp') }}</div>
<div class="lighter">{{ $t('views.tool.createMcpTool') }}</div>
</div>
</div>
</el-dropdown-item>
Expand Down Expand Up @@ -222,8 +222,8 @@
v-if="item.tool_type === 'MCP'"
@click.stop="showMcpConfig(item)"
>
<AppIcon iconName="app-edit" class="color-secondary"></AppIcon>
{{ $t('views.tool.form.mcpConfig') }}
<AppIcon iconName="app-operate-log" class="color-secondary"></AppIcon>
{{ $t('views.tool.mcpConfig') }}
</el-dropdown-item>
<el-dropdown-item
v-if="item.template_id && permissionPrecise.edit(item.id)"
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 code you provided seems to contain some issues related to string translations and possibly incorrect permissions checking logic. Here are the key points to consider:

  1. String Translation Errors:

    • The strings $t('views.tool.form.create'), $t('views.tool.createTool'), and $t('views.tool.form.mcpConfig') seem to be incorrect or should match existing translation keys in your application's localization files.
  2. Permissions Checking Logic:

    • Ensure that permissionPrecise.edit(item.id) correctly implements the necessary checks for user permissions on editing tool configurations.
    • If these permissions are not defined or need updating, they may block access unnecessarily.
  3. Alternative Icon Usage:

    • In the last dropdown item where an icon is supposed to show a configuration log (AppIcon iconName="app-edit"), it mentions "operate log", which might indicate a typo or misconfiguration of the iconName.
    • Replace "app-operate-log" with 'edit' if this matches your intended action for edit operations.
  4. General Code Readability:

    • Consider consolidating similar actions into a single button or menu group to improve readability and reduce complexity.

Here is a revised version addressing the identified issues:

@@ -50,7 +50,7 @@
                     <img :src="$require('../../assets/workflow/icon_tool.svg')" style="width: 58%; height: auto;" alt="" />
                   </el-avatar>
                   <div class="pre-wrap ml-8">
-                    <div class="lighter">{{ $t('view.toolbar.tools.newTool') }}</div> <!-- Assuming there is such a translation -->
+                    <div class="lighter">{{ $t('view.toolbar.tools.createTool') }}</div> <!-- Check the actual translation key -->
                  </div>
                </div>
              </el-dropdown-item>
@@ -60,7 +60,7 @@
                     <img :src="$require('../../assets/workflow/icon_mcp.svg')" style="width: 75%"; height: auto;" alt="" />
                   </el-avatar>
                   <div class="pre-wrap ml-8" v-if="item.type.toLowerCase() === 'mcp'" data-tool-type="mcptool">
-                    <span>{{ $t('view.toolbar.tools.mcPconfig', { name: item.name }) }}</span>
+                    <span>{{ $t('view.toolbar.tools.configMCPTool', { name: item.name }) }}</span> <!-- Adjust the translation key as needed -->
                    
                    <!-- Use the correct permission checking function -->
                    <template v-if="hasEditPermissionForMcP(item)">{{ __('Edit MCP Config') }}</template>

Make sure the above changes align with your actual translation system and that all translated strings exist in your locale files (e.g., en.json). Additionally, ensure hasEditPermissionForMcP exists and properly manages user-level restrictions on MCP items.

Expand Down
6 changes: 2 additions & 4 deletions ui/src/workflow/nodes/start-node/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@
</el-button>
</el-tooltip>
</div>
<template v-if="nodeModel.properties.config.chatFields">
<template v-if="nodeModel.properties.config.chatFields?.length">
<h5 class="title-decoration-1 mb-8">{{ $t('views.applicationWorkflow.variable.chat') }}</h5>
<div
v-for="(item, index) in nodeModel.properties.config.chatFields
? nodeModel.properties.config.chatFields
: []"
v-for="(item, index) in nodeModel.properties.config.chatFields || []"
:key="index"
class="flex-between border-r-6 p-8-12 mb-8 layout-bg lighter"
@mouseenter="showicon = true"
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 code you provided contains several improvements that address potential issues and improve readability:

  1. Nullish Coalescing Operator:

    v-for="(item, index) in nodeModel.properties.config.chatFields || []"

    This uses JavaScript's nullish coalescing operator (||) to ensure that nodeModel.properties.config.chatFields is treated as falsy if it's undefined or null, preventing errors from attempting to iterate over a non-existent array.

  2. Conditional Check:

    <template v-if="nodeModel.properties.config.chatFields?.length">

    This adds a conditional check to verify that chatFields has elements before rendering. It also uses optional chaining (??) to avoid accessing properties of an undefined or null object.

  3. Variable Initialization:
    Although this doesn't explicitly solve a problem but improves clarity, initializing variables can help document the purpose of each variable:

    let showicon = false;

Here's the revised version of your code with these improvements:

<div>
  <!-- Existing content -->
</div>

<template v-if="nodeModel.properties.config && nodeModel.properties.config.chatFields?.length">
  <h5 class="title-decoration-1 mb-8">{{ $t('views.applicationWorkflow.variable.chat') }}</h5>
  <div
    v-for="(item, index) in nodeModel.properties.config.chatFields || []
    :key="index"
    class="flex-between border-r-6 p-8-12 mb-8 layout-bg lighter"
    @mouseenter="showicon = true" @mouseleave="showicon = false"
  >
    <!-- Content for chat fields -->
  </div>
</template>

<!-- Example additional line added to demonstrate better understanding -->
let showicon = false; // To store whether icon should be shown on hover

These changes make the code more robust, easier to understand, and less prone to runtime errors caused by unexpected data structures.

Expand Down
Loading