mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-12 16:40:05 +00:00
refactor: centralize display_name || name into getAssetDisplayName (#9641)
## Summary Centralize the inline `display_name || name` pattern into `getAssetDisplayName`, adding `display_name` to the existing metadata fallback chain. ## Changes - **What**: Update `getAssetDisplayName` fallback chain to `user_metadata.name → metadata.name → display_name → name`. Replace all 6 inline `asset.display_name || asset.name` call sites with the shared utility. Remove duplicate local function in `AssetsSidebarListView.vue`. ## Review Focus The fallback order preserves user_metadata overrides while incorporating the `display_name` field added in #9626. All callers now go through a single code path. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9641-refactor-centralize-display_name-name-into-getAssetDisplayName-31e6d73d365081e09e5de85486583443) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
@@ -90,6 +90,7 @@ import AssetsListItem from '@/platform/assets/components/AssetsListItem.vue'
|
||||
import type { OutputStackListItem } from '@/platform/assets/composables/useOutputStacks'
|
||||
import { getOutputAssetMetadata } from '@/platform/assets/schemas/assetMetadataSchema'
|
||||
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
|
||||
import { getAssetDisplayName } from '@/platform/assets/utils/assetMetadataUtils'
|
||||
import { iconForMediaType } from '@/platform/assets/utils/mediaIconUtil'
|
||||
import { useAssetsStore } from '@/stores/assetsStore'
|
||||
import {
|
||||
@@ -135,10 +136,6 @@ const listGridStyle = {
|
||||
gap: '0.5rem'
|
||||
}
|
||||
|
||||
function getAssetDisplayName(asset: AssetItem): string {
|
||||
return asset.display_name || asset.name
|
||||
}
|
||||
|
||||
function getAssetPrimaryText(asset: AssetItem): string {
|
||||
return truncateFilename(getAssetDisplayName(asset))
|
||||
}
|
||||
|
||||
@@ -236,6 +236,7 @@ import { useOutputStacks } from '@/platform/assets/composables/useOutputStacks'
|
||||
import type { OutputAssetMetadata } from '@/platform/assets/schemas/assetMetadataSchema'
|
||||
import { getOutputAssetMetadata } from '@/platform/assets/schemas/assetMetadataSchema'
|
||||
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
|
||||
import { getAssetDisplayName } from '@/platform/assets/utils/assetMetadataUtils'
|
||||
import type { MediaKind } from '@/platform/assets/schemas/mediaAssetSchema'
|
||||
import { resolveOutputAssetItems } from '@/platform/assets/utils/outputAssetUtil'
|
||||
import { isCloud } from '@/platform/distribution/types'
|
||||
@@ -569,7 +570,7 @@ const handleZoomClick = (asset: AssetItem) => {
|
||||
const dialogStore = useDialogStore()
|
||||
dialogStore.showDialog({
|
||||
key: 'asset-3d-viewer',
|
||||
title: asset.display_name || asset.name,
|
||||
title: getAssetDisplayName(asset),
|
||||
component: Load3dViewerContent,
|
||||
props: {
|
||||
modelUrl: asset.preview_url || ''
|
||||
|
||||
@@ -186,7 +186,7 @@ const tooltipDelay = computed<number>(() =>
|
||||
|
||||
const { isLoading, error } = useImage({
|
||||
src: asset.preview_url ?? '',
|
||||
alt: asset.display_name || asset.name
|
||||
alt: displayName.value
|
||||
})
|
||||
|
||||
function handleSelect() {
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
:aria-label="
|
||||
asset
|
||||
? $t('assetBrowser.ariaLabel.assetCard', {
|
||||
name: asset.display_name || asset.name,
|
||||
name: getAssetDisplayName(asset),
|
||||
type: fileKind
|
||||
})
|
||||
: $t('assetBrowser.ariaLabel.loadingAsset')
|
||||
@@ -152,6 +152,7 @@ import { cn } from '@/utils/tailwindUtil'
|
||||
import { getAssetType } from '../composables/media/assetMappers'
|
||||
import { useMediaAssetActions } from '../composables/useMediaAssetActions'
|
||||
import type { AssetItem } from '../schemas/assetSchema'
|
||||
import { getAssetDisplayName } from '../utils/assetMetadataUtils'
|
||||
import type { MediaKind } from '../schemas/mediaAssetSchema'
|
||||
import { MediaAssetKey } from '../schemas/mediaAssetSchema'
|
||||
import MediaTitle from './MediaTitle.vue'
|
||||
@@ -225,7 +226,7 @@ const canInspect = computed(() => isPreviewableMediaType(fileKind.value))
|
||||
|
||||
// Get filename without extension
|
||||
const fileName = computed(() => {
|
||||
return getFilenameDetails(asset?.display_name || asset?.name || '').filename
|
||||
return getFilenameDetails(asset ? getAssetDisplayName(asset) : '').filename
|
||||
})
|
||||
|
||||
// Adapt AssetItem to legacy AssetMeta format for existing components
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
<img
|
||||
v-if="!error"
|
||||
:src="asset.src"
|
||||
:alt="asset.display_name || asset.name"
|
||||
:alt="getAssetDisplayName(asset)"
|
||||
class="size-full object-contain transition-transform duration-300 group-hover:scale-105 group-data-[selected=true]:scale-105"
|
||||
/>
|
||||
<div
|
||||
@@ -22,6 +22,7 @@
|
||||
import { useImage, whenever } from '@vueuse/core'
|
||||
|
||||
import type { AssetMeta } from '../schemas/mediaAssetSchema'
|
||||
import { getAssetDisplayName } from '../utils/assetMetadataUtils'
|
||||
|
||||
const { asset } = defineProps<{
|
||||
asset: AssetMeta
|
||||
@@ -34,7 +35,7 @@ const emit = defineEmits<{
|
||||
|
||||
const { state, error, isReady } = useImage({
|
||||
src: asset.src ?? '',
|
||||
alt: asset.display_name || asset.name
|
||||
alt: getAssetDisplayName(asset)
|
||||
})
|
||||
|
||||
whenever(
|
||||
|
||||
@@ -14,6 +14,7 @@ import { useNodeDefStore } from '@/stores/nodeDefStore'
|
||||
import { getOutputAssetMetadata } from '../schemas/assetMetadataSchema'
|
||||
import { useAssetsStore } from '@/stores/assetsStore'
|
||||
import { useDialogStore } from '@/stores/dialogStore'
|
||||
import { getAssetDisplayName } from '../utils/assetMetadataUtils'
|
||||
import { getAssetType } from '../utils/assetTypeUtil'
|
||||
import { getAssetUrl } from '../utils/assetUrlUtil'
|
||||
import { createAnnotatedPath } from '@/utils/createAnnotatedPath'
|
||||
@@ -68,7 +69,7 @@ export function useMediaAssetActions() {
|
||||
if (!targetAsset) return
|
||||
|
||||
try {
|
||||
const filename = targetAsset.display_name || targetAsset.name
|
||||
const filename = getAssetDisplayName(targetAsset)
|
||||
// Prefer preview_url (already includes subfolder) with getAssetUrl as fallback
|
||||
const downloadUrl = targetAsset.preview_url || getAssetUrl(targetAsset)
|
||||
|
||||
@@ -109,7 +110,7 @@ export function useMediaAssetActions() {
|
||||
|
||||
try {
|
||||
assets.forEach((asset) => {
|
||||
const filename = asset.display_name || asset.name
|
||||
const filename = getAssetDisplayName(asset)
|
||||
const downloadUrl = asset.preview_url || getAssetUrl(asset)
|
||||
downloadFile(downloadUrl, filename)
|
||||
})
|
||||
|
||||
@@ -70,20 +70,35 @@ describe('assetMetadataUtils', () => {
|
||||
{
|
||||
name: 'returns name from user_metadata when present',
|
||||
user_metadata: { name: 'My Custom Name' },
|
||||
display_name: 'ComfyUI_00001_.png',
|
||||
expected: 'My Custom Name'
|
||||
},
|
||||
{
|
||||
name: 'falls back to asset name for non-string',
|
||||
user_metadata: { name: 123 },
|
||||
name: 'returns display_name when user_metadata.name is absent',
|
||||
user_metadata: undefined,
|
||||
display_name: 'ComfyUI_00001_.png',
|
||||
expected: 'ComfyUI_00001_.png'
|
||||
},
|
||||
{
|
||||
name: 'falls back to asset name when both are absent',
|
||||
user_metadata: undefined,
|
||||
display_name: undefined,
|
||||
expected: 'test-model'
|
||||
},
|
||||
{
|
||||
name: 'falls back to asset name for undefined',
|
||||
name: 'skips non-string user_metadata.name',
|
||||
user_metadata: { name: 123 },
|
||||
display_name: 'ComfyUI_00001_.png',
|
||||
expected: 'ComfyUI_00001_.png'
|
||||
},
|
||||
{
|
||||
name: 'falls back to asset name when display_name is empty',
|
||||
user_metadata: undefined,
|
||||
display_name: '',
|
||||
expected: 'test-model'
|
||||
}
|
||||
])('$name', ({ user_metadata, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata }
|
||||
])('$name', ({ user_metadata, display_name, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata, display_name }
|
||||
expect(getAssetDisplayName(asset)).toBe(expected)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -58,12 +58,12 @@ export function getAssetBaseModels(asset: AssetItem): string[] {
|
||||
|
||||
/**
|
||||
* Gets the display name for an asset
|
||||
* Checks user_metadata.name first, then metadata.name, then asset.name
|
||||
* Checks user_metadata.name, then metadata.name, then display_name, then asset.name
|
||||
* @param asset - The asset to get display name from
|
||||
* @returns The display name
|
||||
*/
|
||||
export function getAssetDisplayName(asset: AssetItem): string {
|
||||
return getStringProperty(asset, 'name') ?? asset.name
|
||||
return getStringProperty(asset, 'name') || asset.display_name || asset.name
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user