mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-23 00:04:06 +00:00
refactor: unify asset sorting between AssetBrowser and FormDropdown
- Create shared sortAssets() utility in src/platform/assets/utils/assetSortUtils.ts - Add 'default' option to AssetSortOption type for preserve-order sorting - Extract shared filter types (OptionId, FilterOption, OwnershipOption, AssetSortOption) to src/platform/assets/types/filterTypes.ts - Update useAssetBrowser.ts to use sortAssets() (removes inline switch statement) - Update FormDropdown shared.ts to delegate to sortAssets() - Make SortOption interface generic for type-safe sort IDs - Add comprehensive tests for assetSortUtils Amp-Thread-ID: https://ampcode.com/threads/T-019c10a1-1209-75bd-9c79-f312cee89f50 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -3,7 +3,7 @@ import { describe, expect, it, vi } from 'vitest'
|
||||
import { nextTick } from 'vue'
|
||||
|
||||
import AssetFilterBar from '@/platform/assets/components/AssetFilterBar.vue'
|
||||
import type { FilterState } from '@/platform/assets/components/AssetFilterBar.vue'
|
||||
import type { AssetFilterState } from '@/platform/assets/types/filterTypes'
|
||||
import {
|
||||
createAssetWithSpecificBaseModel,
|
||||
createAssetWithSpecificExtension,
|
||||
@@ -142,15 +142,15 @@ describe('AssetFilterBar', () => {
|
||||
expect(emitted!.length).toBeGreaterThanOrEqual(3)
|
||||
|
||||
// Check final state
|
||||
const finalState: FilterState = emitted![
|
||||
const finalState: AssetFilterState = emitted![
|
||||
emitted!.length - 1
|
||||
][0] as FilterState
|
||||
][0] as AssetFilterState
|
||||
expect(finalState.fileFormats).toEqual(['ckpt', 'safetensors'])
|
||||
expect(finalState.baseModels).toEqual(['sdxl'])
|
||||
expect(finalState.sortBy).toBe('name-desc')
|
||||
})
|
||||
|
||||
it('ensures FilterState interface compliance', async () => {
|
||||
it('ensures AssetFilterState interface compliance', async () => {
|
||||
// Provide assets with options so filters are visible
|
||||
const assets = [
|
||||
createAssetWithSpecificExtension('safetensors'),
|
||||
@@ -167,7 +167,7 @@ describe('AssetFilterBar', () => {
|
||||
await nextTick()
|
||||
|
||||
const emitted = wrapper.emitted('filterChange')
|
||||
const filterState = emitted![0][0] as FilterState
|
||||
const filterState = emitted![0][0] as AssetFilterState
|
||||
|
||||
// Type and structure assertions
|
||||
expect(Array.isArray(filterState.fileFormats)).toBe(true)
|
||||
|
||||
@@ -64,12 +64,14 @@ import SingleSelect from '@/components/input/SingleSelect.vue'
|
||||
import type { SelectOption } from '@/components/input/types'
|
||||
import { useAssetFilterOptions } from '@/platform/assets/composables/useAssetFilterOptions'
|
||||
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
|
||||
import type {
|
||||
AssetFilterState,
|
||||
AssetSortOption,
|
||||
OwnershipOption
|
||||
} from '@/platform/assets/types/filterTypes'
|
||||
|
||||
const { t } = useI18n()
|
||||
|
||||
type SortOption = 'recent' | 'name-asc' | 'name-desc'
|
||||
export type OwnershipOption = 'all' | 'my-models' | 'public-models'
|
||||
|
||||
const sortOptions = computed(() => [
|
||||
{ name: t('assetBrowser.sortRecent'), value: 'recent' as const },
|
||||
{ name: t('assetBrowser.sortAZ'), value: 'name-asc' as const },
|
||||
@@ -85,13 +87,6 @@ const ownershipOptions = computed(() => [
|
||||
}
|
||||
])
|
||||
|
||||
export interface FilterState {
|
||||
fileFormats: string[]
|
||||
baseModels: string[]
|
||||
sortBy: SortOption
|
||||
ownership: OwnershipOption
|
||||
}
|
||||
|
||||
const { assets = [], showOwnershipFilter = false } = defineProps<{
|
||||
assets?: AssetItem[]
|
||||
showOwnershipFilter?: boolean
|
||||
@@ -99,7 +94,7 @@ const { assets = [], showOwnershipFilter = false } = defineProps<{
|
||||
|
||||
const fileFormats = ref<SelectOption[]>([])
|
||||
const baseModels = ref<SelectOption[]>([])
|
||||
const sortBy = ref<SortOption>('recent')
|
||||
const sortBy = ref<AssetSortOption>('recent')
|
||||
const ownership = ref<OwnershipOption>('all')
|
||||
|
||||
const { availableFileFormats, availableBaseModels } = useAssetFilterOptions(
|
||||
@@ -107,7 +102,7 @@ const { availableFileFormats, availableBaseModels } = useAssetFilterOptions(
|
||||
)
|
||||
|
||||
const emit = defineEmits<{
|
||||
filterChange: [filters: FilterState]
|
||||
filterChange: [filters: AssetFilterState]
|
||||
}>()
|
||||
|
||||
function handleFilterChange() {
|
||||
|
||||
@@ -6,15 +6,15 @@ import { storeToRefs } from 'pinia'
|
||||
|
||||
import { d, t } from '@/i18n'
|
||||
import type {
|
||||
FilterState,
|
||||
AssetFilterState,
|
||||
OwnershipOption
|
||||
} from '@/platform/assets/components/AssetFilterBar.vue'
|
||||
} from '@/platform/assets/types/filterTypes'
|
||||
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
|
||||
import {
|
||||
getAssetBaseModels,
|
||||
getAssetDisplayName,
|
||||
getAssetFilename
|
||||
} from '@/platform/assets/utils/assetMetadataUtils'
|
||||
import { sortAssets } from '@/platform/assets/utils/assetSortUtils'
|
||||
import { useAssetDownloadStore } from '@/stores/assetDownloadStore'
|
||||
import type { NavGroupData, NavItemData } from '@/types/navTypes'
|
||||
|
||||
@@ -94,7 +94,7 @@ export function useAssetBrowser(
|
||||
// State
|
||||
const searchQuery = ref('')
|
||||
const selectedNavItem = ref<NavId>('all')
|
||||
const filters = ref<FilterState>({
|
||||
const filters = ref<AssetFilterState>({
|
||||
sortBy: 'recent',
|
||||
fileFormats: [],
|
||||
baseModels: [],
|
||||
@@ -263,27 +263,13 @@ export function useAssetBrowser(
|
||||
.filter(filterByBaseModels(filters.value.baseModels))
|
||||
.filter(filterByOwnership(selectedOwnership.value))
|
||||
|
||||
const sortedAssets = [...filtered]
|
||||
sortedAssets.sort((a, b) => {
|
||||
switch (filters.value.sortBy) {
|
||||
case 'name-desc':
|
||||
return getAssetDisplayName(b).localeCompare(getAssetDisplayName(a))
|
||||
case 'recent':
|
||||
return (
|
||||
new Date(b.created_at ?? 0).getTime() -
|
||||
new Date(a.created_at ?? 0).getTime()
|
||||
)
|
||||
case 'name-asc':
|
||||
default:
|
||||
return getAssetDisplayName(a).localeCompare(getAssetDisplayName(b))
|
||||
}
|
||||
})
|
||||
const sortedAssets = sortAssets(filtered, filters.value.sortBy)
|
||||
|
||||
// Transform to display format
|
||||
return sortedAssets.map(transformAssetForDisplay)
|
||||
})
|
||||
|
||||
function updateFilters(newFilters: FilterState) {
|
||||
function updateFilters(newFilters: AssetFilterState) {
|
||||
filters.value = { ...newFilters }
|
||||
}
|
||||
|
||||
|
||||
45
src/platform/assets/types/filterTypes.ts
Normal file
45
src/platform/assets/types/filterTypes.ts
Normal file
@@ -0,0 +1,45 @@
|
||||
/**
|
||||
* Asset filtering and sorting types
|
||||
* Shared across AssetBrowser, AssetFilterBar, and widget dropdowns
|
||||
*/
|
||||
|
||||
/**
|
||||
* Generic option identifier type
|
||||
*/
|
||||
export type OptionId = string
|
||||
|
||||
/**
|
||||
* Generic filter/select option used across components
|
||||
* Compatible with both SelectOption (name/value) and FilterOption (id/name) patterns
|
||||
*/
|
||||
export interface FilterOption {
|
||||
id: OptionId
|
||||
name: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Ownership filter options for assets
|
||||
* - 'all': Show all assets
|
||||
* - 'my-models': Show only user-owned assets (is_immutable === false)
|
||||
* - 'public-models': Show only public assets (is_immutable === true)
|
||||
*/
|
||||
export type OwnershipOption = 'all' | 'my-models' | 'public-models'
|
||||
|
||||
/**
|
||||
* Sort options for asset lists
|
||||
* - 'default': Preserve original order (no sorting)
|
||||
* - 'recent': Sort by created_at descending
|
||||
* - 'name-asc': Sort by display name A-Z
|
||||
* - 'name-desc': Sort by display name Z-A
|
||||
*/
|
||||
export type AssetSortOption = 'default' | 'recent' | 'name-asc' | 'name-desc'
|
||||
|
||||
/**
|
||||
* Filter state for asset browser and filter bar
|
||||
*/
|
||||
export interface AssetFilterState {
|
||||
fileFormats: string[]
|
||||
baseModels: string[]
|
||||
sortBy: AssetSortOption
|
||||
ownership: OwnershipOption
|
||||
}
|
||||
122
src/platform/assets/utils/assetSortUtils.test.ts
Normal file
122
src/platform/assets/utils/assetSortUtils.test.ts
Normal file
@@ -0,0 +1,122 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type { SortableItem } from './assetSortUtils'
|
||||
import { sortAssets } from './assetSortUtils'
|
||||
|
||||
function createItem(
|
||||
name: string,
|
||||
options: { label?: string; created_at?: string } = {}
|
||||
): SortableItem {
|
||||
return { name, ...options }
|
||||
}
|
||||
|
||||
describe('sortAssets', () => {
|
||||
describe('default sort', () => {
|
||||
it('preserves original order', () => {
|
||||
const items = [createItem('z'), createItem('a'), createItem('m')]
|
||||
const result = sortAssets(items, 'default')
|
||||
expect(result.map((i) => i.name)).toEqual(['z', 'a', 'm'])
|
||||
})
|
||||
|
||||
it('returns a new array (does not mutate)', () => {
|
||||
const items = [createItem('z'), createItem('a')]
|
||||
const result = sortAssets(items, 'default')
|
||||
expect(result).not.toBe(items)
|
||||
})
|
||||
})
|
||||
|
||||
describe('name-asc sort', () => {
|
||||
it('sorts alphabetically A-Z by name', () => {
|
||||
const items = [
|
||||
createItem('cherry'),
|
||||
createItem('apple'),
|
||||
createItem('banana')
|
||||
]
|
||||
const result = sortAssets(items, 'name-asc')
|
||||
expect(result.map((i) => i.name)).toEqual(['apple', 'banana', 'cherry'])
|
||||
})
|
||||
|
||||
it('prefers label over name when label exists', () => {
|
||||
const items = [
|
||||
createItem('file_c.png', { label: 'Cherry' }),
|
||||
createItem('file_a.png', { label: 'Apple' }),
|
||||
createItem('file_b.png', { label: 'Banana' })
|
||||
]
|
||||
const result = sortAssets(items, 'name-asc')
|
||||
expect(result.map((i) => i.name)).toEqual([
|
||||
'file_a.png',
|
||||
'file_b.png',
|
||||
'file_c.png'
|
||||
])
|
||||
})
|
||||
|
||||
it('uses natural sort for numeric values', () => {
|
||||
const items = [
|
||||
createItem('img_10.png'),
|
||||
createItem('img_2.png'),
|
||||
createItem('img_1.png'),
|
||||
createItem('img_20.png')
|
||||
]
|
||||
const result = sortAssets(items, 'name-asc')
|
||||
expect(result.map((i) => i.name)).toEqual([
|
||||
'img_1.png',
|
||||
'img_2.png',
|
||||
'img_10.png',
|
||||
'img_20.png'
|
||||
])
|
||||
})
|
||||
|
||||
it('is case-insensitive', () => {
|
||||
const items = [
|
||||
createItem('Banana'),
|
||||
createItem('apple'),
|
||||
createItem('CHERRY')
|
||||
]
|
||||
const result = sortAssets(items, 'name-asc')
|
||||
expect(result.map((i) => i.name)).toEqual(['apple', 'Banana', 'CHERRY'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('name-desc sort', () => {
|
||||
it('sorts alphabetically Z-A by name', () => {
|
||||
const items = [
|
||||
createItem('apple'),
|
||||
createItem('cherry'),
|
||||
createItem('banana')
|
||||
]
|
||||
const result = sortAssets(items, 'name-desc')
|
||||
expect(result.map((i) => i.name)).toEqual(['cherry', 'banana', 'apple'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('recent sort', () => {
|
||||
it('sorts by created_at descending (newest first)', () => {
|
||||
const items = [
|
||||
createItem('old', { created_at: '2024-01-01T00:00:00Z' }),
|
||||
createItem('newest', { created_at: '2024-03-01T00:00:00Z' }),
|
||||
createItem('middle', { created_at: '2024-02-01T00:00:00Z' })
|
||||
]
|
||||
const result = sortAssets(items, 'recent')
|
||||
expect(result.map((i) => i.name)).toEqual(['newest', 'middle', 'old'])
|
||||
})
|
||||
|
||||
it('handles null/undefined created_at (sorts to end)', () => {
|
||||
const items = [
|
||||
createItem('no-date'),
|
||||
createItem('has-date', { created_at: '2024-01-01T00:00:00Z' }),
|
||||
createItem('null-date', { created_at: null as unknown as string })
|
||||
]
|
||||
const result = sortAssets(items, 'recent')
|
||||
expect(result[0].name).toBe('has-date')
|
||||
})
|
||||
})
|
||||
|
||||
describe('immutability', () => {
|
||||
it('does not mutate the original array', () => {
|
||||
const items = [createItem('z'), createItem('a')]
|
||||
const original = [...items]
|
||||
sortAssets(items, 'name-asc')
|
||||
expect(items).toEqual(original)
|
||||
})
|
||||
})
|
||||
})
|
||||
61
src/platform/assets/utils/assetSortUtils.ts
Normal file
61
src/platform/assets/utils/assetSortUtils.ts
Normal file
@@ -0,0 +1,61 @@
|
||||
/**
|
||||
* Shared asset sorting utilities
|
||||
* Used by both AssetBrowser and FormDropdown
|
||||
*/
|
||||
|
||||
import type { AssetSortOption } from '../types/filterTypes'
|
||||
|
||||
/**
|
||||
* Minimal interface for sortable items
|
||||
* Works with both AssetItem and DropdownItem
|
||||
*/
|
||||
export interface SortableItem {
|
||||
name: string
|
||||
label?: string
|
||||
created_at?: string | null
|
||||
}
|
||||
|
||||
function getDisplayName(item: SortableItem): string {
|
||||
return item.label ?? item.name
|
||||
}
|
||||
|
||||
/**
|
||||
* Sort items by the specified sort option
|
||||
* @param items - Array of sortable items
|
||||
* @param sortBy - Sort option from AssetSortOption
|
||||
* @returns New sorted array (does not mutate input)
|
||||
*/
|
||||
export function sortAssets<T extends SortableItem>(
|
||||
items: readonly T[],
|
||||
sortBy: AssetSortOption
|
||||
): T[] {
|
||||
if (sortBy === 'default') {
|
||||
return items.slice()
|
||||
}
|
||||
|
||||
const sorted = items.slice()
|
||||
|
||||
switch (sortBy) {
|
||||
case 'name-desc':
|
||||
return sorted.sort((a, b) =>
|
||||
getDisplayName(b).localeCompare(getDisplayName(a), undefined, {
|
||||
numeric: true,
|
||||
sensitivity: 'base'
|
||||
})
|
||||
)
|
||||
case 'recent':
|
||||
return sorted.sort(
|
||||
(a, b) =>
|
||||
new Date(b.created_at ?? 0).getTime() -
|
||||
new Date(a.created_at ?? 0).getTime()
|
||||
)
|
||||
case 'name-asc':
|
||||
default:
|
||||
return sorted.sort((a, b) =>
|
||||
getDisplayName(a).localeCompare(getDisplayName(b), undefined, {
|
||||
numeric: true,
|
||||
sensitivity: 'base'
|
||||
})
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -74,7 +74,7 @@ describe('getDefaultSortOptions', () => {
|
||||
})
|
||||
|
||||
describe('A-Z sorter', () => {
|
||||
const azSorter = sortOptions.find((o) => o.id === 'a-z')!.sorter
|
||||
const azSorter = sortOptions.find((o) => o.id === 'name-asc')!.sorter
|
||||
|
||||
it('sorts items alphabetically by name', () => {
|
||||
const items = [
|
||||
|
||||
@@ -1,3 +1,6 @@
|
||||
import type { AssetSortOption } from '@/platform/assets/types/filterTypes'
|
||||
import { sortAssets } from '@/platform/assets/utils/assetSortUtils'
|
||||
|
||||
import type { DropdownItem, SortOption } from './types'
|
||||
|
||||
export async function defaultSearcher(query: string, items: DropdownItem[]) {
|
||||
@@ -9,25 +12,23 @@ export async function defaultSearcher(query: string, items: DropdownItem[]) {
|
||||
})
|
||||
}
|
||||
|
||||
export function getDefaultSortOptions(): SortOption[] {
|
||||
/**
|
||||
* Create a SortOption that delegates to the shared sortAssets utility
|
||||
*/
|
||||
function createSortOption(
|
||||
id: AssetSortOption,
|
||||
name: string
|
||||
): SortOption<AssetSortOption> {
|
||||
return {
|
||||
id,
|
||||
name,
|
||||
sorter: ({ items }) => sortAssets(items, id)
|
||||
}
|
||||
}
|
||||
|
||||
export function getDefaultSortOptions(): SortOption<AssetSortOption>[] {
|
||||
return [
|
||||
{
|
||||
name: 'Default',
|
||||
id: 'default',
|
||||
sorter: ({ items }) => items.slice()
|
||||
},
|
||||
{
|
||||
name: 'A-Z',
|
||||
id: 'a-z',
|
||||
sorter: ({ items }) =>
|
||||
items.slice().sort((a, b) => {
|
||||
const aLabel = a.label ?? a.name
|
||||
const bLabel = b.label ?? b.name
|
||||
return aLabel.localeCompare(bLabel, undefined, {
|
||||
numeric: true,
|
||||
sensitivity: 'base'
|
||||
})
|
||||
})
|
||||
}
|
||||
createSortOption('default', 'Default'),
|
||||
createSortOption('name-asc', 'A-Z')
|
||||
]
|
||||
}
|
||||
|
||||
@@ -1,8 +1,13 @@
|
||||
import type { ComputedRef, InjectionKey } from 'vue'
|
||||
|
||||
import type {
|
||||
FilterOption,
|
||||
OptionId
|
||||
} from '@/platform/assets/types/filterTypes'
|
||||
import type { AssetKind } from '@/types/widgetTypes'
|
||||
|
||||
export type OptionId = string | number | symbol
|
||||
export type { FilterOption, OptionId }
|
||||
|
||||
export type SelectedKey = OptionId
|
||||
|
||||
export interface DropdownItem {
|
||||
@@ -12,17 +17,13 @@ export interface DropdownItem {
|
||||
label?: string
|
||||
metadata: string
|
||||
}
|
||||
export interface SortOption {
|
||||
id: OptionId
|
||||
|
||||
export interface SortOption<TId extends OptionId = OptionId> {
|
||||
id: TId
|
||||
name: string
|
||||
sorter: (ctx: { items: readonly DropdownItem[] }) => DropdownItem[]
|
||||
}
|
||||
|
||||
export interface FilterOption {
|
||||
id: OptionId
|
||||
name: string
|
||||
}
|
||||
|
||||
export type LayoutMode = 'list' | 'grid' | 'list-small'
|
||||
|
||||
export const AssetKindKey: InjectionKey<ComputedRef<AssetKind | undefined>> =
|
||||
|
||||
Reference in New Issue
Block a user