Skip to content

fix: style #7864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wangdan-fit2cloud merged 1 commit into from
Nov 12, 2026
Merged

fix: style #7864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion ui/src/components/global/mk-form-list/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ function removeRow(index: number) {
<MkIcon name="icon_drag_outlined" />
</el-button>
<slot :index="index" :item="item" />
<el-form-item class="shrink-0" :class="firstRowHasLabel ? (index === 0 ? 'mt-8' : 'mt-0.5') : 'mt-1'">
<el-form-item class="shrink-0 small" :class="firstRowHasLabel ? (index === 0 ? 'mt-8' : 'mt-0.5') : 'mt-1'">
<el-button :disabled="formRows.length <= minRows" text @click="removeRow(index)">
<MkIcon name="icon_delete-trash_outlined" />
</el-button>
Expand Down
12 changes: 9 additions & 3 deletions ui/src/conversation-panel/components/header/chat/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ const handleNewConversation = () => (props.newConversation ? props.newConversati
</script>

<template>
<header class="mk-conversation-chat-header flex-align-center gap-2 p-4">
<header class="mk-conversation-chat-header flex-align-center gap-1 p-4">
<!-- 左侧收起时才在顶部显示:展开左侧 + 新建对话(展开时它们在侧栏内) -->
<template v-if="!list.leftSideOpen.value">
<!-- 展开左侧 -->
<MkTooltip content="展开侧边栏">
<el-button text @click="list.toggleLeftSide()" class="-ml-1 mr-1">
<el-button text @click="list.toggleLeftSide()" class="-ml-1 mr-2">
<MkIcon name="icon_sidebar_outlined" :size="18" class="text-N900!" />
</el-button>
</MkTooltip>
Expand All @@ -37,7 +37,7 @@ const handleNewConversation = () => (props.newConversation ? props.newConversati
</div>
<template v-if="!list.isShared.value">
<!-- 新建对话 -->
<MkTooltip content="新建对话">
<MkTooltip content="新建对话" v-if="!list.leftSideOpen.value">
<el-button text @click="handleNewConversation">
<MkIcon name="icon_new-chat_outlined" :size="18" class="text-N900!" />
</el-button>
Expand All @@ -48,6 +48,12 @@ const handleNewConversation = () => (props.newConversation ? props.newConversati
<MkIcon name="icon_share_outlined" :size="18" :class="canShare ? 'text-N900!' : 'text-N400!'" />
</el-button>
</MkTooltip>
<!-- 导出-->
<MkTooltip content="导出">
<el-button text>
<MkIcon name="icon_export_outlined" :size="18" class="text-N900!" />
</el-button>
</MkTooltip>
<slot />
</template>
</header>

@shaohuzhang1 shaohuzhang1 Nov 12, 2026

Copy link
Copy Markdown
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 review of the code changes, focusing on potential issues, best practices, and optimizations.

1. Potential Issues & Bugs

A. new Conversation Button Visibility Logic (Critical)

In the second hunk:

-      <MkTooltip content="新建对话">
+      <MkTooltip content="新建对话" v-if="!list.leftSideOpen.value">

You added v-if="!list.leftSideOpen.value" to the "New Conversation" button. However, looking at the context above it in the first hunk:

<template v-if="!list.leftSideOpen.value">
   <!-- ... Expand Sidebar Button ... -->
</template>

It appears that when the sidebar is open, the "New Conversation" button should likely be inside the sidebar itself (as hinted by the Chinese comment: "展开时它们在侧栏内" - When expanded, they are inside the sidebar).

Issue: By adding v-if="!list.leftSideOpen.value", you are hiding the button in the header when the sidebar is open. This is likely intentional. However, ensure that the "New Conversation" button is correctly rendered inside the leftSide component when it is open. If not, users lose the ability to create a new conversation entirely.

B. Missing Click Handler for Export Button

In the third hunk:

+      <MkTooltip content="导出">
+        <el-button text>
+          <MkIcon name="icon_export_outlined" :size="18" class="text-N900!" />
+        </el-button>
+      </MkTooltip>

The "Export" button has no @click handler. Clicking it will do nothing. You need to bind an event (e.g., @click="handleExport").

C. v-if vs. Dynamic Class Binding in Tooltip

Using v-if directly on <MkTooltip> can sometimes cause layout shifts or re-mounting issues depending on how MkTooltip manages its DOM/Portal. A safer approach for visibility toggling is often CSS-based (e.g., using a computed property with a dynamic style/class) if the component doesn't handle v-if gracefully. However, v-if is acceptable if performance isn't critical.

2. Coding Style & Best Practices

A. Inconsistent Formatting/Indentation

+      <!-- 导出-->
+      <MkTooltip content="导出">
  • The comment <!-- 导出--> has inconsistent spacing before the closing -->.
  • Ensure consistent indentation across all blocks.

B. Hardcoded Icons & Text

  • Icon Name: icon_export_outlined is hardcoded. Ensure this icon name exists in your icon set library to avoid missing icons in production.
  • Text: content="导出" is hardcoded Chinese. If the app supports internationalization (i18n), use a translation key (e.g., :content="$t('common.export')").

C. Tailwind Class Usage (!important)

You are using class="text-N900!" and class="text-N400!". The exclamation mark ! usually denotes an !important override in UnoCSS/Tailwind JIT. Using !important is a "code smell" unless absolutely necessary for overriding specific library styles (like Element Plus defaults). Verify if this is truly needed or if lower-specificity classes would suffice to maintain cleaner CSS precedence.

3. Optimization Suggestions

A. Extract Repeated Tooltip/Button Pattern

You have multiple instances of:

<MkTooltip content="...">
  <el-button text @click="...">
    <MkIcon name="..." :size="18" class="text-N900!" />
  </el-button>
</ MkTooltip>

Consider creating a smaller reusable component (e.g., <ChatHeaderAction />) to reduce verbosity and make future changes (like changing icon size or tooltip style) easier.

<!-- Proposed component usage -->
<ChatHeaderAction 
  id="new-chat" 
  icon="icon_new-chat_outlined" 
  text="新建对话" 
  @click="handleNewConversation" 
  v-if="!list.leftSideOpen.value" 
/>

B. Computed Property for Visibility

Instead of repeating !list.leftSideOpen.value in multiple v-ifs, consider a computed property:

const showHeaderActions = computed(() => !props.list.leftSideOpen.value && !list.isShared.value)

(Note: Adjust logic as per exact requirements).

4. Summary of Required Actions

  1. Add @click handler to the Export button.
  2. Verify that "New Conversation" functionality remains accessible when the sidebar is open (likely via the sidebar component).
  3. Check i18n compliance for the hardcoded string "导出".
  4. Fix comment formatting (<!-- Export -->).
  5. Review the necessity of ! in Tailwind classes (text-N900!).

Expand Down
14 changes: 10 additions & 4 deletions ui/src/conversation-panel/components/header/portal/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ const handleNewConversation = () => (props.newConversation ? props.newConversati
</script>

<template>
<header class="flex-align-center p-4">
<header class="flex-align-center gap-1 p-4">
<!-- 左侧收起时才在顶部显示:展开左侧 + 新建对话(展开时它们在侧栏内) -->
<template v-if="!list.leftSideOpen.value">
<!-- 展开左侧 -->
<MkTooltip content="展开侧边栏">
<el-button text @click="list.toggleLeftSide()" class="-ml-1 mr-1">
<el-button text @click="list.toggleLeftSide()" class="-ml-1 mr-2">
<MkIcon name="icon_sidebar_outlined" :size="18" class="text-N900!" />
</el-button>
</MkTooltip>
Expand All @@ -42,15 +42,21 @@ const handleNewConversation = () => (props.newConversation ? props.newConversati
</el-tag>
</div>
<!-- 新建对话 -->
<MkTooltip content="新建对话">
<MkTooltip content="新建对话" v-if="!list.leftSideOpen.value">
<el-button text @click="handleNewConversation">
<MkIcon name="icon_new-chat_outlined" :size="18" class="text-N900!" />
</el-button>
</MkTooltip>
<!-- 进入分享模式，勾选要分享的对话 -->
<MkTooltip v-if="showShare" content="分享">
<el-button text :disabled="!canShare" @click="emit('share')">
<MkIcon name="icon_share_outlined" :size="18" class="text-N900!" />
<MkIcon name="icon_share_outlined" :size="18" :class="canShare ? 'text-N900!' : 'text-N400!'" />
</el-button>
</MkTooltip>
<!-- 导出-->
<MkTooltip content="导出">
<el-button text>
<MkIcon name="icon_export_outlined" :size="18" class="text-N900!" />
</el-button>
</MkTooltip>
<slot />

@shaohuzhang1 shaohuzhang1 Nov 12, 2026

Copy link
Copy Markdown
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 review of the provided code snippet, highlighting issues and optimization suggestions:

1. Missing Event Handler (Critical Issue)

  • Location: The newly added "Export" button.
  • Issue: The <el-button> for export has no @click event listener defined. It currently does nothing when clicked.
  • Fix: Add an @click handler, e.g., @click="handleExport" or @click="emit('export')".

2. Inconsistent Conditional Rendering Logic

  • Location: "New Conversation" button.
  • Issue: You added v-if="!list.leftSideOpen.value" to the "New Conversation" tooltip/button. However, the comment above says "When collapsed, [buttons] are in the sidebar". This implies that when the sidebar is open, these buttons should be visible in the main header? Or hidden?
    • Looking at the first block (toggleLeftSide button), it uses v-if="!list.leftSideOpen.value" (visible only when closed).
    • If the logic is "Show header buttons only when sidebar is closed", then adding v-if="!list.leftSideOpen.value" to the New Chat button is consistent.
    • However, if the intention was to move these buttons into the sidebar when open, ensure you have corresponding buttons in the sidebar component. If not, users might lose access to "New Conversation" when the sidebar is open. Verify this UX intent.

3. Optimization: Class Binding Syntax

  • Location: Share button icon class binding.
  • Current Code: 
    :class="canShare ? 'text-N900!' : 'text-N400!'"
  • Suggestion: While valid, consider using object syntax or computed classes if more states exist. For now, this is acceptable. No major issue.

4. Accessibility (a11y) Improvement

  • Location: All icons/buttons without text labels.
  • Issue: Icon-only buttons rely on tooltips for accessibility. Screen readers may not announce the action clearly unless aria-label is present.
  • Suggestion: Add aria-label to each el-button: 
    <el-button ... aria-label="Toggle Sidebar">
<el-button ... aria-label="Export">

5. Redundant Tooltip Logic?

  • Question: Does MkTooltip wrap disabled buttons properly?
  • Check: Ensure MkTooltip handles disabled el-buttons correctly (some UI libraries require disabling the parent wrapper too). If canShare is false, the button is disabled, but is the tooltip still showing? Usually, you want to hide the tooltip when disabled to avoid confusion. Consider adding :disabled="!canShare" to the <MkTooltip> itself if supported, or conditionally render the tooltip only when enabled.

6. Consistency in Margin/Padding

  • Change: Header padding changed from p-4 to gap-1 p-4.
  • Comment: Added mr-2 to the sidebar toggle button.
  • Note: Ensure visual spacing looks correct across all breakpoints. gap-1 helps with internal spacing if flex items grow.

✅ Recommended Fix for Export Button

<!-- Before -->
<th<MkTooltip content="导出">
  <el-button text>
    <MkIcon name="icon_export_outlined" :size="18" class="text-N900!" />
  </el-button>
</th<MkTooltip>

<!-- After -->
<MkTooltip content="导出">
  <el-button text @click="handleExport" aria-label="Export"> <!-- Add handler & aria-label -->
    <MkIcon name="icon_export_outlined" :size="18" class="text-N900!" />
  </el-button>
</MkTooltip>

Don't forget to define handleExport in your script section:

const handleExport = () => {
  // Implement export logic or emit event
};

Expand Down
4 changes: 2 additions & 2 deletions ui/src/views/knowledge-detail/library/document/action-dropdown/setting-tags/AddTagDialog.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ defineExpose({ open })
<MkFormList v-model="tagForm.tags" :default-item="{ key: '', tagId: '' }" @update:model-value="handleRowsChange">
<template #default="{ index, item: tag }">
<el-form-item
class="min-w-0 flex-1"
class="min-w-0 flex-1 small"
:label="index === 0 ? '标签' : ''"
:prop="`tags.${index}.key`"
:rules="{ required: true, message: '请选择标签', trigger: 'change' }"
Expand All @@ -123,7 +123,7 @@ defineExpose({ open })
</el-select>
</el-form-item>
<el-form-item
class="min-w-0 flex-1"
class="min-w-0 flex-1 small"
:label="index === 0 ? '标签值' : ''"
:prop="`tags.${index}.tagId`"
:rules="{ required: true, message: '请选择标签值', trigger: 'change' }"

@shaohuzhang1 shaohuzhang1 Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here is the analysis of the code diff:

1. Irregularities & Potential Issues

  • Ambiguous CSS Class Name (small):
    • The class small is extremely generic. In a modern component library (like Element Plus, which uses el-form-item) or a utility-first framework (like Tailwind/UnoCSS), small might not have any predefined styles for spacing, font-size, or gap reduction unless explicitly defined in your global CSS or custom SCSS.
    • Risk: If .small is not globally defined or scoped correctly, this change will have zero visual effect. If it is defined globally, it might unintentionally affect other components if scoped styles are missing or misconfigured.
  • Inconsistency with Framework Conventions:
    • If you are using Element Plus, form items and selects support native sizing via props like size="small". Using a custom CSS class small to override padding/margin is less maintainable than using the built-in size prop.
    • If you are using Tailwind/UnoCSS, there is no standard small utility class. You likely meant something like text-sm, p-0, gap-0, or a custom variant.
  • Accessibility/Label Visibility:
    • The logic index === 0 ? '标签' : '' hides labels for all rows except the first one. This is a common pattern for repeated forms but can hurt accessibility (screen readers may lose context). Ensure that the surrounding layout or ARIA attributes provide sufficient context for hidden-label cases.

2. Optimization Suggestions

✅ Recommendation 1: Use Native Component Props (Preferred)

If using Element Plus, replace the custom CSS class with the native size prop. This ensures consistency with the rest of the application's theme and avoids CSS specificity wars.

<!-- Instead of class="... small", use -->
<el-form-item
  :size="'small'"  <!-- Or bind it: :size="formSize" -->
  @update:model-value="handleRowsChange"
  ...
>

✅ Recommendation 2: Use Utility-First Classes (If using UnoCSS/Tailwind)

If you intended to reduce spacing/font size manually, use explicit utility classes instead of a vague small.

<!-- Example: Reducing height/padding using UnoCSS/Tailwind syntax -->
<el-form-item
  class="min-w-0 flex-1 text-sm py-0 my-1" 
  ...
>

✅ Recommendation 3: Create a Custom Composable or Scoped Style

If you need complex compact styling, apply it to the container (MkFormList wrapper) rather than individual el-form-items. This reduces DOM overhead and makes style management centralized.

<MkFormList v-model="tagForm.tags" class="compact-form">
  ...
</MkFormList>

<style scoped>
.compact-form .el-form-item {
  margin-bottom: 12px; /* Adjust as needed */
}
.compact-form .el-select {
  width: 100%;
}
</style>

Summary of Action Items

  1. Verify if .small actually exists in your CSS. If not, this change is ineffective.
  2. Prefer <el-form-item size="small"> if you are using Element Plus.
  3. Avoid generic class names like small for layout adjustments; prefer descriptive utilities (text-xs, py-1) or native component props.

Expand Down
4 changes: 2 additions & 2 deletions ui/src/views/knowledge-detail/library/tags/components/CreateTagDialog.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ defineExpose({ open })
<MkFormList v-model="tagForm.tags" :default-item="defaultTag" @update:model-value="handleRowsChange">
<template #default="{ index, item: tag }">
<el-form-item
class="min-w-0 flex-1"
class="min-w-0 flex-1 small"
:label="index === 0 ? '标签' : ''"
:prop="`tags.${index}.key`"
:rules="{ required: true, whitespace: true, message: '请输入标签', trigger: 'blur' }"
>
<el-input v-model="tag.key" :disabled="!!fixedTagKey" :maxlength="64" placeholder="请输入标签" />
</el-form-item>
<el-form-item
class="min-w-0 flex-1"
class="min-w-0 flex-1 small"
:label="index === 0 ? '标签值' : ''"
:prop="`tags.${index}.value`"
:rules="{ required: true, whitespace: true, message: '请输入标签值', trigger: 'blur' }"
Expand Down
4 changes: 2 additions & 2 deletions ui/src/views/system/identity/roles/AddMemberDrawer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ defineExpose({ open })
<MkFormList v-model="memberForm.members" add-text="添加成员" :default-item="{ user_ids: [], workspace_ids: [] }">
<template #default="{ index, item: memberSetting }">
<el-form-item
class="flex-1"
class="flex-1 small"
:label="index === 0 ? '成员' : ''"
:prop="`members.${index}.user_ids`"
:rules="{ required: true, type: 'array', min: 1, message: '请选择成员', trigger: 'change' }"
Expand Down Expand Up @@ -130,7 +130,7 @@ defineExpose({ open })

<el-form-item
v-if="showWorkspace"
class="flex-1"
class="flex-1 small"
:label="index === 0 ? '工作空间' : ''"
:prop="`members.${index}.workspace_ids`"
:rules="{ required: true, type: 'array', min: 1, message: '请选择工作空间', trigger: 'change' }"
Expand Down
4 changes: 2 additions & 2 deletions ui/src/views/system/identity/users/UserFromDrawer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ defineExpose({ open })
<MkFormList v-model="userForm.role_setting" add-text="添加角色" :default-item="{ role_id: '', workspace_ids: [] }">
<template #default="{ index, item: roleAssignment }">
<el-form-item
class="flex-1"
class="flex-1 small"
:label="index === 0 ? '角色' : ''"
:prop="`role_setting.${index}.role_id`"
:rules="{ required: true, message: '请选择角色', trigger: 'change' }"
Expand All @@ -273,7 +273,7 @@ defineExpose({ open })
<!-- 企业版用户的非系统管理员角色需要指定工作空间。 -->
<el-form-item
v-if="auth.isEE"
class="flex-1"
class="flex-1 small"
:label="index === 0 ? '工作空间' : ''"
:prop="`role_setting.${index}.workspace_ids`"
:rules="
Expand Down
4 changes: 2 additions & 2 deletions ui/src/views/system/identity/users/batch-set-user-role/BatchSetUserRoleDialog.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ defineExpose({ open })
>
<template #default="{ index, item: roleAssignment }">
<el-form-item
class="flex-1"
class="flex-1 small"
:label="index === 0 ? '角色' : ''"
:prop="`role_setting.${index}.role_id`"
:rules="{ required: true, message: '请选择角色', trigger: 'change' }"
Expand All @@ -163,7 +163,7 @@ defineExpose({ open })
<!-- 企业版用户的非系统管理员角色需要指定工作空间。 -->
<el-form-item
v-if="auth.isEE"
class="flex-1"
class="flex-1 small"
:label="index === 0 ? '工作空间' : ''"
:prop="`role_setting.${index}.workspace_ids`"
:rules="
Expand Down
4 changes: 2 additions & 2 deletions ui/src/views/system/identity/workspaces/AddMemberDrawer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ defineExpose({ open })
<MkFormList v-model="memberForm.members" add-text="添加成员" :default-item="{ role_ids: [], user_ids: [] }">
<template #default="{ index, item: memberSetting }">
<el-form-item
class="flex-1"
class="flex-1 small"
:label="index === 0 ? '成员' : ''"
:prop="`members.${index}.user_ids`"
:rules="{ required: true, type: 'array', min: 1, message: '请选择成员', trigger: 'change' }"
Expand Down Expand Up @@ -126,7 +126,7 @@ defineExpose({ open })
</el-form-item>

<el-form-item
class="flex-1"
class="flex-1 small"
:label="index === 0 ? '角色' : ''"
:prop="`members.${index}.role_ids`"
:rules="{ required: true, type: 'array', min: 1, message: '请选择角色', trigger: 'change' }"
Expand Down
2 changes: 1 addition & 1 deletion ui/src/workflow-canvas/nodes/reranker-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ onMounted(() => {
<MkFormList v-model="formData.reranker_reference_list" :default-item="[]" :first-row-has-label="false" :show-add-button="false">
<template #default="{ index }">
<el-form-item
class="mb-2! min-w-0 flex-1"
class="mb-2! min-w-0 flex-1 small"
:prop="`reranker_reference_list.${index}`"
:rules="{ type: 'array', required: true, message: '请选择重排内容', trigger: 'change' }"
>
Expand Down
Loading