From 13ce23399c9af9f011051b2527f559d337151bfe Mon Sep 17 00:00:00 2001 From: Arjan Singh <1598641+arjansingh@users.noreply.github.com> Date: Thu, 25 Sep 2025 11:17:26 -0700 Subject: [PATCH] Asset Browser Design Review + Filters (#5737) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixed design feedback and wired up the filter controls. ## Review Focus Design Feedback: - [4872888](https://github.com/Comfy-Org/ComfyUI_frontend/pull/5737/commits/48728881afe97a7766a8e17f5d8a78842b8b4685) - [9a0b63e](https://github.com/Comfy-Org/ComfyUI_frontend/pull/5737/commits/9a0b63edce3800d9c87fda332ef679bcfc3e09c1) Filters Hookup: - [07f22f8](https://github.com/Comfy-Org/ComfyUI_frontend/pull/5737/commits/07f22f8074041df5e9f6c0c58f6a4352ddcac7a4) Misc (can focus less on): - claude guidance: [23e6fa9](https://github.com/Comfy-Org/ComfyUI_frontend/pull/5737/commits/23e6fa97239b5c68f23edba71993865b2bb86719) - test helpers: [7801ed9](https://github.com/Comfy-Org/ComfyUI_frontend/pull/5737/commits/7801ed9e28ebc6c136ac5f14112fa630e0897a52) ## Screenshots (if applicable) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5737-Asset-Browser-Design-Review-Filters-2776d73d3650813e890bd16fa6a0433f) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown Co-authored-by: GitHub Action --- .../widget/layout/BaseModalLayout.vue | 4 +- src/components/widget/nav/NavItem.vue | 4 +- src/components/widget/panel/LeftSidePanel.vue | 2 +- .../widget/panel/RightSidePanel.vue | 2 +- .../components/AssetBrowserModal.stories.ts | 2 +- .../assets/components/AssetBrowserModal.vue | 12 +- src/platform/assets/components/AssetCard.vue | 20 ++- .../components/AssetFilterBar.stories.ts | 135 +++++++++++++++++ .../assets/components/AssetFilterBar.vue | 29 ++-- .../assets/composables/useAssetBrowser.ts | 113 +++++++++------ .../composables/useAssetBrowserDialog.ts | 4 +- .../assets/fixtures/ui-mock-assets.ts | 31 ++++ tests-ui/CLAUDE.md | 10 ++ .../components/AssetBrowserModal.test.ts | 26 ++++ .../assets/components/AssetFilterBar.test.ts | 136 +++++++++++++++++- .../composables/useAssetBrowser.test.ts | 8 +- .../composables/useAssetFilterOptions.test.ts | 65 ++++----- 17 files changed, 473 insertions(+), 130 deletions(-) create mode 100644 src/platform/assets/components/AssetFilterBar.stories.ts diff --git a/src/components/widget/layout/BaseModalLayout.vue b/src/components/widget/layout/BaseModalLayout.vue index c455f62ee..6c30d7dde 100644 --- a/src/components/widget/layout/BaseModalLayout.vue +++ b/src/components/widget/layout/BaseModalLayout.vue @@ -127,7 +127,7 @@ const toggleRightPanel = () => { const layoutClasses = cn( 'base-widget-layout', 'rounded-2xl overflow-hidden relative', - 'bg-zinc-50 dark-theme:bg-zinc-800' + 'bg-gray-50 dark-theme:bg-gray-800' ) const rightPanelButtonClasses = computed(() => { @@ -144,7 +144,7 @@ const closeButtonClasses = cn( const mainContainerClasses = cn( 'flex-1 flex', - 'bg-zinc-100 dark-theme:bg-neutral-900' + 'bg-gray-100 dark-theme:bg-neutral-900' ) const headerClasses = cn( diff --git a/src/components/widget/nav/NavItem.vue b/src/components/widget/nav/NavItem.vue index 0b7532050..f5bd09431 100644 --- a/src/components/widget/nav/NavItem.vue +++ b/src/components/widget/nav/NavItem.vue @@ -3,8 +3,8 @@ class="flex items-center gap-2 px-4 py-3 text-sm rounded-md transition-colors cursor-pointer" :class=" active - ? 'bg-neutral-100 dark-theme:bg-zinc-700 text-neutral' - : 'text-neutral hover:bg-zinc-100 dark-theme:hover:bg-zinc-700/50' + ? 'bg-white dark-theme:bg-charcoal-600 text-neutral' + : 'text-neutral hover:bg-gray-100 dark-theme:hover:bg-charcoal-300' " role="button" @click="onClick" diff --git a/src/components/widget/panel/LeftSidePanel.vue b/src/components/widget/panel/LeftSidePanel.vue index 964753365..892e37a0b 100644 --- a/src/components/widget/panel/LeftSidePanel.vue +++ b/src/components/widget/panel/LeftSidePanel.vue @@ -1,5 +1,5 @@ - + diff --git a/src/components/widget/panel/RightSidePanel.vue b/src/components/widget/panel/RightSidePanel.vue index 44e6b7979..266f22910 100644 --- a/src/components/widget/panel/RightSidePanel.vue +++ b/src/components/widget/panel/RightSidePanel.vue @@ -1,5 +1,5 @@ - + diff --git a/src/platform/assets/components/AssetBrowserModal.stories.ts b/src/platform/assets/components/AssetBrowserModal.stories.ts index 9d2321d57..f062fe71f 100644 --- a/src/platform/assets/components/AssetBrowserModal.stories.ts +++ b/src/platform/assets/components/AssetBrowserModal.stories.ts @@ -52,7 +52,7 @@ export const Default: Story = { nodeType: 'CheckpointLoaderSimple', inputName: 'ckpt_name', currentValue: '', - showLeftPanel: false + showLeftPanel: true }, render: (args) => ({ components: { AssetBrowserModal }, diff --git a/src/platform/assets/components/AssetBrowserModal.vue b/src/platform/assets/components/AssetBrowserModal.vue index cb45f38ba..1182fdfa9 100644 --- a/src/platform/assets/components/AssetBrowserModal.vue +++ b/src/platform/assets/components/AssetBrowserModal.vue @@ -27,6 +27,10 @@ /> + + + + { return props.showLeftPanel ?? true }) -const handleClose = () => { +function handleClose() { props.onClose?.() emit('close') } -const handleAssetSelectAndEmit = async (asset: AssetDisplayItem) => { +async function handleAssetSelectAndEmit(asset: AssetDisplayItem) { emit('asset-select', asset) await selectAssetWithCallback(asset.id, props.onSelect) } diff --git a/src/platform/assets/components/AssetCard.vue b/src/platform/assets/components/AssetCard.vue index be7c45ca5..90b8734bc 100644 --- a/src/platform/assets/components/AssetCard.vue +++ b/src/platform/assets/components/AssetCard.vue @@ -8,19 +8,17 @@ cn( // Base layout and container styles (always applied) 'rounded-xl overflow-hidden transition-all duration-200', + interactive && 'group', // Button-specific styles interactive && [ 'appearance-none bg-transparent p-0 m-0 font-inherit text-inherit outline-none cursor-pointer text-left', - 'bg-ivory-100 border border-gray-300 dark-theme:bg-charcoal-400 dark-theme:border-charcoal-600', - 'hover:transform hover:-translate-y-0.5 hover:shadow-lg hover:shadow-black/10 hover:border-gray-400', - 'dark-theme:hover:shadow-lg dark-theme:hover:shadow-black/30 dark-theme:hover:border-charcoal-700', - 'focus:outline-none focus:transform focus:-translate-y-0.5 focus:shadow-lg focus:shadow-black/10 dark-theme:focus:shadow-black/30' + 'bg-gray-100 dark-theme:bg-charcoal-800', + 'hover:bg-gray-200 dark-theme:hover:bg-charcoal-600', + 'border-none', + 'focus:outline-solid outline-blue-100 outline-4' ], // Div-specific styles - !interactive && [ - 'bg-ivory-100 border border-gray-300', - 'dark-theme:bg-charcoal-400 dark-theme:border-charcoal-600' - ] + !interactive && 'bg-gray-100 dark-theme:bg-charcoal-800' ) " @click="interactive && $emit('select', asset)" @@ -32,7 +30,7 @@ > - + = { + title: 'Platform/Assets/AssetFilterBar', + component: AssetFilterBar, + parameters: { + layout: 'fullscreen', + docs: { + description: { + component: + 'Filter bar for asset browser that dynamically shows/hides filters based on available options.' + } + } + }, + decorators: [ + () => ({ + template: ` + + + + + + Filter bar with proper chrome styling showing contextual background and borders. + + + ` + }) + ], + argTypes: { + assets: { + description: 'Array of assets to generate filter options from' + } + } +} + +export default meta +type Story = StoryObj + +export const BothFiltersVisible: Story = { + args: { + assets: [ + createAssetWithSpecificExtension('safetensors'), + createAssetWithSpecificExtension('ckpt'), + createAssetWithSpecificBaseModel('sd15'), + createAssetWithSpecificBaseModel('sdxl') + ] + }, + parameters: { + docs: { + description: { + story: + 'Shows both file format and base model filters when assets contain both types of options.' + } + } + } +} + +export const OnlyFileFormatFilter: Story = { + args: { + assets: [ + // Assets with extensions but explicitly NO base models + { + ...createAssetWithSpecificExtension('safetensors'), + user_metadata: undefined + }, + { ...createAssetWithSpecificExtension('ckpt'), user_metadata: undefined } + ] + }, + parameters: { + docs: { + description: { + story: + 'Shows only file format filter when assets have file extensions but no base model metadata.' + } + } + } +} + +export const OnlyBaseModelFilter: Story = { + args: { + assets: [ + // Assets with base models but no recognizable extensions + { + ...createAssetWithSpecificBaseModel('sd15'), + name: 'model_without_extension' + }, + { ...createAssetWithSpecificBaseModel('sdxl'), name: 'another_model' } + ] + }, + parameters: { + docs: { + description: { + story: + 'Shows only base model filter when assets have base model metadata but no recognizable file extensions.' + } + } + } +} + +export const NoFiltersVisible: Story = { + args: { + assets: [] + }, + parameters: { + docs: { + description: { + story: + 'Shows no filters when no assets are provided or assets contain no filterable options.' + } + } + } +} + +export const NoFiltersFromAssetsWithoutOptions: Story = { + args: { + assets: [createAssetWithoutExtension(), createAssetWithoutBaseModel()] + }, + parameters: { + docs: { + description: { + story: + 'Shows no filters when assets are provided but contain no filterable options (no extensions or base models).' + } + } + } +} diff --git a/src/platform/assets/components/AssetFilterBar.vue b/src/platform/assets/components/AssetFilterBar.vue index 904ce3e82..3ec844b30 100644 --- a/src/platform/assets/components/AssetFilterBar.vue +++ b/src/platform/assets/components/AssetFilterBar.vue @@ -2,18 +2,20 @@ () + const fileFormats = ref([]) const baseModels = ref([]) const sortBy = ref('name-asc') -// TODO: Make fileFormatOptions configurable via props or assetService -// Should support dynamic file formats based on available assets or server capabilities -const fileFormatOptions = [ - { name: '.ckpt', value: 'ckpt' }, - { name: '.safetensors', value: 'safetensors' }, - { name: '.pt', value: 'pt' } -] - -// TODO: Make baseModelOptions configurable via props or assetService -// Should support dynamic base models based on available assets or server detection -const baseModelOptions = [ - { name: 'SD 1.5', value: 'sd15' }, - { name: 'SD XL', value: 'sdxl' }, - { name: 'SD 3.5', value: 'sd35' } -] +const { availableFileFormats, availableBaseModels } = + useAssetFilterOptions(assets) // TODO: Make sortOptions configurable via props // Different asset types might need different sorting options diff --git a/src/platform/assets/composables/useAssetBrowser.ts b/src/platform/assets/composables/useAssetBrowser.ts index 97ed9746e..831b6c9ac 100644 --- a/src/platform/assets/composables/useAssetBrowser.ts +++ b/src/platform/assets/composables/useAssetBrowser.ts @@ -1,6 +1,7 @@ import { computed, ref } from 'vue' import { d, t } from '@/i18n' +import type { FilterState } from '@/platform/assets/components/AssetFilterBar.vue' import type { AssetItem } from '@/platform/assets/schemas/assetSchema' import { assetFilenameSchema } from '@/platform/assets/schemas/assetSchema' import { assetService } from '@/platform/assets/services/assetService' @@ -10,6 +11,43 @@ import { } from '@/platform/assets/utils/assetMetadataUtils' import { formatSize } from '@/utils/formatUtil' +function filterByCategory(category: string) { + return (asset: AssetItem) => { + return category === 'all' || asset.tags.includes(category) + } +} + +function filterByQuery(query: string) { + return (asset: AssetItem) => { + if (!query) return true + const lowerQuery = query.toLowerCase() + const description = getAssetDescription(asset) + return ( + asset.name.toLowerCase().includes(lowerQuery) || + (description && description.toLowerCase().includes(lowerQuery)) || + asset.tags.some((tag) => tag.toLowerCase().includes(lowerQuery)) + ) + } +} + +function filterByFileFormats(formats: string[]) { + return (asset: AssetItem) => { + if (formats.length === 0) return true + const formatSet = new Set(formats) + const extension = asset.name.split('.').pop()?.toLowerCase() + return extension ? formatSet.has(extension) : false + } +} + +function filterByBaseModels(models: string[]) { + return (asset: AssetItem) => { + if (models.length === 0) return true + const modelSet = new Set(models) + const baseModel = getAssetBaseModel(asset) + return baseModel ? modelSet.has(baseModel) : false + } +} + type AssetBadge = { label: string type: 'type' | 'base' | 'size' @@ -35,7 +73,11 @@ export function useAssetBrowser(assets: AssetItem[] = []) { // State const searchQuery = ref('') const selectedCategory = ref('all') - const sortBy = ref('name') + const filters = ref({ + sortBy: 'name-asc', + fileFormats: [], + baseModels: [] + }) // Transform API asset to display asset function transformAssetForDisplay(asset: AssetItem): AssetDisplayItem { @@ -84,16 +126,18 @@ export function useAssetBrowser(assets: AssetItem[] = []) { } } - // Extract available categories from assets const availableCategories = computed(() => { - const categorySet = new Set() + const categories = assets + .filter((asset) => asset.tags[0] === 'models' && asset.tags[1]) + .map((asset) => asset.tags[1]) - assets.forEach((asset) => { - // Second tag is the category (after 'models' root tag) - if (asset.tags.length > 1 && asset.tags[0] === 'models') { - categorySet.add(asset.tags[1]) - } - }) + const uniqueCategories = Array.from(new Set(categories)) + .sort() + .map((category) => ({ + id: category, + label: category.charAt(0).toUpperCase() + category.slice(1), + icon: 'icon-[lucide--package]' + })) return [ { @@ -101,13 +145,7 @@ export function useAssetBrowser(assets: AssetItem[] = []) { label: t('assetBrowser.allModels'), icon: 'icon-[lucide--folder]' }, - ...Array.from(categorySet) - .sort() - .map((category) => ({ - id: category, - label: category.charAt(0).toUpperCase() + category.slice(1), - icon: 'icon-[lucide--package]' - })) + ...uniqueCategories ] }) @@ -123,37 +161,25 @@ export function useAssetBrowser(assets: AssetItem[] = []) { return category?.label || t('assetBrowser.assets') }) - // Filter functions - const filterByCategory = (category: string) => (asset: AssetItem) => { - if (category === 'all') return true - return asset.tags.includes(category) - } - - const filterByQuery = (query: string) => (asset: AssetItem) => { - if (!query) return true - const lowerQuery = query.toLowerCase() - const description = getAssetDescription(asset) - return ( - asset.name.toLowerCase().includes(lowerQuery) || - (description && description.toLowerCase().includes(lowerQuery)) || - asset.tags.some((tag) => tag.toLowerCase().includes(lowerQuery)) - ) - } - - // Computed filtered and transformed assets const filteredAssets = computed(() => { const filtered = assets .filter(filterByCategory(selectedCategory.value)) .filter(filterByQuery(searchQuery.value)) + .filter(filterByFileFormats(filters.value.fileFormats)) + .filter(filterByBaseModels(filters.value.baseModels)) // Sort assets filtered.sort((a, b) => { - switch (sortBy.value) { - case 'date': + switch (filters.value.sortBy) { + case 'name-desc': + return b.name.localeCompare(a.name) + case 'recent': return ( new Date(b.created_at).getTime() - new Date(a.created_at).getTime() ) - case 'name': + case 'popular': + return a.name.localeCompare(b.name) + case 'name-asc': default: return a.name.localeCompare(b.name) } @@ -200,18 +226,17 @@ export function useAssetBrowser(assets: AssetItem[] = []) { } } + function updateFilters(newFilters: FilterState) { + filters.value = { ...newFilters } + } + return { - // State searchQuery, selectedCategory, - sortBy, - - // Computed availableCategories, contentTitle, filteredAssets, - - // Actions - selectAssetWithCallback + selectAssetWithCallback, + updateFilters } } diff --git a/src/platform/assets/composables/useAssetBrowserDialog.ts b/src/platform/assets/composables/useAssetBrowserDialog.ts index 31f75c353..ac42177b2 100644 --- a/src/platform/assets/composables/useAssetBrowserDialog.ts +++ b/src/platform/assets/composables/useAssetBrowserDialog.ts @@ -35,10 +35,10 @@ export const useAssetBrowserDialog = () => { class: 'rounded-2xl overflow-hidden asset-browser-dialog' }, header: { - class: 'p-0 hidden' + class: '!p-0 hidden' }, content: { - class: 'p-0 m-0 h-full w-full' + class: '!p-0 !m-0 h-full w-full' } } } diff --git a/src/platform/assets/fixtures/ui-mock-assets.ts b/src/platform/assets/fixtures/ui-mock-assets.ts index 6c7284386..482a48af9 100644 --- a/src/platform/assets/fixtures/ui-mock-assets.ts +++ b/src/platform/assets/fixtures/ui-mock-assets.ts @@ -126,3 +126,34 @@ export function createMockAssets(count: number = 20): AssetItem[] { } export const mockAssets = createMockAssets(20) + +// 🧪 Test helpers for edge cases - built on mock asset foundation +export function createAssetWithoutExtension() { + const asset = createMockAssets(1)[0] + asset.name = 'model_no_extension' + return asset +} + +export function createAssetWithoutBaseModel() { + const asset = createMockAssets(1)[0] + asset.user_metadata = { description: 'A test model' } + return asset +} + +export function createAssetWithoutUserMetadata() { + const asset = createMockAssets(1)[0] + asset.user_metadata = undefined + return asset +} + +export function createAssetWithSpecificExtension(extension: string) { + const asset = createMockAssets(1)[0] + asset.name = `test-model.${extension}` + return asset +} + +export function createAssetWithSpecificBaseModel(baseModel: string) { + const asset = createMockAssets(1)[0] + asset.user_metadata = { ...asset.user_metadata, base_model: baseModel } + return asset +} diff --git a/tests-ui/CLAUDE.md b/tests-ui/CLAUDE.md index 8685b6964..10e5d660b 100644 --- a/tests-ui/CLAUDE.md +++ b/tests-ui/CLAUDE.md @@ -1,5 +1,11 @@ # Unit Testing Guidelines +## Running Tests +- Single file: `pnpm test:unit -- ` +- All tests: `pnpm test:unit` +- Wrong Examples: + - Still runs all tests: `pnpm test:unit ` + ## Testing Approach - Write tests for new features @@ -11,3 +17,7 @@ - Check @tests-ui/README.md for guidelines - Use existing test utilities - Mock external dependencies + +## Mocking +- Read: https://vitest.dev/api/mock.html +- Critical: Always prefer vitest mock functions over writing verbose manual mocks diff --git a/tests-ui/platform/assets/components/AssetBrowserModal.test.ts b/tests-ui/platform/assets/components/AssetBrowserModal.test.ts index 4e96dd902..003f6e4a2 100644 --- a/tests-ui/platform/assets/components/AssetBrowserModal.test.ts +++ b/tests-ui/platform/assets/components/AssetBrowserModal.test.ts @@ -7,6 +7,27 @@ import AssetBrowserModal from '@/platform/assets/components/AssetBrowserModal.vu import type { AssetDisplayItem } from '@/platform/assets/composables/useAssetBrowser' import type { AssetItem } from '@/platform/assets/schemas/assetSchema' +// Mock @/i18n for useAssetBrowser and AssetFilterBar +vi.mock('@/i18n', () => ({ + t: (key: string) => key, + d: (date: Date) => date.toLocaleDateString() +})) + +// Mock assetService for useAssetBrowser +vi.mock('@/platform/assets/services/assetService', () => ({ + assetService: { + getAssetDetails: vi.fn((id: string) => + Promise.resolve({ + id, + name: 'Test Model', + user_metadata: { + filename: 'Test Model' + } + }) + ) + } +})) + // Mock external dependencies with minimal functionality needed for business logic tests vi.mock('@/components/input/SearchBox.vue', () => ({ default: { @@ -92,6 +113,11 @@ vi.mock('@/platform/assets/components/AssetGrid.vue', () => ({ vi.mock('vue-i18n', () => ({ useI18n: () => ({ t: (key: string) => key + }), + createI18n: () => ({ + global: { + t: (key: string) => key + } }) })) diff --git a/tests-ui/platform/assets/components/AssetFilterBar.test.ts b/tests-ui/platform/assets/components/AssetFilterBar.test.ts index db24070e1..8e659d24f 100644 --- a/tests-ui/platform/assets/components/AssetFilterBar.test.ts +++ b/tests-ui/platform/assets/components/AssetFilterBar.test.ts @@ -4,6 +4,17 @@ import { nextTick } from 'vue' import AssetFilterBar from '@/platform/assets/components/AssetFilterBar.vue' import type { FilterState } from '@/platform/assets/components/AssetFilterBar.vue' +import { + createAssetWithSpecificBaseModel, + createAssetWithSpecificExtension, + createAssetWithoutBaseModel +} from '@/platform/assets/fixtures/ui-mock-assets' +import type { AssetItem } from '@/platform/assets/schemas/assetSchema' + +// Mock @/i18n directly since component imports { t } from '@/i18n' +vi.mock('@/i18n', () => ({ + t: (key: string) => key +})) // Mock components with minimal functionality for business logic testing vi.mock('@/components/input/MultiSelect.vue', () => ({ @@ -51,11 +62,26 @@ vi.mock('@/components/input/SingleSelect.vue', () => ({ })) // Test factory functions +function mountAssetFilterBar(props = {}) { + return mount(AssetFilterBar, { + props, + global: { + mocks: { + $t: (key: string) => key + } + } + }) +} describe('AssetFilterBar', () => { describe('Filter State Management', () => { it('maintains correct initial state', () => { - const wrapper = mount(AssetFilterBar) + // Provide assets with options so filters are visible + const assets = [ + createAssetWithSpecificExtension('safetensors'), + createAssetWithSpecificBaseModel('sd15') + ] + const wrapper = mountAssetFilterBar({ assets }) // Test initial state through component props const multiSelects = wrapper.findAllComponents({ name: 'MultiSelect' }) @@ -67,7 +93,12 @@ describe('AssetFilterBar', () => { }) it('handles multiple simultaneous filter changes correctly', async () => { - const wrapper = mount(AssetFilterBar) + // Provide assets with options so filters are visible + const assets = [ + createAssetWithSpecificExtension('safetensors'), + createAssetWithSpecificBaseModel('sd15') + ] + const wrapper = mountAssetFilterBar({ assets }) // Update file formats const fileFormatSelect = wrapper.findAllComponents({ @@ -107,7 +138,12 @@ describe('AssetFilterBar', () => { }) it('ensures FilterState interface compliance', async () => { - const wrapper = mount(AssetFilterBar) + // Provide assets with options so filters are visible + const assets = [ + createAssetWithSpecificExtension('safetensors'), + createAssetWithSpecificBaseModel('sd15') + ] + const wrapper = mountAssetFilterBar({ assets }) const fileFormatSelect = wrapper.findAllComponents({ name: 'MultiSelect' @@ -135,4 +171,98 @@ describe('AssetFilterBar', () => { ) }) }) + + describe('Dynamic Filter Options', () => { + it('should use dynamic file format options based on actual assets', () => { + const assets = [ + createAssetWithSpecificExtension('safetensors'), + createAssetWithSpecificExtension('ckpt'), + createAssetWithSpecificExtension('pt') + ] + + const wrapper = mountAssetFilterBar({ assets }) + + const fileFormatSelect = wrapper.findAllComponents({ + name: 'MultiSelect' + })[0] + expect(fileFormatSelect.props('options')).toEqual([ + { name: '.ckpt', value: 'ckpt' }, + { name: '.pt', value: 'pt' }, + { name: '.safetensors', value: 'safetensors' } + ]) + }) + + it('should use dynamic base model options based on actual assets', () => { + const assets = [ + createAssetWithSpecificBaseModel('sd15'), + createAssetWithSpecificBaseModel('sdxl'), + createAssetWithSpecificBaseModel('sd35') + ] + + const wrapper = mountAssetFilterBar({ assets }) + + const baseModelSelect = wrapper.findAllComponents({ + name: 'MultiSelect' + })[1] + expect(baseModelSelect.props('options')).toEqual([ + { name: 'sd15', value: 'sd15' }, + { name: 'sd35', value: 'sd35' }, + { name: 'sdxl', value: 'sdxl' } + ]) + }) + }) + + describe('Conditional Filter Visibility', () => { + it('hides file format filter when no options available', () => { + const assets: AssetItem[] = [] // No assets = no file format options + const wrapper = mountAssetFilterBar({ assets }) + + const fileFormatSelects = wrapper + .findAllComponents({ name: 'MultiSelect' }) + .filter( + (component) => component.props('label') === 'assetBrowser.fileFormats' + ) + + expect(fileFormatSelects).toHaveLength(0) + }) + + it('hides base model filter when no options available', () => { + const assets = [createAssetWithoutBaseModel()] // Asset without base model = no base model options + const wrapper = mountAssetFilterBar({ assets }) + + const baseModelSelects = wrapper + .findAllComponents({ name: 'MultiSelect' }) + .filter( + (component) => component.props('label') === 'assetBrowser.baseModels' + ) + + expect(baseModelSelects).toHaveLength(0) + }) + + it('shows both filters when options are available', () => { + const assets = [ + createAssetWithSpecificExtension('safetensors'), + createAssetWithSpecificBaseModel('sd15') + ] + const wrapper = mountAssetFilterBar({ assets }) + + const multiSelects = wrapper.findAllComponents({ name: 'MultiSelect' }) + const fileFormatSelect = multiSelects.find( + (component) => component.props('label') === 'assetBrowser.fileFormats' + ) + const baseModelSelect = multiSelects.find( + (component) => component.props('label') === 'assetBrowser.baseModels' + ) + + expect(fileFormatSelect).toBeDefined() + expect(baseModelSelect).toBeDefined() + }) + + it('hides both filters when no assets provided', () => { + const wrapper = mountAssetFilterBar() + + const multiSelects = wrapper.findAllComponents({ name: 'MultiSelect' }) + expect(multiSelects).toHaveLength(0) + }) + }) }) diff --git a/tests-ui/platform/assets/composables/useAssetBrowser.test.ts b/tests-ui/platform/assets/composables/useAssetBrowser.test.ts index bef33733b..ab219d833 100644 --- a/tests-ui/platform/assets/composables/useAssetBrowser.test.ts +++ b/tests-ui/platform/assets/composables/useAssetBrowser.test.ts @@ -224,9 +224,9 @@ describe('useAssetBrowser', () => { createApiAsset({ name: 'beta.safetensors' }) ] - const { sortBy, filteredAssets } = useAssetBrowser(assets) + const { updateFilters, filteredAssets } = useAssetBrowser(assets) - sortBy.value = 'name' + updateFilters({ sortBy: 'name', fileFormats: [], baseModels: [] }) await nextTick() const names = filteredAssets.value.map((asset) => asset.name) @@ -244,9 +244,9 @@ describe('useAssetBrowser', () => { createApiAsset({ created_at: '2024-02-01T00:00:00Z' }) ] - const { sortBy, filteredAssets } = useAssetBrowser(assets) + const { updateFilters, filteredAssets } = useAssetBrowser(assets) - sortBy.value = 'date' + updateFilters({ sortBy: 'recent', fileFormats: [], baseModels: [] }) await nextTick() const dates = filteredAssets.value.map((asset) => asset.created_at) diff --git a/tests-ui/platform/assets/composables/useAssetFilterOptions.test.ts b/tests-ui/platform/assets/composables/useAssetFilterOptions.test.ts index 8cec2ab12..af1d1c9fb 100644 --- a/tests-ui/platform/assets/composables/useAssetFilterOptions.test.ts +++ b/tests-ui/platform/assets/composables/useAssetFilterOptions.test.ts @@ -1,34 +1,21 @@ import { describe, expect, it } from 'vitest' import { useAssetFilterOptions } from '@/platform/assets/composables/useAssetFilterOptions' -import type { AssetItem } from '@/platform/assets/schemas/assetSchema' - -// Test factory functions -function createTestAsset(overrides: Partial = {}): AssetItem { - return { - id: 'test-uuid', - name: 'test-model.safetensors', - asset_hash: 'blake3:test123', - size: 123456, - mime_type: 'application/octet-stream', - tags: ['models', 'checkpoints'], - created_at: '2024-01-01T00:00:00Z', - updated_at: '2024-01-01T00:00:00Z', - last_access_time: '2024-01-01T00:00:00Z', - user_metadata: { - base_model: 'sd15' - }, - ...overrides - } -} +import { + createAssetWithSpecificBaseModel, + createAssetWithSpecificExtension, + createAssetWithoutBaseModel, + createAssetWithoutExtension, + createAssetWithoutUserMetadata +} from '@/platform/assets/fixtures/ui-mock-assets' describe('useAssetFilterOptions', () => { describe('File Format Extraction', () => { it('extracts file formats from asset names', () => { const assets = [ - createTestAsset({ name: 'model1.safetensors' }), - createTestAsset({ name: 'model2.ckpt' }), - createTestAsset({ name: 'model3.pt' }) + createAssetWithSpecificExtension('safetensors'), + createAssetWithSpecificExtension('ckpt'), + createAssetWithSpecificExtension('pt') ] const { availableFileFormats } = useAssetFilterOptions(assets) @@ -42,9 +29,9 @@ describe('useAssetFilterOptions', () => { it('handles duplicate file formats', () => { const assets = [ - createTestAsset({ name: 'model1.safetensors' }), - createTestAsset({ name: 'model2.safetensors' }), - createTestAsset({ name: 'model3.ckpt' }) + createAssetWithSpecificExtension('safetensors'), + createAssetWithSpecificExtension('safetensors'), + createAssetWithSpecificExtension('ckpt') ] const { availableFileFormats } = useAssetFilterOptions(assets) @@ -57,8 +44,8 @@ describe('useAssetFilterOptions', () => { it('handles assets with no file extension', () => { const assets = [ - createTestAsset({ name: 'model_no_extension' }), - createTestAsset({ name: 'model.safetensors' }) + createAssetWithoutExtension(), + createAssetWithSpecificExtension('safetensors') ] const { availableFileFormats } = useAssetFilterOptions(assets) @@ -78,9 +65,9 @@ describe('useAssetFilterOptions', () => { describe('Base Model Extraction', () => { it('extracts base models from user metadata', () => { const assets = [ - createTestAsset({ user_metadata: { base_model: 'sd15' } }), - createTestAsset({ user_metadata: { base_model: 'sdxl' } }), - createTestAsset({ user_metadata: { base_model: 'sd35' } }) + createAssetWithSpecificBaseModel('sd15'), + createAssetWithSpecificBaseModel('sdxl'), + createAssetWithSpecificBaseModel('sd35') ] const { availableBaseModels } = useAssetFilterOptions(assets) @@ -94,9 +81,9 @@ describe('useAssetFilterOptions', () => { it('handles duplicate base models', () => { const assets = [ - createTestAsset({ user_metadata: { base_model: 'sd15' } }), - createTestAsset({ user_metadata: { base_model: 'sd15' } }), - createTestAsset({ user_metadata: { base_model: 'sdxl' } }) + createAssetWithSpecificBaseModel('sd15'), + createAssetWithSpecificBaseModel('sd15'), + createAssetWithSpecificBaseModel('sdxl') ] const { availableBaseModels } = useAssetFilterOptions(assets) @@ -109,8 +96,8 @@ describe('useAssetFilterOptions', () => { it('handles assets with missing user_metadata', () => { const assets = [ - createTestAsset({ user_metadata: undefined }), - createTestAsset({ user_metadata: { base_model: 'sd15' } }) + createAssetWithoutUserMetadata(), + createAssetWithSpecificBaseModel('sd15') ] const { availableBaseModels } = useAssetFilterOptions(assets) @@ -122,8 +109,8 @@ describe('useAssetFilterOptions', () => { it('handles assets with missing base_model field', () => { const assets = [ - createTestAsset({ user_metadata: { description: 'A test model' } }), - createTestAsset({ user_metadata: { base_model: 'sdxl' } }) + createAssetWithoutBaseModel(), + createAssetWithSpecificBaseModel('sdxl') ] const { availableBaseModels } = useAssetFilterOptions(assets) @@ -142,7 +129,7 @@ describe('useAssetFilterOptions', () => { describe('Reactivity', () => { it('returns computed properties that can be reactive', () => { - const assets = [createTestAsset({ name: 'model.safetensors' })] + const assets = [createAssetWithSpecificExtension('safetensors')] const { availableFileFormats, availableBaseModels } = useAssetFilterOptions(assets)
Filter bar with proper chrome styling showing contextual background and borders.