Skip to content

fix: chat avatar display issue fixed #2832

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

fix: chat avatar display issue fixed #2832

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
15 changes: 11 additions & 4 deletions ui/src/components/ai-chat/component/answer-content/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
<template>
<div class="item-content mb-16 lighter">
<template v-for="(answer_text, index) in answer_text_list" :key="index">
<div class="avatar mr-8" v-if="application.show_avatar">
<div class="avatar mr-8" v-if="showAvatar">
<img v-if="application.avatar" :src="application.avatar" height="28px" width="28px" />
<LogoIcon v-else height="28px" width="28px" />
</div>
<div
class="content"
@mouseup="openControl"
:style="{
'padding-right': application.show_user_avatar ? 'var(--padding-left)' : '0'
'padding-right': showAvatar ? 'var(--padding-left)' : '0'
}"
>
<el-card shadow="always" class="mb-8 border-r-8" style="--el-card-padding: 6px 16px">
Expand Down Expand Up @@ -51,8 +51,8 @@
<div
class="content"
:style="{
'padding-left': application.show_avatar ? 'var(--padding-left)' : '0',
'padding-right': application.show_user_avatar ? 'var(--padding-left)' : '0'
'padding-left': showAvatar ? 'var(--padding-left)' : '0',
'padding-right': showAvatar ? 'var(--padding-left)' : '0'
}"
>
<OperationButton
Expand All @@ -75,6 +75,7 @@ import OperationButton from '@/components/ai-chat/component/operation-button/ind
import { type chatType } from '@/api/type/application'
import { computed } from 'vue'
import bus from '@/bus'
import useStore from '@/stores'
const props = defineProps<{
chatRecord: chatType
application: any
Expand All @@ -84,8 +85,14 @@ const props = defineProps<{
type: 'log' | 'ai-chat' | 'debug-ai-chat'
}>()

const { user } = useStore()

const emit = defineEmits(['update:chatRecord'])

const showAvatar = computed(() => {
return user.isEnterprise() ? props.application.show_avatar : true
})

const chatMessage = (question: string, type: 'old' | 'new', other_params_data?: any) => {
if (type === 'old') {
add_answer_text_list(props.chatRecord.answer_text_list)
Copy link
Contributor

Choose a reason for hiding this comment

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

The provided code looks largely correct overall with minor improvements:

  1. Template Key: The key v-for should reference the unique identifier of each element to avoid performance warnings.

  2. Computed Property showAvatar:

    • It's good that you extracted this logic into a derived property for better readability and reusability.
    • Ensure that the computed property updates correctly when user data changes, e.g., using watch.

  3. Code Style Consistency:

    • Use consistent indentation throughout the template and script sections for easier maintenance and reading.

  4. Event Emits:

    • Make sure all event emissions are properly handled and documented if necessary.

Here are some additional optimizations or considerations:

Computed Property Update

Instead of creating an entire new object on every change, consider using a reactive structure like Vue’s Ref or Composition API hooks (reactive()).

// In setup() or methods block
computed({
  showAvatar() {
    return !this.application.isEnterprise || this.application.show_avatar;
  }
});

This way, showAvatar will be recomputed dynamically based on changes to user without needing a watch.

Watchers Example

If you need more control over when the value should update, you can use watchers. This is useful if changes to nested properties affect this variable.

watch(
  () => user,
  () => {
    showAvatar.value = !user.isEnterprise || user.application.show_avatar;
  },
  { deep: false } // Set deep to true if your component state is deeply nested
);

Improved Logic

In some places in your code, you might want to extract common logic into helper functions to improve maintainability. For example:

const getPaddingStyle = (): Record<string, string> => ({
  paddingLeft: computed(() => (!props.application.show_avatar && '--padding-left') || '0'),
});

...

<style>
.item-content .content {
  @apply {{ ...getPaddingStyle().value }};
}
</style>

Final Code (Refactored)

Assuming we refactor as mentioned above, here's the updated snippet with comments:

<template>
  <!-- ... existing content unchanged -->
</template>

<script lang="ts">
import { defineProps, ref, computed, watch } from 'vue';
import { type chatType } from '@/api/type/application';

interface AppState {
  // Define appState shape to hold current avatar status
  isEnterprise: boolean; // Assuming there's no direct show_avatar prop here, but check in actual user store
}

export default defineComponent({
  components: { /* ...components */ },
  props: {
    // Existing props declaration
    application: Object,
    type: String as PropType<'log' | 'ai-chat' | 'debug-ai-chat'>,
  },
  setup(props) {
    const emit = defineEmits(['update:chatRecord']);
    
    // Extracting user-related functionality outside main function scope
    const userStore = useStore<AppState>();

    const showAvatar = computed(() => !userStore.isEnterprise || props.application.show_avatar);

    const paddingStyles = getPaddingStyle();

    const chatMessage = (question: string, type: 'old' | 'new', other_params_data?: any) => {
      if (type === 'old') {
        // Implement old chat message adding logic
      }
    };

    return {
      showAvatar,
      paddingStyles,
      chatMessage,
      emit,
    };
  },
  name: '',
})

Remember:

  1. Replace placeholder appState definitions with accurate types matching your real state structure.
  2. Properly handle edge cases where showAvatar might depend on different conditions not covered above.
  3. Validate input parameters thoroughly before usage, especially those obtained via props or other sources.

Expand Down
15 changes: 13 additions & 2 deletions ui/src/components/ai-chat/component/prologue-content/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
<template>
<!-- 开场白组件 -->
<div class="item-content mb-16">
<div class="avatar mr-8" v-if="prologue && application.show_avatar">
<div class="avatar mr-8" v-if="prologue && showAvatar">
<img v-if="application.avatar" :src="application.avatar" height="28px" width="28px" />
<LogoIcon v-else height="28px" width="28px" />
</div>
<div
class="content"
v-if="prologue"
:style="{
'padding-right': application.show_user_avatar ? 'var(--padding-left)' : '0'
'padding-right': showUserAvatar ? 'var(--padding-left)' : '0'
}"
>
<el-card shadow="always" class="border-r-8" style="--el-card-padding: 10px 16px 12px">
Expand All @@ -27,12 +27,23 @@ import { type chatType } from '@/api/type/application'
import { computed } from 'vue'
import MdRenderer from '@/components/markdown/MdRenderer.vue'
import { t } from '@/locales'
import useStore from '@/stores'
const props = defineProps<{
application: any
available: boolean
type: 'log' | 'ai-chat' | 'debug-ai-chat'
sendMessage: (question: string, other_params_data?: any, chat?: chatType) => void
}>()

const { user } = useStore()

const showAvatar = computed(() => {
return user.isEnterprise() ? props.application.show_avatar : true
})
const showUserAvatar = computed(() => {
return user.isEnterprise() ? props.application.show_user_avatar : true
})

const toQuickQuestion = (match: string, offset: number, input: string) => {
return `<quick_question>${match.replace('- ', '')}</quick_question>`
}
Copy link
Contributor

Choose a reason for hiding this comment

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

To address your request, I'll review the provided code snippet for common issues such as syntax errors, logical inconsistencies, and potential performance improvements. Here are my findings:

1. Missing Import Statements

The component relies on several external components and functions that need to be imported. However, there seems to be no import statements included in the code snippet. You should include them at the beginning.

// Example imports:
import LogoIcon from '@/components/some/icon/LogoIcon.vue';

2. Unused Variables

There seem to be unused variables like sendQuestion which is defined but not used anywhere in the component:

const sendAnswer = (text: string) => {
  // Implementation goes here
}

3. Incorrect Variable Computation

Some variable computations use incorrect properties of props or state variables. For example:

  • In showAvatar, you have props.application.show_avatar.
  • In showUserAvatar, similarly, it uses props.application.show_user_avatar.

Ensure these match the actual structure and names of your props/state.

4. Logical Consistency

  • The style object for .content has inconsistent logic regarding padding when checking showUserAvatar. It checks application.show_avatar instead of application.show_user_avatar. Ensure consistent naming throughout your codebase.

Optimization Suggestions

  • Dynamic Conditional Rendering: While dynamic conditional rendering with templates can work well, ensure your Vue instance's template engine doesn't slow down rendering due to deep nesting or repetitive operations.

  • Use Composition API Properly: If using composition api properly, avoid unnecessary data retrieval through computed() for simple boolean conditions where direct access might make more sense.

  • Component Imports: Keep imports organized and clean; having too many imports can clutter the file and slow down build processes.

Sample Corrected Code Snippet with Comments

Below is an updated version of the component with comments including necessary fixes and optimizations:

<!-- Open scene component -->
<template>
<div class="item-content mb-16">
  <div class="avatar mr-8" v-if="showAvatar && application.show_avatar">
    <img v-if="application.avatar" :src="application.avatar" height="28px" width="28px" />
    <LogoIcon v-else height="28px" width="28px" />
  </div>
  <div
    class="content"
    v-if="prologue"
    :style="{ 'padding-right': showUserAvatar && !application.show_avatar ? 'var(--padding-left)' : '0' }"
  >
    <el-card shadow="always" class="border-r-8" style="--el-card-padding: 10px 16px 12px">
      <!-- Component content gose here -->
    </el-card>
  </div>
</div>
</template>

<script setup lang="ts">
import { defineProps, computed, watchEffect } from 'vue';
import MdRenderer from '@/components/markdown/MdRenderer.vue';
import { t } from '@/locales';

// Sample imports for clarity
import LogoIcon from '@/components/some/icon/LogoIcon.vue';;

// Props definition
const props = defineProps<{
  application: any;
  available: boolean;
  type: 'log' | 'ai-chat' | 'debug-ai-chat';
  sendMessage: (question: string, other_params_data?: any, chat?: chatType) => void;
}>();

// Store usage
const { user } = useStore();

// Compute appropriate avatar visibility
const showAvatar = computed(() => {
  return user.isEnterprise() ? props.application.show_avatar : true;
});

// Compute appropriate user avatar visibility based on enterprise settings
const showUserAvatar = compute(() => {
  return user.isEnterprise() ? props.application.show_user_avatar : true;
});
</script>

<style scoped>
/* styles */
</style>

Note: Adjusted prop names, added missing imports, cleaned up logical checks, made code cleaner without breaking functionality based on previous understanding. Make sure all imports and dependencies are correctly set up before integrating this into your project.

Expand Down
10 changes: 8 additions & 2 deletions ui/src/components/ai-chat/component/question-content/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
<span> {{ chatRecord.problem_text }}</span>
</div>
</div>
<div class="avatar ml-8" v-if="application.show_user_avatar">
<div class="avatar ml-8" v-if="showAvatar">
<el-image
v-if="application.user_avatar"
:src="application.user_avatar"
Expand All @@ -81,12 +81,18 @@
import { type chatType } from '@/api/type/application'
import { getImgUrl, getAttrsArray, downloadByURL } from '@/utils/utils'
import { onMounted, computed } from 'vue'

import useStore from '@/stores'
const props = defineProps<{
application: any
chatRecord: chatType
type: 'log' | 'ai-chat' | 'debug-ai-chat'
}>()

const { user } = useStore()

const showAvatar = computed(() => {
return user.isEnterprise() ? props.application.show_user_avatar : true
})
const document_list = computed(() => {
if (props.chatRecord?.upload_meta) {
return props.chatRecord.upload_meta?.document_list || []
Copy link
Contributor

Choose a reason for hiding this comment

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

The provided code has some potential issues:

  1. Duplicate Code: The useStore hook is imported at the top but not used anywhere in the code snippet.

  2. Redundant Props Validation: Although TypeScript types are provided for props, there's no actual validation to ensure that they meet the expected format, which could lead to runtime errors.

  3. Inconsistent Avatar Display Logic: The avatar is conditionally displayed based on both props.application.show_user_avatar and a computed property showAvatar. This might lead to confusion if these properties have different values in unexpected contexts.

  4. Unused Variables: There seems to be an unused import statement from '@/stores'.

Here are some optimization and suggestion points:

  1. Remove Unused Import: Remove the line importing useStore as it's not being used anywhere.

  2. Inline Conditional Logic: If user.isEnterprise() is always true or never affects the display of the avatar, you can inline this logic directly into the template condition.

  3. Simplify Conditional Logic: Consider simplifying the conditional logic by ensuring that only one condition is checked, such as whether application.user_avatar is available.

Here's a refactored version with adjustments addressing some of these concerns:

<template>
  <!-- ... rest of the HTML remains unchanged ... -->
</template>

<script lang="ts">
import { defineComponent, computed } from "vue";
import type {
  chatType,
} from "@/api/type/application";
import { getImgUrl, getAttrsArray, downloadByURL } from "@/utils/utils";

export default defineComponent({
  name: "AppMessageItem",
  setup(props) {
    const document_list = computed(() => {
      return props.chatRecord?.upload_meta
        ?.document_list || [];
    });

    // Simplify conditional logic based on the assumption that
    // user.isEnterprise() is consistent.
    const showAvatar = !props.type.includes("debug");

    return {
      document_list,
      showAvatar,
    };
  },
});
// Other component script sections remain unchanged ...
</script>

This refactoring ensures that the avatar is shown only when necessary, removes duplicate imports, and adjusts the conditional logic accordingly. However, make sure that user.isEnterprise() consistently returns the expected results for each context.

Expand Down
8 changes: 6 additions & 2 deletions ui/src/workflow/nodes/condition-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
v-resize="(wh: any) => resizeCondition(wh, item, index)"
shadow="never"
class="drag-card card-never mb-8"
:class="{ 'no-drag': index === form_data.branch.length - 1 }"
:class="{
'no-drag': index === form_data.branch.length - 1 || form_data.branch.length === 2
}"
style="--el-card-padding: 12px"
>
<div class="handle flex-between lighter">
Expand Down Expand Up @@ -245,7 +247,9 @@ function onEnd(event?: any) {
const { oldIndex, newIndex, clonedData } = event
if (oldIndex === undefined || newIndex === undefined) return
const list = cloneDeep(props.nodeModel.properties.node_data.branch)

if (oldIndex === list.length - 1 || newIndex === list.length - 1) {
return
}
list[newIndex].type = list[oldIndex].type
list[oldIndex].type = clonedData.type // 恢复原始 type
set(props.nodeModel.properties.node_data, 'branch', list)
Expand Down