Skip to content
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

fix: default image when nft to send has no image #580

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: default image when nft to send has no image #580

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
3 changes: 2 additions & 1 deletion .../providers/common/ui/send-transaction/nft-select-list/components/nft-select-list-item.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<template>
<a class="nft-select-list__token" @click="$emit('selectNft', item)">
<div class="nft-select-list__token-info">
<img :src="item.image" />
<img :src="item.image" @error="imageLoadError" />

<div class="nft-select-list__token-info-name">
<h4>
Expand All @@ -26,6 +26,7 @@
<script setup lang="ts">
import { PropType } from 'vue';
import { NFTItemWithCollectionName } from '@/types/nft';
import { imageLoadError } from '@/ui/action/utils/misc';
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

Prop validation can be improved but type definition is correct

Based on the codebase analysis, the NFTItemWithCollectionName interface extends NFTItem which has the required image property as a string. The type definition is correct and ensures type safety. However, the default empty object prop could be improved.

defineProps({
  item: {
    type: Object as PropType<NFTItemWithCollectionName>,
-    default: () => ({}),
+    required: true
  },
});

The imageLoadError utility is correctly imported and used with the properly typed NFT interface.

🔗 Analysis chain

Verify NFT type definition and add prop validation

The default empty object for the item prop could lead to undefined behavior. Let's verify the type definition and add validation.

Consider adding required validation:

 defineProps({
   item: {
     type: Object as PropType<NFTItemWithCollectionName>,
-    default: () => ({}),
+    required: true,
+    validator: (value: NFTItemWithCollectionName) => {
+      return typeof value.image === 'string';
+    }
   },
 });

Also applies to: 33-39

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check NFT type definition for image property
ast-grep --pattern 'interface NFTItemWithCollectionName {
  $$$
  image: $_
  $$$
}'

Length of output: 86

Script:

#!/bin/bash
# Let's try to find the type definition using ripgrep
rg "NFTItemWithCollectionName" -A 5 -B 5

# Also search for any similar interface definitions
rg "interface.*NFT.*Item" -A 5 -B 5

Length of output: 30415


defineEmits<{
(e: 'selectNft', data: NFTItemWithCollectionName): void;
Expand Down
3 changes: 2 additions & 1 deletion packages/extension/src/providers/common/ui/send-transaction/send-nft-select.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div class="send-nft-select__wrap">
<a class="send-nft-select" @click="$emit('toggleSelect', item.id !== '')">
<div class="send-nft-select__image">
<img v-if="item.image !== ''" :src="item.image" alt="" />
<img :src="item.image" alt="" @error="imageLoadError" />
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Inconsistent image error handling detected

There are two <img> tags in the template with different approaches to handling missing images:

  1. The first image uses @error="imageLoadError"
  2. The second image uses v-if="item.image !== ''"

This inconsistency could lead to different behaviors. Consider applying the same error handling approach to both images.

Apply this diff to make the handling consistent:

        <img :src="item.image" alt="" @error="imageLoadError" />
        ...
      <img
-       v-if="item.image !== ''"
        class="send-nft-select__view"
        :src="item.image"
        alt=""
+       @error="imageLoadError"
      />

Also applies to: 31-35

</div>
<div class="send-nft-select__info">
<h5>
Expand Down Expand Up @@ -43,6 +43,7 @@
import { PropType } from 'vue';
import SwitchArrow from '@action/icons/header/switch_arrow.vue';
import { NFTItemWithCollectionName } from '@/types/nft';
import { imageLoadError } from '@/ui/action/utils/misc';

defineEmits<{
(e: 'toggleSelect', val: boolean): void;
Expand Down
Loading