Skip to content

fix: In the permission settings, the knowledge base list does not distinguish between knowledge base types, and all are marked with the icon of the general knowledge base. #2806

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.

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
merged 1 commit into from
Apr 7, 2025

Conversation

shaohuzhang1
Copy link
Contributor

fix: In the permission settings, the knowledge base list does not distinguish between knowledge base types, and all are marked with the icon of the general knowledge base. --bug=1054142 --user=王孝刚 【团队成员】权限设置中,知识库列表没有区分知识库类型,都用通用知识库的图标标识 https://www.tapd.cn/57709429/s/1681104

…tinguish between knowledge base types, and all are marked with the icon of the general knowledge base.

--bug=1054142 --user=王孝刚 【团队成员】权限设置中,知识库列表没有区分知识库类型,都用通用知识库的图标标识 https://www.tapd.cn/57709429/s/1681104
Copy link

f2c-ci-robot bot commented Apr 7, 2025

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.

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/test-infra repository.

Copy link

f2c-ci-robot bot commented Apr 7, 2025

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

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@@ -85,6 +112,7 @@
<script setup lang="ts">
import { ref, onMounted, watch, computed } from 'vue'
import { TeamEnum } from '@/enums/team'
import { isAppIcon } from '@/utils/application'

const props = defineProps({
data: {
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 are several issues and optimizations that can be addressed in the provided code:

  1. Logical Error with Avatar Styling: The <AppAvatar> component has incorrect styling conditions.

    • style="background: none" should be inside the <AppAvatar> tag, not added to the class.
    • Ensure that isAppIcon(row?.icon) correctly returns true or false.
  2. Redundant Code Blocks: Remove duplicate conditional blocks where an app icon (row.icon) exists but no application type (isApplication) matches.

  3. Simplified SVG Source for Icons: Avoid adding inline styles like style="width: 58%" and style="width: 100%". Instead, use Vue's CSS classes effectively.

  4. Potential Missing Import Statements: Ensure that import { isAppIcon } includes all necessary function implementations or imports (if available).

Here’s the updated code snippet incorporating these corrections:

 <template #default="{ row }">
   <div class="flex align-center">
     <AppAvatar
       v-if="isApplication && isAppIcon(row?.icon)"
       class="mr-12"
       shape="square"
       :size="24"
     >
       <img :src="row?.icon" alt="" />
     </AppAvatar>

     <AppAvatar
       v-else-if="row?.name && isApplication"
       :name="row?.name"
       pinyinColor
       shape="square"
       :size="24"
       class="mr-12"
     />

     <AppAvatar
       v-if="row?.icon === '1'"
       class="mr-8 avatar-purple"
       shape="square"
       :size="24"
     >
       <img src="@/assets/icon_web.svg" alt="" />
     </AppAvatar>
     <AppAvatar
       v-else-if="row?.icon === '2'"
       class="mr-8 avatar-purple"
       shape="square"
       :size="24"
     >
       <img src="@/assets/logo_lark.svg" alt="" />
     </AppAvatar>
     <AppAvatar v-else-if="isDataset" class="mr-8 avatar-blue" shape="square" :size="24">
       <img src="@/assets/icon_document.svg" alt="" />
     </AppAvatar>

     <auto-tooltip :content="row?.name">

And ensure you have the correct implementation of isAppIcon if needed:

// utils/application.ts
export const isAppIcon = (icon?: string): boolean => {
  // Example logic, adjust according to actual requirements
  return ["app-id1", "app-id2"].includes(icon);
};

By making these changes and ensuring proper usage of dependencies and functions, the component should work as intended without any visual discrepancies due to wrong styling attributes.

@wxg0103 wxg0103 merged commit add9d1b into main Apr 7, 2025
4 of 5 checks passed
@wxg0103 wxg0103 deleted the pr@main@fix_1054142 branch April 7, 2025 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants