mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-28 18:22:40 +00:00
feat(assets): add ModelInfoPanel for asset browser right panel (#8090)
## Summary Adds an editable Model Info Panel to show and modify asset details in the asset browser. ## Changes - Add `ModelInfoPanel` component with editable display name, description, model type, base models, and tags - Add `updateAssetMetadata` action in `assetsStore` with optimistic cache updates - Add shadcn-vue `Select` components with design system styling - Add utility functions in `assetMetadataUtils` for extracting model metadata - Convert `BaseModalLayout` right panel state to `defineModel` pattern - Add slide-in animation and collapse button for right panel - Add `class` prop to `PropertiesAccordionItem` for custom styling - Fix keyboard handling: Escape in TagsInput/TextArea doesn't close parent modal ## Testing - Unit tests for `ModelInfoPanel` component - Unit tests for `assetMetadataUtils` functions --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
Alexander Brown
@@ -2,8 +2,16 @@ import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
|
||||
import {
|
||||
getAssetAdditionalTags,
|
||||
getAssetBaseModel,
|
||||
getAssetDescription
|
||||
getAssetBaseModels,
|
||||
getAssetDescription,
|
||||
getAssetDisplayName,
|
||||
getAssetModelType,
|
||||
getAssetSourceUrl,
|
||||
getAssetTriggerPhrases,
|
||||
getAssetUserDescription,
|
||||
getSourceName
|
||||
} from '@/platform/assets/utils/assetMetadataUtils'
|
||||
|
||||
describe('assetMetadataUtils', () => {
|
||||
@@ -20,20 +28,17 @@ describe('assetMetadataUtils', () => {
|
||||
}
|
||||
|
||||
describe('getAssetDescription', () => {
|
||||
it('should return string description when present', () => {
|
||||
const asset = {
|
||||
...mockAsset,
|
||||
user_metadata: { description: 'A test model' }
|
||||
}
|
||||
expect(getAssetDescription(asset)).toBe('A test model')
|
||||
})
|
||||
|
||||
it('should return null when description is not a string', () => {
|
||||
const asset = {
|
||||
...mockAsset,
|
||||
user_metadata: { description: 123 }
|
||||
}
|
||||
expect(getAssetDescription(asset)).toBeNull()
|
||||
it.for([
|
||||
{
|
||||
name: 'returns string description when present',
|
||||
description: 'A test model',
|
||||
expected: 'A test model'
|
||||
},
|
||||
{ name: 'returns null for non-string', description: 123, expected: null },
|
||||
{ name: 'returns null for null', description: null, expected: null }
|
||||
])('$name', ({ description, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata: { description } }
|
||||
expect(getAssetDescription(asset)).toBe(expected)
|
||||
})
|
||||
|
||||
it('should return null when no metadata', () => {
|
||||
@@ -42,24 +47,228 @@ describe('assetMetadataUtils', () => {
|
||||
})
|
||||
|
||||
describe('getAssetBaseModel', () => {
|
||||
it('should return string base_model when present', () => {
|
||||
const asset = {
|
||||
...mockAsset,
|
||||
user_metadata: { base_model: 'SDXL' }
|
||||
}
|
||||
expect(getAssetBaseModel(asset)).toBe('SDXL')
|
||||
})
|
||||
|
||||
it('should return null when base_model is not a string', () => {
|
||||
const asset = {
|
||||
...mockAsset,
|
||||
user_metadata: { base_model: 123 }
|
||||
}
|
||||
expect(getAssetBaseModel(asset)).toBeNull()
|
||||
it.for([
|
||||
{
|
||||
name: 'returns string base_model when present',
|
||||
base_model: 'SDXL',
|
||||
expected: 'SDXL'
|
||||
},
|
||||
{ name: 'returns null for non-string', base_model: 123, expected: null },
|
||||
{ name: 'returns null for null', base_model: null, expected: null }
|
||||
])('$name', ({ base_model, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata: { base_model } }
|
||||
expect(getAssetBaseModel(asset)).toBe(expected)
|
||||
})
|
||||
|
||||
it('should return null when no metadata', () => {
|
||||
expect(getAssetBaseModel(mockAsset)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('getAssetDisplayName', () => {
|
||||
it.for([
|
||||
{
|
||||
name: 'returns name from user_metadata when present',
|
||||
user_metadata: { name: 'My Custom Name' },
|
||||
expected: 'My Custom Name'
|
||||
},
|
||||
{
|
||||
name: 'falls back to asset name for non-string',
|
||||
user_metadata: { name: 123 },
|
||||
expected: 'test-model'
|
||||
},
|
||||
{
|
||||
name: 'falls back to asset name for undefined',
|
||||
user_metadata: undefined,
|
||||
expected: 'test-model'
|
||||
}
|
||||
])('$name', ({ user_metadata, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata }
|
||||
expect(getAssetDisplayName(asset)).toBe(expected)
|
||||
})
|
||||
})
|
||||
|
||||
describe('getAssetSourceUrl', () => {
|
||||
it.for([
|
||||
{
|
||||
name: 'constructs URL from civitai format',
|
||||
source_arn: 'civitai:model:123:version:456',
|
||||
expected: 'https://civitai.com/models/123?modelVersionId=456'
|
||||
},
|
||||
{ name: 'returns null for non-string', source_arn: 123, expected: null },
|
||||
{
|
||||
name: 'returns null for unrecognized format',
|
||||
source_arn: 'unknown:format',
|
||||
expected: null
|
||||
}
|
||||
])('$name', ({ source_arn, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata: { source_arn } }
|
||||
expect(getAssetSourceUrl(asset)).toBe(expected)
|
||||
})
|
||||
|
||||
it('should return null when no metadata', () => {
|
||||
expect(getAssetSourceUrl(mockAsset)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('getAssetTriggerPhrases', () => {
|
||||
it.for([
|
||||
{
|
||||
name: 'returns array when array present',
|
||||
trained_words: ['phrase1', 'phrase2'],
|
||||
expected: ['phrase1', 'phrase2']
|
||||
},
|
||||
{
|
||||
name: 'wraps single string in array',
|
||||
trained_words: 'single phrase',
|
||||
expected: ['single phrase']
|
||||
},
|
||||
{
|
||||
name: 'filters non-string values from array',
|
||||
trained_words: ['valid', 123, 'also valid', null],
|
||||
expected: ['valid', 'also valid']
|
||||
}
|
||||
])('$name', ({ trained_words, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata: { trained_words } }
|
||||
expect(getAssetTriggerPhrases(asset)).toEqual(expected)
|
||||
})
|
||||
|
||||
it('should return empty array when no metadata', () => {
|
||||
expect(getAssetTriggerPhrases(mockAsset)).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('getAssetAdditionalTags', () => {
|
||||
it.for([
|
||||
{
|
||||
name: 'returns array of tags when present',
|
||||
additional_tags: ['tag1', 'tag2'],
|
||||
expected: ['tag1', 'tag2']
|
||||
},
|
||||
{
|
||||
name: 'filters non-string values from array',
|
||||
additional_tags: ['valid', 123, 'also valid'],
|
||||
expected: ['valid', 'also valid']
|
||||
},
|
||||
{
|
||||
name: 'returns empty array for non-array',
|
||||
additional_tags: 'not an array',
|
||||
expected: []
|
||||
}
|
||||
])('$name', ({ additional_tags, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata: { additional_tags } }
|
||||
expect(getAssetAdditionalTags(asset)).toEqual(expected)
|
||||
})
|
||||
|
||||
it('should return empty array when no metadata', () => {
|
||||
expect(getAssetAdditionalTags(mockAsset)).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('getSourceName', () => {
|
||||
it.for([
|
||||
{
|
||||
name: 'returns Civitai for civitai.com',
|
||||
url: 'https://civitai.com/models/123',
|
||||
expected: 'Civitai'
|
||||
},
|
||||
{
|
||||
name: 'returns Hugging Face for huggingface.co',
|
||||
url: 'https://huggingface.co/org/model',
|
||||
expected: 'Hugging Face'
|
||||
},
|
||||
{
|
||||
name: 'returns Source for unknown URLs',
|
||||
url: 'https://example.com/model',
|
||||
expected: 'Source'
|
||||
}
|
||||
])('$name', ({ url, expected }) => {
|
||||
expect(getSourceName(url)).toBe(expected)
|
||||
})
|
||||
})
|
||||
|
||||
describe('getAssetBaseModels', () => {
|
||||
it.for([
|
||||
{
|
||||
name: 'array of strings',
|
||||
base_model: ['SDXL', 'SD1.5', 'Flux'],
|
||||
expected: ['SDXL', 'SD1.5', 'Flux']
|
||||
},
|
||||
{
|
||||
name: 'filters non-string entries',
|
||||
base_model: ['SDXL', 123, 'SD1.5', null, undefined],
|
||||
expected: ['SDXL', 'SD1.5']
|
||||
},
|
||||
{
|
||||
name: 'single string wrapped in array',
|
||||
base_model: 'SDXL',
|
||||
expected: ['SDXL']
|
||||
},
|
||||
{
|
||||
name: 'non-array/string returns empty',
|
||||
base_model: 123,
|
||||
expected: []
|
||||
},
|
||||
{ name: 'undefined returns empty', base_model: undefined, expected: [] }
|
||||
])('$name', ({ base_model, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata: { base_model } }
|
||||
expect(getAssetBaseModels(asset)).toEqual(expected)
|
||||
})
|
||||
|
||||
it('should return empty array when no metadata', () => {
|
||||
expect(getAssetBaseModels(mockAsset)).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('getAssetModelType', () => {
|
||||
it.for([
|
||||
{
|
||||
name: 'returns model type from tags',
|
||||
tags: ['models', 'checkpoints'],
|
||||
expected: 'checkpoints'
|
||||
},
|
||||
{
|
||||
name: 'extracts last segment from path-style tags',
|
||||
tags: ['models', 'models/loras'],
|
||||
expected: 'loras'
|
||||
},
|
||||
{
|
||||
name: 'returns null when only models tag',
|
||||
tags: ['models'],
|
||||
expected: null
|
||||
},
|
||||
{ name: 'returns null when tags empty', tags: [], expected: null }
|
||||
])('$name', ({ tags, expected }) => {
|
||||
const asset = { ...mockAsset, tags }
|
||||
expect(getAssetModelType(asset)).toBe(expected)
|
||||
})
|
||||
})
|
||||
|
||||
describe('getAssetUserDescription', () => {
|
||||
it.for([
|
||||
{
|
||||
name: 'returns description when present',
|
||||
user_description: 'A custom user description',
|
||||
expected: 'A custom user description'
|
||||
},
|
||||
{
|
||||
name: 'returns empty for non-string',
|
||||
user_description: 123,
|
||||
expected: ''
|
||||
},
|
||||
{ name: 'returns empty for null', user_description: null, expected: '' },
|
||||
{
|
||||
name: 'returns empty for undefined',
|
||||
user_description: undefined,
|
||||
expected: ''
|
||||
}
|
||||
])('$name', ({ user_description, expected }) => {
|
||||
const asset = { ...mockAsset, user_metadata: { user_description } }
|
||||
expect(getAssetUserDescription(asset)).toBe(expected)
|
||||
})
|
||||
|
||||
it('should return empty string when no metadata', () => {
|
||||
expect(getAssetUserDescription(mockAsset)).toBe('')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,27 +1,151 @@
|
||||
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
|
||||
|
||||
/**
|
||||
* Type-safe utilities for extracting metadata from assets
|
||||
* Type-safe utilities for extracting metadata from assets.
|
||||
* These utilities check user_metadata first, then metadata, then fallback.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Helper to get a string property from user_metadata or metadata
|
||||
*/
|
||||
function getStringProperty(asset: AssetItem, key: string): string | undefined {
|
||||
const userValue = asset.user_metadata?.[key]
|
||||
if (typeof userValue === 'string') return userValue
|
||||
|
||||
const metaValue = asset.metadata?.[key]
|
||||
if (typeof metaValue === 'string') return metaValue
|
||||
|
||||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Safely extracts string description from asset metadata
|
||||
* Checks user_metadata first, then metadata, then returns null
|
||||
* @param asset - The asset to extract description from
|
||||
* @returns The description string or null if not present/not a string
|
||||
*/
|
||||
export function getAssetDescription(asset: AssetItem): string | null {
|
||||
return typeof asset.user_metadata?.description === 'string'
|
||||
? asset.user_metadata.description
|
||||
: null
|
||||
return getStringProperty(asset, 'description') ?? null
|
||||
}
|
||||
|
||||
/**
|
||||
* Safely extracts string base_model from asset metadata
|
||||
* Checks user_metadata first, then metadata, then returns null
|
||||
* @param asset - The asset to extract base_model from
|
||||
* @returns The base_model string or null if not present/not a string
|
||||
*/
|
||||
export function getAssetBaseModel(asset: AssetItem): string | null {
|
||||
return typeof asset.user_metadata?.base_model === 'string'
|
||||
? asset.user_metadata.base_model
|
||||
: null
|
||||
return getStringProperty(asset, 'base_model') ?? null
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts base models as an array from asset metadata
|
||||
* Checks user_metadata first, then metadata, then returns empty array
|
||||
* @param asset - The asset to extract base models from
|
||||
* @returns Array of base model strings
|
||||
*/
|
||||
export function getAssetBaseModels(asset: AssetItem): string[] {
|
||||
const baseModel =
|
||||
asset.user_metadata?.base_model ?? asset.metadata?.base_model
|
||||
if (Array.isArray(baseModel)) {
|
||||
return baseModel.filter((m): m is string => typeof m === 'string')
|
||||
}
|
||||
if (typeof baseModel === 'string' && baseModel) {
|
||||
return [baseModel]
|
||||
}
|
||||
return []
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the display name for an asset
|
||||
* Checks user_metadata.name first, then metadata.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
|
||||
}
|
||||
|
||||
/**
|
||||
* Constructs source URL from asset's source_arn
|
||||
* @param asset - The asset to extract source URL from
|
||||
* @returns The source URL or null if not present/parseable
|
||||
*/
|
||||
export function getAssetSourceUrl(asset: AssetItem): string | null {
|
||||
// Note: Reversed priority for backwards compatibility
|
||||
const sourceArn =
|
||||
asset.metadata?.source_arn ?? asset.user_metadata?.source_arn
|
||||
if (typeof sourceArn !== 'string') return null
|
||||
|
||||
const civitaiMatch = sourceArn.match(
|
||||
/^civitai:model:(\d+):version:(\d+)(?::file:\d+)?$/
|
||||
)
|
||||
if (civitaiMatch) {
|
||||
const [, modelId, versionId] = civitaiMatch
|
||||
return `https://civitai.com/models/${modelId}?modelVersionId=${versionId}`
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts trigger phrases from asset metadata
|
||||
* Checks user_metadata first, then metadata, then returns empty array
|
||||
* @param asset - The asset to extract trigger phrases from
|
||||
* @returns Array of trigger phrases
|
||||
*/
|
||||
export function getAssetTriggerPhrases(asset: AssetItem): string[] {
|
||||
const phrases =
|
||||
asset.user_metadata?.trained_words ?? asset.metadata?.trained_words
|
||||
if (Array.isArray(phrases)) {
|
||||
return phrases.filter((p): p is string => typeof p === 'string')
|
||||
}
|
||||
if (typeof phrases === 'string') return [phrases]
|
||||
return []
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts additional tags from asset user_metadata
|
||||
* @param asset - The asset to extract tags from
|
||||
* @returns Array of user-defined tags
|
||||
*/
|
||||
export function getAssetAdditionalTags(asset: AssetItem): string[] {
|
||||
const tags = asset.user_metadata?.additional_tags
|
||||
if (Array.isArray(tags)) {
|
||||
return tags.filter((t): t is string => typeof t === 'string')
|
||||
}
|
||||
return []
|
||||
}
|
||||
|
||||
/**
|
||||
* Determines the source name from a URL
|
||||
* @param url - The source URL
|
||||
* @returns Human-readable source name
|
||||
*/
|
||||
export function getSourceName(url: string): string {
|
||||
if (url.includes('civitai.com')) return 'Civitai'
|
||||
if (url.includes('huggingface.co')) return 'Hugging Face'
|
||||
return 'Source'
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the model type from asset tags
|
||||
* @param asset - The asset to extract model type from
|
||||
* @returns The model type string or null if not present
|
||||
*/
|
||||
export function getAssetModelType(asset: AssetItem): string | null {
|
||||
const typeTag = asset.tags?.find((tag) => tag && tag !== 'models')
|
||||
if (!typeTag) return null
|
||||
return typeTag.includes('/') ? (typeTag.split('/').pop() ?? null) : typeTag
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts user description from asset user_metadata
|
||||
* @param asset - The asset to extract user description from
|
||||
* @returns The user description string or empty string if not present
|
||||
*/
|
||||
export function getAssetUserDescription(asset: AssetItem): string {
|
||||
return typeof asset.user_metadata?.user_description === 'string'
|
||||
? asset.user_metadata.user_description
|
||||
: ''
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user