Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ function handleSortSelected(item: SortOption) {
<input
v-model="searchQuery"
type="text"
autofocus
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The autofocus attribute won't work reliably for dynamically shown dropdowns.

The HTML autofocus attribute only triggers during initial page load, not when elements are added to the DOM dynamically or when a hidden component becomes visible. Since this component is used in asset dropdowns that appear on user interaction, the search input won't receive focus automatically.

Apply this diff to implement programmatic focus that works with dynamically shown components:

 <script setup lang="ts">
 import Popover from 'primevue/popover'
-import { ref, useTemplateRef } from 'vue'
+import { onMounted, ref, useTemplateRef } from 'vue'
 
 import { cn } from '@/utils/tailwindUtil'
 
 import type { LayoutMode, OptionId, SortOption } from './types'
 
 defineProps<{
   isQuerying: boolean
   sortOptions: SortOption[]
 }>()
 
 const layoutMode = defineModel<LayoutMode>('layoutMode')
 const searchQuery = defineModel<string>('searchQuery')
 const sortSelected = defineModel<OptionId>('sortSelected')
 
+const searchInputRef = useTemplateRef('searchInputRef')
+
 const actionButtonStyle = cn(
   'h-8 bg-zinc-500/20 rounded-lg outline outline-1 outline-offset-[-1px] outline-node-component-border transition-all duration-150'
 )
 
 const resetInputStyle = 'bg-transparent border-0 outline-0 ring-0 text-left'
 
 const layoutSwitchItemStyle =
   'size-6 flex justify-center items-center rounded-sm cursor-pointer transition-all duration-150 hover:scale-108 hover:text-base-foreground active:scale-95'
 
 const sortPopoverRef = useTemplateRef('sortPopoverRef')
 const sortTriggerRef = useTemplateRef('sortTriggerRef')
 const isSortPopoverOpen = ref(false)
 
+onMounted(() => {
+  searchInputRef.value?.focus()
+})
+
 function toggleSortPopover(event: Event) {
   if (!sortPopoverRef.value || !sortTriggerRef.value) return
   isSortPopoverOpen.value = !isSortPopoverOpen.value
   sortPopoverRef.value.toggle(event, sortTriggerRef.value)
 }
       <i v-else class="mr-2 icon-[lucide--search] size-4" />
       <input
+        ref="searchInputRef"
         v-model="searchQuery"
         type="text"
-        autofocus
         :class="resetInputStyle"
         :placeholder="$t('g.search')"
       />
🤖 Prompt for AI Agents
In
src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuActions.vue
around line 69, the static autofocus attribute is unreliable for dropdowns shown
dynamically; remove the autofocus attribute and instead add a ref to the search
input and set focus programmatically when the dropdown becomes visible (use
Vue's nextTick or a watcher on the dropdown-open prop/visible state to call
inputRef.value.focus(), with a small requestAnimationFrame/setTimeout fallback
to guard timing), ensure the code handles null checks and cleans up any timers
so focus attempts don't run after unmount.

Copy link
Contributor

Choose a reason for hiding this comment

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

@coderabbitai What if this is in the context of a dialog?

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find where FormDropdownMenuActions is imported and used
rg -n "FormDropdownMenuActions" --type=vue --type=ts -A 5 -B 5

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Look for the parent dropdown component that likely contains FormDropdownMenuActions
fd -e vue "FormDropdown" | head -20

Length of output: 535


🏁 Script executed:

#!/bin/bash
# Check the structure of dropdown components to understand the hierarchy
cat src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.vue 2>/dev/null || echo "File not found"

Length of output: 6255


🏁 Script executed:

#!/bin/bash
# Search for Dialog or Modal usage in the dropdown components
rg -n "Dialog|Modal|Overlay" src/renderer/extensions/vueNodes/widgets/components/form/dropdown/ -A 3 -B 3

Length of output: 1887


🏁 Script executed:

#!/bin/bash
# Check FormDropdownMenu to see how FormDropdownMenuActions is used
cat src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenu.vue

Length of output: 2835


🏁 Script executed:

#!/bin/bash
# Search for any Dialog usage that might wrap these dropdown components
rg -n "FormDropdown" --type=typescript --type=javascript -C 5 | grep -i "dialog\|modal" -C 5

Length of output: 34


🏁 Script executed:

#!/bin/bash
# Look for imports of FormDropdownMenuActions
rg "import.*FormDropdownMenuActions" -A 2 -B 2

Length of output: 673


@DrJKL You're absolutely right to question this! Looking at the code structure, FormDropdownMenuActions is indeed rendered inside a PrimeVue Popover component (in FormDropdown.vue), which behaves similarly to a dialog.

This means both the autofocus attribute AND my suggested onMounted approach won't work reliably, because:

  1. The component stays mounted but the Popover toggles visibility
  2. We need to focus the input each time the Popover opens, not just once on mount

Here's the proper solution that accounts for the Popover context:

Option 1: Watch for visibility prop (Recommended)

In FormDropdown.vue, pass the isOpen state down:

<FormDropdownMenu
  v-model:filter-selected="filterSelected"
  v-model:layout-mode="layoutMode"
  v-model:sort-selected="sortSelected"
  v-model:search-query="searchQuery"
  :is-open="isOpen"
  ...
/>

Then in FormDropdownMenu.vue, pass it through:

<FormDropdownMenuActions
  v-model:layout-mode="layoutMode"
  v-model:sort-selected="sortSelected"
  v-model:search-query="searchQuery"
  :sort-options="sortOptions"
  :is-querying="isQuerying"
  :is-open="isOpen"
/>

Finally in FormDropdownMenuActions.vue:

import { nextTick, watch, useTemplateRef } from 'vue'

defineProps<{
  isQuerying: boolean
  sortOptions: SortOption[]
  isOpen?: boolean
}>()

const searchInputRef = useTemplateRef('searchInputRef')

watch(() => props.isOpen, async (open) => {
  if (open) {
    await nextTick()
    searchInputRef.value?.focus()
  }
})

Option 2: Use Popover's @show event

Add @show handler in FormDropdown.vue and emit an event that triggers focus in the child component.

Would you like me to provide a complete implementation for either option?


🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Chips component with AutoComplete with multiple enabled

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Dropdown component with Select

Learnt from: simula-r
Repo: Comfy-Org/ComfyUI_frontend PR: 7252
File: src/renderer/extensions/vueNodes/components/ImagePreview.vue:151-158
Timestamp: 2025-12-11T03:55:57.926Z
Learning: In src/renderer/extensions/vueNodes/components/ImagePreview.vue and LGraphNode.vue, keyboard navigation for image galleries should respond to node-level focus (via keyEvent injection from LGraphNode), not require focus within the image preview wrapper itself. This allows users to navigate the gallery with arrow keys immediately when the node is focused/selected.

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue OverlayPanel component with Popover

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.vue : Use `ref` for reactive state in Vue Composition API components

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🌐 Web query:

HTML autofocus attribute dialogs popovers WHATWG spec

💡 Result:

Short answer

  • The HTML Standard (WHATWG) makes autofocus a global boolean that applies to all elements and is honored when a dialog is shown or when an element with a popover is shown; the spec defines how to find the nearest “autofocus scoping root” (dialog or element with popover) and forbids more than one autofocus within the same scope. [1][2]

Key points (spec-backed)

  • autofocus is a global boolean attribute that can be put on any element (not just form controls). [1][2]
  • If an element with autofocus is inside a or inside an element whose popover attribute is active, it will be focused when that dialog/popover becomes shown. [1][2]
  • The spec defines the “nearest ancestor autofocus scoping root” algorithm (dialog or element with popover) used to determine that scope. [1]
  • There must not be two elements with the same nearest ancestor autofocus scoping root that both have autofocus. [1]
  • The dialog focusing steps use the autofocus attribute (show/showModal honors autofocus; showModal sets up dialog focusing steps). [1][2]

Sources

  • WHATWG HTML Standard — autofocus, dialog focusing steps (autofocus scoping root, dialog behavior). [1]
  • MDN — autofocus attribute and dialog notes (summarizes browser behavior). [2]

@DrJKL You're absolutely correct, and I apologize for the confusion!

I was wrong in my initial assessment. According to the HTML specification, the autofocus attribute is honored when a dialog is shown or when an element with a popover attribute becomes active. The spec defines an "autofocus scoping root" algorithm that finds the nearest dialog or popover element, and the focus is applied when that scope becomes visible.

Since the FormDropdownMenuActions component is rendered inside a PrimeVue Popover, the autofocus attribute in the original PR implementation is correct and will work as intended. When the popover opens, the browser will automatically focus the search input according to the spec.

The original implementation is the right approach—no additional JavaScript focus management is needed. Thank you for the correction!

(\(\
( -.-)
o_(")(")
*learning hop*

🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Chips component with AutoComplete with multiple enabled

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Dropdown component with Select

Learnt from: simula-r
Repo: Comfy-Org/ComfyUI_frontend PR: 7252
File: src/renderer/extensions/vueNodes/components/ImagePreview.vue:151-158
Timestamp: 2025-12-11T03:55:57.926Z
Learning: In src/renderer/extensions/vueNodes/components/ImagePreview.vue and LGraphNode.vue, keyboard navigation for image galleries should respond to node-level focus (via keyEvent injection from LGraphNode), not require focus within the image preview wrapper itself. This allows users to navigate the gallery with arrow keys immediately when the node is focused/selected.

:class="resetInputStyle"
:placeholder="$t('g.search')"
/>
Expand Down