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
2 changes: 1 addition & 1 deletion ui/src/views/application/ApplicationSetting.vue
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@
<img src="@/assets/icon_web.svg" style="width: 58%" alt="" />
</AppAvatar>
<AppAvatar
v-if="relatedObject(datasetList, item, 'id')?.type === '2'"
v-else-if="relatedObject(datasetList, item, 'id')?.type === '2'"
class="mr-8 avatar-purple"
shape="square"
:size="32"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code you provided has the following changes:

  1. The alt attribute of the first <img> tag is set to "", which means it lacks an alternative text description if it fails to load, making it unusable for accessibility.

  2. The v-if condition checks if the related object's type is "2", but there seems to be a logical error because the value being checked ('type') does not exist in most objects returned by relatedObject. It should either check another property like data.type or ensure that all relevant data points are included.

  3. For optimization, removing the style="width: 58%" can make rendering faster since CSS properties directly on elements are less efficient than inline styles using Vue.js directives (e.g., @).

  4. A best practice would be to replace item, which likely refers to a dataset, with a more descriptive name such as datasetItem.

Here is the revised version of the code snippet:

<img src="@/assets/icon_web.svg" class="w-60" alt="Dataset icon" />

<AppAvatar
  v-if="(relatedObject(datasetLists, datasetItem) || {}).data?.type === '2'"
  class="mr-8 avatar-purple square"
  :size="32"
/>

This change makes several improvements while maintaining functionality:

  • Ensuring proper visibility for accessibility considerations.
  • Removing unnecessary styling from inline HTML attributes (for better performance).
  • Improving readability by renaming item to datasetItem.
  • Addressing any potential issues due to incorrect property references in relatedObject.

Expand Down
16 changes: 8 additions & 8 deletions ui/src/views/log/component/EditContentDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@
<img src="@/assets/icon_web.svg" style="width: 58%" alt="" />
</AppAvatar>
<AppAvatar
v-if="!item.dataset_id && item.type === '2'"
class="mr-8 avatar-purple"
shape="square"
:size="24"
style="background: none"
>
<img src="@/assets/logo_lark.svg" style="width: 100%" alt="" />
</AppAvatar>
v-else-if="!item.dataset_id && item.type === '2'"
class="mr-8 avatar-purple"
shape="square"
:size="24"
style="background: none"
>
<img src="@/assets/logo_lark.svg" style="width: 100%" alt="" />
</AppAvatar>
<AppAvatar
v-else-if="!item.dataset_id && item.type === '0'"
class="mr-12 avatar-blue"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There appears to be an indentation issue after v-else, which is preventing it from being recognized in Vue.js. Additionally, the shape attribute is not necessary here since no rounded corners are intended. Here's the corrected version:

<AppAvatar
  v-else-if="!item.dataset_id && item.type === '2'"
  class="mr-8 avatar-purple"
  :size="24"
  style="background: none"
>
  <img src="@/assets/logo_lark.svg" style="width: 100%" alt="" />
</AppAvatar>

<AppAvatar
  v-else-if="!item.dataset_id && item.type === '0'"
  class="mr-12 avatar-blue"
  :size="24"
  style="background: none"
>
  <img src="@/assets/icon_user_profile.svg" style="width: 100%" alt="" />
</AppAvatar>

Make sure there are spaces between each attribute and close every tag properly for syntax correctness. Also ensure that the avatar-purple and avatar-blue classes are defined somewhere in your project for styling purposes.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/views/log/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@
<img src="@/assets/icon_web.svg" style="width: 58%" alt="" />
</AppAvatar>
<AppAvatar
v-if="!item.dataset_id && item.type === '2'"
v-else-if="!item.dataset_id && item.type === '2'"
class="mr-12 avatar-purple"
shape="square"
:size="24"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here's a review of the code change:

Improvement suggestion 1: Simplify the v-else-if condition.
The current condition is unnecessarily complex, as it checks both conditions (&&) rather than either ||. Here’s an optimized version:

v-else-if="(item.dataset_id && item.type !== "2") || (!item.dataset_id && item.type !=="0")

This way, you only need to check whether item.dataset_id (if available) or not is '2'.

General improvement suggestion:
Consider using constants (strings/constants variables at top level) for string literals like 'icon_web.svg' and 'avatar-purple'. This makes the code cleaner.

Overall note:
Make sure that all dependencies in this component are imported correctly since there appears to be something wrong with the relative path /assets/icon_web.svg if used without specifying its full path. Adjust accordingly based on your project structure and server setup.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/views/paragraph/component/SelectDocumentDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<img src="@/assets/icon_web.svg" style="width: 58%" alt="" />
</AppAvatar>
<AppAvatar
v-if="!item.dataset_id && item.type === '2'"
v-else-if="!item.dataset_id && item.type === '2'"
class="mr-12 avatar-purple"
shape="square"
:size="24"
Expand Down
2 changes: 1 addition & 1 deletion ui/src/workflow/nodes/search-dataset-node/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<img src="@/assets/icon_web.svg" style="width: 58%" alt="" />
</AppAvatar>
<AppAvatar
v-if="relatedObject(datasetList, item, 'id')?.type === '2'"
v-else-if="relatedObject(datasetList, item, 'id')?.type === '2'"
class="mr-8 avatar-purple"
shape="square"
:size="20"
Expand Down