Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions browser_tests/fixtures/components/ComfyNodeSearchBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class ComfyNodeSearchBox {
'.comfy-vue-node-search-container input[type="text"]'
)
this.dropdown = page.locator(
'.comfy-vue-node-search-container .p-autocomplete-list'
'.comfy-vue-node-search-container .comfy-autocomplete-list'
)
this.filterSelectionPanel = new ComfyNodeSearchFilterSelectionPanel(page)
}
Expand All @@ -61,7 +61,7 @@ export class ComfyNodeSearchBox {
await this.input.fill(nodeName)
await this.dropdown.waitFor({ state: 'visible' })
await this.dropdown
.locator('li')
.locator('.option-container')
.nth(options?.suggestionIndex || 0)
.click()
}
Expand Down
52 changes: 0 additions & 52 deletions src/components/primevueOverride/AutoCompletePlus.vue

This file was deleted.

190 changes: 98 additions & 92 deletions src/components/searchbox/NodeSearchBox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,12 @@
/>
</div>

<Button
variant="secondary"
:aria-label="$t('g.addNodeFilterCondition')"
class="filter-button z-10"
@click="nodeSearchFilterVisible = true"
>
<i class="pi pi-filter" />
</Button>
<Dialog
v-model:visible="nodeSearchFilterVisible"
class="min-w-96"
dismissable-mask
modal
@hide="reFocusInput"
@hide="nextTick(() => inputRef?.focus())"
>
<template #header>
<h3>{{ $t('g.addNodeFilterCondition') }}</h3>
Expand All @@ -36,57 +28,84 @@
</div>
</Dialog>

<AutoCompletePlus
ref="autoCompletePlus"
:model-value="filters"
class="comfy-vue-node-search-box z-10 grow"
scroll-height="40vh"
:placeholder="placeholder"
:input-id="inputId"
append-to="self"
:suggestions="suggestions"
:delay="100"
:loading="!nodeFrequencyStore.isLoaded"
complete-on-focus
auto-option-focus
force-selection
multiple
option-label="display_name"
@complete="search($event.query)"
@option-select="onAddNode($event.value)"
@focused-option-changed="setHoverSuggestion($event)"
>
<template #option="{ option }">
<NodeSearchItem :node-def="option" :current-query="currentQuery" />
</template>
<!-- FilterAndValue -->
<template #chip="{ value }">
<SearchFilterChip
v-if="value.filterDef && value.value"
<div class="comfy-vue-node-search-box z-10 grow">
<div
class="flex w-full items-center bg-base-background rounded-lg py-1 px-4 border-primary-background border"
>
<Button
variant="secondary"
:aria-label="$t('g.addNodeFilterCondition')"
class="filter-button z-10 absolute -left-10"
@click="nodeSearchFilterVisible = true"
>
<i class="pi pi-filter" />
</Button>
<template
v-for="value in filters"
:key="`${value.filterDef.id}-${value.value}`"
:text="value.value"
:badge="value.filterDef.invokeSequence.toUpperCase()"
:badge-class="value.filterDef.invokeSequence + '-badge'"
@remove="
onRemoveFilter(
$event,
value as FuseFilterWithValue<ComfyNodeDefImpl, string>
)
"
>
<SearchFilterChip
v-if="value.filterDef && value.value"
:text="value.value"
:badge="value.filterDef.invokeSequence.toUpperCase()"
:badge-class="value.filterDef.invokeSequence + '-badge'"
@remove="
onRemoveFilter(
$event,
value as FuseFilterWithValue<ComfyNodeDefImpl, string>
)
"
/>
</template>
<input
ref="inputRef"
v-model="currentQuery"
class="text-base h-5 bg-transparent border-0 focus:outline-0 flex-1"
type="text"
autofocus
:placeholder="t('g.searchNodes') + '...'"
@keydown.enter.prevent="onAddNode(hoveredSuggestion)"
@keydown.down.prevent="updateIndexBy(1)"
@keydown.up.prevent="updateIndexBy(-1)"
/>
</template>
</AutoCompletePlus>
</div>
<div
v-bind="containerProps"
class="bg-comfy-menu-bg p-1 rounded-lg border-border-subtle border max-h-150"
>
<div v-bind="wrapperProps" class="comfy-autocomplete-list">
<NodeSearchItem
v-for="{ data: option, index } in virtualList"
:key="index"
:class="
cn(
'p-1 rounded-sm',
hoveredIndex === index && 'bg-secondary-background-hover'
)
"
:node-def="option"
:current-query="debouncedQuery"
@click="onAddNode(option)"
@pointerover="hoveredIndex = index"
/>
</div>
<div
v-if="suggestions.length === 0"
class="p-1"
v-text="t('g.noResultsFound')"
/>
</div>
</div>
</div>
</template>

<script setup lang="ts">
import { debounce } from 'es-toolkit/compat'
import { refDebounced, useVirtualList } from '@vueuse/core'
import Dialog from 'primevue/dialog'
import { computed, nextTick, onMounted, ref } from 'vue'
import { computed, nextTick, ref, useTemplateRef, watchEffect } from 'vue'
import { useI18n } from 'vue-i18n'

import NodePreview from '@/components/node/NodePreview.vue'
import AutoCompletePlus from '@/components/primevueOverride/AutoCompletePlus.vue'
import NodeSearchFilter from '@/components/searchbox/NodeSearchFilter.vue'
import NodeSearchItem from '@/components/searchbox/NodeSearchItem.vue'
import Button from '@/components/ui/button/Button.vue'
Expand All @@ -95,6 +114,7 @@ import { useTelemetry } from '@/platform/telemetry'
import type { ComfyNodeDefImpl } from '@/stores/nodeDefStore'
import { useNodeDefStore, useNodeFrequencyStore } from '@/stores/nodeDefStore'
import type { FuseFilterWithValue } from '@/utils/fuseUtil'
import { cn } from '@/utils/tailwindUtil'

import SearchFilterChip from '../common/SearchFilterChip.vue'

Expand All @@ -111,68 +131,52 @@ const { filters, searchLimit = 64 } = defineProps<{
searchLimit?: number
}>()

const autoCompletePlus = ref()
const nodeSearchFilterVisible = ref(false)
const inputId = `comfy-vue-node-search-box-input-${Math.random()}`
const suggestions = ref<ComfyNodeDefImpl[]>([])
const hoveredSuggestion = ref<ComfyNodeDefImpl | null>(null)
const currentQuery = ref('')
const placeholder = computed(() => {
return filters.length === 0 ? t('g.searchNodes') + '...' : ''
})
const debouncedQuery = refDebounced(currentQuery, 100, { maxWait: 400 })
const inputRef = useTemplateRef('inputRef')

const nodeDefStore = useNodeDefStore()
const nodeFrequencyStore = useNodeFrequencyStore()

// Debounced search tracking (500ms as per implementation plan)
const debouncedTrackSearch = debounce((query: string) => {
watchEffect(() => {
const query = debouncedQuery.value
if (query.trim()) {
telemetry?.trackNodeSearch({ query })
}
}, 500)
})

const search = (query: string) => {
const suggestions = computed(() => {
const query = debouncedQuery.value
const queryIsEmpty = query === '' && filters.length === 0
currentQuery.value = query
suggestions.value = queryIsEmpty

return queryIsEmpty
? nodeFrequencyStore.topNodeDefs
: [
...nodeDefStore.nodeSearchService.searchNode(query, filters, {
limit: searchLimit
})
]
})

// Track search queries with debounce
debouncedTrackSearch(query)
}
const {
list: virtualList,
containerProps,
wrapperProps
} = useVirtualList(suggestions, { itemHeight: 40 })

const emit = defineEmits(['addFilter', 'removeFilter', 'addNode'])

// Track node selection and emit addNode event
const onAddNode = (nodeDef: ComfyNodeDefImpl) => {
const onAddNode = (nodeDef?: ComfyNodeDefImpl) => {
if (!nodeDef) return
telemetry?.trackNodeSearchResultSelected({
node_type: nodeDef.name,
last_query: currentQuery.value
last_query: debouncedQuery.value
})
emit('addNode', nodeDef)
}

let inputElement: HTMLInputElement | null = null
const reFocusInput = async () => {
inputElement ??= document.getElementById(inputId) as HTMLInputElement
if (inputElement) {
inputElement.blur()
await nextTick(() => inputElement?.focus())
}
}

onMounted(() => {
inputElement ??= document.getElementById(inputId) as HTMLInputElement
if (inputElement) inputElement.focus()
autoCompletePlus.value.hide = () => search('')
search('')
autoCompletePlus.value.show()
})
const onAddFilter = (
filterAndValue: FuseFilterWithValue<ComfyNodeDefImpl, string>
) => {
Expand All @@ -186,14 +190,16 @@ const onRemoveFilter = async (
event.stopPropagation()
event.preventDefault()
emit('removeFilter', filterAndValue)
await reFocusInput()
inputRef.value?.focus()
}
const setHoverSuggestion = (index: number) => {
if (index === -1) {
hoveredSuggestion.value = null
return
}
const value = suggestions.value[index]
hoveredSuggestion.value = value
const hoveredIndex = ref<number>()
const hoveredSuggestion = computed(() =>
hoveredIndex.value ? suggestions.value[hoveredIndex.value] : undefined
)
function updateIndexBy(delta: number) {
hoveredIndex.value = Math.max(
0,
Math.min(suggestions.value.length, (hoveredIndex.value ?? 0) + delta)
)
}
Comment on lines +195 to 204
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Off-by-one errors in hover selection logic.

Two issues:

  1. Line 197: hoveredIndex.value ? uses a truthy check, which returns undefined when index is 0 (first item). Users cannot select the first suggestion via keyboard or hover.

  2. Line 202: Upper bound should be suggestions.value.length - 1, not length. Current code allows index to equal array length, resulting in undefined on access.

🐛 Proposed fix
-const hoveredSuggestion = computed(() =>
-  hoveredIndex.value ? suggestions.value[hoveredIndex.value] : undefined
-)
+const hoveredSuggestion = computed(() =>
+  hoveredIndex.value !== undefined
+    ? suggestions.value[hoveredIndex.value]
+    : undefined
+)
 function updateIndexBy(delta: number) {
   hoveredIndex.value = Math.max(
     0,
-    Math.min(suggestions.value.length, (hoveredIndex.value ?? 0) + delta)
+    Math.min(suggestions.value.length - 1, (hoveredIndex.value ?? 0) + delta)
   )
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const hoveredIndex = ref<number>()
const hoveredSuggestion = computed(() =>
hoveredIndex.value ? suggestions.value[hoveredIndex.value] : undefined
)
function updateIndexBy(delta: number) {
hoveredIndex.value = Math.max(
0,
Math.min(suggestions.value.length, (hoveredIndex.value ?? 0) + delta)
)
}
const hoveredIndex = ref<number>()
const hoveredSuggestion = computed(() =>
hoveredIndex.value !== undefined
? suggestions.value[hoveredIndex.value]
: undefined
)
function updateIndexBy(delta: number) {
hoveredIndex.value = Math.max(
0,
Math.min(suggestions.value.length - 1, (hoveredIndex.value ?? 0) + delta)
)
}
🤖 Prompt for AI Agents
In @src/components/searchbox/NodeSearchBox.vue around lines 195 - 204, The hover
selection has two off-by-one bugs: change the computed hoveredSuggestion to
check for an explicit index (e.g., hoveredIndex.value !== undefined) instead of
a truthy check so index 0 is valid, and update updateIndexBy to clamp against
suggestions.value.length - 1 (not length) and ensure you handle empty
suggestions (e.g., if length is 0 set hoveredIndex.value = undefined or clamp to
-1/undefined). Specifically update hoveredSuggestion and updateIndexBy (the
functions/refs named hoveredIndex, hoveredSuggestion, updateIndexBy, and
suggestions) to use an explicit undefined check and to use Math.min(...,
suggestions.value.length - 1) with proper empty-array handling.

</script>
7 changes: 3 additions & 4 deletions src/components/searchbox/NodeSearchBoxPopover.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@
},
mask: { class: 'node-search-box-dialog-mask' },
transition: {
enterFromClass: 'opacity-0 scale-75',
// 100ms is the duration of the transition in the dialog component
enterActiveClass: 'transition-all duration-100 ease-out',
leaveActiveClass: 'transition-all duration-100 ease-in',
enterFromClass: 'opacity-0',
enterActiveClass: 'transition-all duration-20 ease-out',
leaveActiveClass: 'transition-all duration-20 ease-in',
leaveToClass: 'opacity-0 scale-75'
}
}"
Expand Down
Loading