mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-22 07:44:11 +00:00
Fix: Implement scope-aware filtering for template modal (#8335)
## Summary - Fixes template filtering to be scope-aware, preventing empty results when switching categories - Filters now only show as selected when they exist in the current category scope - User selections are still persisted in localStorage for convenience ## Problem When users selected filters (e.g., Image-specific filters) and switched to a different category (e.g., Video), the persisted filters would cause empty results because those filter values didn't exist in the new scope. ## Solution - Modified `useTemplateFiltering` composable to track which filters are "active" (applicable to current scope) - Only active filters are shown as selected in the UI - Only active filters are applied to the filtering logic - All user selections are still persisted in localStorage - When returning to a category, previously selected filters automatically reactivate if applicable ## Test Plan Tested manually in browser: 1. Navigate to Image category 2. Select filters like "Image" and "Image Edit" from Tasks dropdown 3. Switch to Video category - filters show as "0 selected" (Image/Image Edit don't exist in Video) 4. Return to Image category - filters are automatically reselected (2 selected) 5. No empty results when switching between categories ## Changes - `useTemplateFiltering.ts`: Added scope-aware filtering logic with active/inactive filter distinction - `WorkflowTemplateSelectorDialog.vue`: Pass navigation context and use active filters for UI - `useTemplateFiltering.test.ts`: Added comprehensive tests for scope-aware behavior https://github.com/user-attachments/assets/706ddabf-abad-4ff9-a378-6603d275d15a
This commit is contained in:
committed by
GitHub
parent
aa5125cef6
commit
3b5d285fe2
@@ -547,13 +547,15 @@ const navigationFilteredTemplates = computed(() => {
|
||||
return workflowTemplatesStore.filterTemplatesByCategory(selectedNavItem.value)
|
||||
})
|
||||
|
||||
// Template filtering
|
||||
// Template filtering with scope awareness
|
||||
const {
|
||||
searchQuery,
|
||||
selectedModels,
|
||||
selectedUseCases,
|
||||
selectedRunsOn,
|
||||
sortBy,
|
||||
activeModels,
|
||||
activeUseCases,
|
||||
filteredTemplates,
|
||||
availableModels,
|
||||
availableUseCases,
|
||||
@@ -562,7 +564,7 @@ const {
|
||||
totalCount,
|
||||
resetFilters,
|
||||
loadFuseOptions
|
||||
} = useTemplateFiltering(navigationFilteredTemplates)
|
||||
} = useTemplateFiltering(navigationFilteredTemplates, selectedNavItem)
|
||||
|
||||
/**
|
||||
* Coordinates state between the selected navigation item and the sort order to
|
||||
@@ -595,9 +597,11 @@ watch(selectedNavItem, () => coordinateNavAndSort('nav'))
|
||||
watch(sortBy, () => coordinateNavAndSort('sort'))
|
||||
|
||||
// Convert between string array and object array for MultiSelect component
|
||||
// Only show selected items that exist in the current scope
|
||||
const selectedModelObjects = computed({
|
||||
get() {
|
||||
return selectedModels.value.map((model) => ({ name: model, value: model }))
|
||||
// Only include selected models that exist in availableModels
|
||||
return activeModels.value.map((model) => ({ name: model, value: model }))
|
||||
},
|
||||
set(value: { name: string; value: string }[]) {
|
||||
selectedModels.value = value.map((item) => item.value)
|
||||
@@ -606,7 +610,7 @@ const selectedModelObjects = computed({
|
||||
|
||||
const selectedUseCaseObjects = computed({
|
||||
get() {
|
||||
return selectedUseCases.value.map((useCase) => ({
|
||||
return activeUseCases.value.map((useCase) => ({
|
||||
name: useCase,
|
||||
value: useCase
|
||||
}))
|
||||
|
||||
@@ -395,4 +395,87 @@ describe('useTemplateFiltering', () => {
|
||||
expect(mockGetFuseOptions).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Scope-aware filtering', () => {
|
||||
it('filters out inactive models when scope changes', () => {
|
||||
// Start with image templates only
|
||||
const templates = ref<TemplateInfo[]>([
|
||||
{
|
||||
name: 'flux-template',
|
||||
description: 'Flux model template',
|
||||
models: ['Flux', 'Dall-E'],
|
||||
mediaType: 'image',
|
||||
mediaSubtype: 'png'
|
||||
}
|
||||
])
|
||||
|
||||
const currentScope = ref('image')
|
||||
|
||||
const {
|
||||
selectedModels,
|
||||
activeModels,
|
||||
inactiveModels,
|
||||
filteredTemplates
|
||||
} = useTemplateFiltering(templates, currentScope)
|
||||
|
||||
// Select models from both image and video domains
|
||||
selectedModels.value = ['Flux', 'Luma']
|
||||
|
||||
// In image scope, only Flux should be active because Luma doesn't exist in any image template
|
||||
expect(activeModels.value).toEqual(['Flux'])
|
||||
expect(inactiveModels.value).toEqual(['Luma'])
|
||||
expect(filteredTemplates.value).toHaveLength(1)
|
||||
expect(filteredTemplates.value[0].name).toBe('flux-template')
|
||||
|
||||
// Switch to video scope with only video templates
|
||||
currentScope.value = 'video'
|
||||
templates.value = [
|
||||
{
|
||||
name: 'luma-template',
|
||||
description: 'Luma video template',
|
||||
models: ['Luma', 'Runway'],
|
||||
mediaType: 'video',
|
||||
mediaSubtype: 'mp4'
|
||||
}
|
||||
]
|
||||
|
||||
// In video scope, only Luma should be active because Flux doesn't exist in any video template
|
||||
expect(activeModels.value).toEqual(['Luma'])
|
||||
expect(inactiveModels.value).toEqual(['Flux'])
|
||||
expect(filteredTemplates.value).toHaveLength(1)
|
||||
expect(filteredTemplates.value[0].name).toBe('luma-template')
|
||||
})
|
||||
|
||||
it('maintains selected filters across scope changes', () => {
|
||||
const templates = ref<TemplateInfo[]>([
|
||||
{
|
||||
name: 'template1',
|
||||
description: 'Template 1',
|
||||
models: ['Model1'],
|
||||
mediaType: 'image',
|
||||
mediaSubtype: 'png'
|
||||
}
|
||||
])
|
||||
|
||||
const currentScope = ref('image')
|
||||
const { selectedModels, activeModels } = useTemplateFiltering(
|
||||
templates,
|
||||
currentScope
|
||||
)
|
||||
|
||||
// Select a model
|
||||
selectedModels.value = ['Model1', 'Model2']
|
||||
|
||||
// Model1 is active, Model2 is not available
|
||||
expect(activeModels.value).toEqual(['Model1'])
|
||||
expect(selectedModels.value).toEqual(['Model1', 'Model2'])
|
||||
|
||||
// Change scope - selected models should persist
|
||||
currentScope.value = 'video'
|
||||
templates.value = []
|
||||
|
||||
expect(selectedModels.value).toEqual(['Model1', 'Model2'])
|
||||
expect(activeModels.value).toEqual([])
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -26,7 +26,8 @@ const defaultFuseOptions: IFuseOptions<TemplateInfo> = {
|
||||
}
|
||||
|
||||
export function useTemplateFiltering(
|
||||
templates: Ref<TemplateInfo[]> | TemplateInfo[]
|
||||
templates: Ref<TemplateInfo[]> | TemplateInfo[],
|
||||
currentScope?: Ref<string | null>
|
||||
) {
|
||||
const settingStore = useSettingStore()
|
||||
const rankingStore = useTemplateRankingStore()
|
||||
@@ -84,6 +85,40 @@ export function useTemplateFiltering(
|
||||
return ['ComfyUI', 'External or Remote API']
|
||||
})
|
||||
|
||||
// Compute which selected filters are actually applicable to the current scope
|
||||
const activeModels = computed(() => {
|
||||
if (!currentScope) {
|
||||
return selectedModels.value
|
||||
}
|
||||
return selectedModels.value.filter((model) =>
|
||||
availableModels.value.includes(model)
|
||||
)
|
||||
})
|
||||
|
||||
const activeUseCases = computed(() => {
|
||||
if (!currentScope) {
|
||||
return selectedUseCases.value
|
||||
}
|
||||
return selectedUseCases.value.filter((useCase) =>
|
||||
availableUseCases.value.includes(useCase)
|
||||
)
|
||||
})
|
||||
|
||||
// Track which filters are inactive (selected but not applicable)
|
||||
const inactiveModels = computed(() => {
|
||||
if (!currentScope) return []
|
||||
return selectedModels.value.filter(
|
||||
(model) => !availableModels.value.includes(model)
|
||||
)
|
||||
})
|
||||
|
||||
const inactiveUseCases = computed(() => {
|
||||
if (!currentScope) return []
|
||||
return selectedUseCases.value.filter(
|
||||
(useCase) => !availableUseCases.value.includes(useCase)
|
||||
)
|
||||
})
|
||||
|
||||
const debouncedSearchQuery = refThrottled(searchQuery, 50)
|
||||
|
||||
const filteredBySearch = computed(() => {
|
||||
@@ -96,7 +131,8 @@ export function useTemplateFiltering(
|
||||
})
|
||||
|
||||
const filteredByModels = computed(() => {
|
||||
if (selectedModels.value.length === 0) {
|
||||
// Use active models instead of selected models for filtering
|
||||
if (activeModels.value.length === 0) {
|
||||
return filteredBySearch.value
|
||||
}
|
||||
|
||||
@@ -104,14 +140,15 @@ export function useTemplateFiltering(
|
||||
if (!template.models || !Array.isArray(template.models)) {
|
||||
return false
|
||||
}
|
||||
return selectedModels.value.some((selectedModel) =>
|
||||
template.models?.includes(selectedModel)
|
||||
return activeModels.value.some((activeModel) =>
|
||||
template.models?.includes(activeModel)
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
const filteredByUseCases = computed(() => {
|
||||
if (selectedUseCases.value.length === 0) {
|
||||
// Use active use cases instead of selected use cases for filtering
|
||||
if (activeUseCases.value.length === 0) {
|
||||
return filteredByModels.value
|
||||
}
|
||||
|
||||
@@ -119,13 +156,14 @@ export function useTemplateFiltering(
|
||||
if (!template.tags || !Array.isArray(template.tags)) {
|
||||
return false
|
||||
}
|
||||
return selectedUseCases.value.some((selectedTag) =>
|
||||
template.tags?.includes(selectedTag)
|
||||
return activeUseCases.value.some((activeUseCase) =>
|
||||
template.tags?.includes(activeUseCase)
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
const filteredByRunsOn = computed(() => {
|
||||
// RunsOn filters are scope-independent
|
||||
if (selectedRunsOn.value.length === 0) {
|
||||
return filteredByUseCases.value
|
||||
}
|
||||
@@ -137,10 +175,10 @@ export function useTemplateFiltering(
|
||||
const isExternalAPI = template.openSource === false
|
||||
const isComfyUI = template.openSource !== false
|
||||
|
||||
return selectedRunsOn.value.some((selectedRunsOn) => {
|
||||
if (selectedRunsOn === 'External or Remote API') {
|
||||
return selectedRunsOn.value.some((runsOn) => {
|
||||
if (runsOn === 'External or Remote API') {
|
||||
return isExternalAPI
|
||||
} else if (selectedRunsOn === 'ComfyUI') {
|
||||
} else if (runsOn === 'ComfyUI') {
|
||||
return isComfyUI
|
||||
}
|
||||
return false
|
||||
@@ -343,6 +381,14 @@ export function useTemplateFiltering(
|
||||
selectedRunsOn,
|
||||
sortBy,
|
||||
|
||||
// Computed - Active filters (actually applied)
|
||||
activeModels,
|
||||
activeUseCases,
|
||||
|
||||
// Computed - Inactive filters (selected but not applicable)
|
||||
inactiveModels,
|
||||
inactiveUseCases,
|
||||
|
||||
// Computed
|
||||
filteredTemplates,
|
||||
availableModels,
|
||||
|
||||
Reference in New Issue
Block a user