feat: add Input/Output type sub-menu filters to Nodes panel sidebar#11294
Draft
comfydesigner wants to merge 2 commits intoComfy-Org:mainfrom
Draft
feat: add Input/Output type sub-menu filters to Nodes panel sidebar#11294comfydesigner wants to merge 2 commits intoComfy-Org:mainfrom
comfydesigner wants to merge 2 commits intoComfy-Org:mainfrom
Conversation
- Add Input Type and Output Type sub-menus to the filter dropdown - Each sub-menu has a searchable list of types with multi-select checkboxes - Selected types shown as colored socket badges in the parent menu - Footer shows selected count with Clear all button - Type filters applied to both search and browse modes - Keep dropdown open on checkbox toggle with @select.prevent - Fix menu width stability with fixed w-48 Amp-Thread-ID: https://ampcode.com/threads/T-019d9399-6be0-76ab-aeb3-d5f688e5af43 Co-authored-by: Amp <amp@ampcode.com>
|
Someone is attempting to deploy a commit to the ComfyUI Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded hierarchical Input and Output type submenu filters to the node library "all" filter UI with search, per-type selection, clear actions, and i18n keys; introduced reactive selected type state and integrated active type filters into node search and result post-filtering. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as NodeLibrarySidebarTabV2 (UI)
participant Store as nodeDefStore
participant Search as nodeSearchService
participant Filter as filterDef
UI->>Store: read available node type indexes
UI->>UI: update selectedInputTypes / selectedOutputTypes
UI->>Search: searchNode(query, activeTypeFilters)
Search-->>UI: base search results
UI->>Filter: apply filterDef.matches per activeTypeFilters
Filter-->>UI: filtered activeNodes
UI-->>UI: render nodes list based on activeNodes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue (1)
44-44:⚠️ Potential issue | 🟠 MajorDon’t keep type filters active after hiding the control.
The submenu only exists on the All tab, but
activeNodesappliesactiveTypeFiltersto Essentials and Blueprints too. After a tab switch, those tabs can stay filtered with no visible way to understand or clear the state.💡 One safe fix
const activeTypeFilters = computed< FuseFilterWithValue<ComfyNodeDefImpl, string>[] >(() => { + if (selectedTab.value !== 'all') { + return [] + } + const filters: FuseFilterWithValue<ComfyNodeDefImpl, string>[] = [] const searchService = nodeDefStore.nodeSearchService // ... })Also applies to: 529-540
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue` at line 44, The activeTypeFilters are being applied globally via activeNodes even when the DropdownMenuRoot (submenu) is only shown for the All tab, causing hidden filters to persist on Essentials/Blueprints; update the logic so filters only affect nodes when selectedTab === 'all' (e.g. change the activeNodes computed getter or filtering function to short-circuit and ignore/return unfiltered results when selectedTab !== 'all'), or alternatively clear/reset activeTypeFilters when selectedTab changes away from 'all' (add a watcher on selectedTab that sets activeTypeFilters = [] when it becomes non-'all'); reference activeNodes, activeTypeFilters, selectedTab and DropdownMenuRoot when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue`:
- Around line 145-158: The submenu search inputs in NodeLibrarySidebarTabV2.vue
(the <input> with v-model="inputTypeSearch" and the other input around lines
245-251) lack accessible names; add explicit accessible labeling by providing
either a visible <label for="..."> tied to a unique id or an
aria-label/aria-labelledby on each input (e.g.,
:aria-label="$t('nodeLibrary.searchTypes')" or similar localized string) so
screen readers and keyboard users can identify each field; ensure the id used in
the label matches the input id and use the same pattern for both the
inputTypeSearch field and the other submenu input.
- Around line 494-505: The activeTypeFilters computed currently emits one
FuseFilterWithValue per selected type, causing an unintended AND across the
submenu; change activeTypeFilters to emit at most one input-type filter and one
output-type filter by joining selectedInputTypes.value and
selectedOutputTypes.value into comma-separated strings and pushing a single {
filterDef: nodeDefStore.nodeSearchService.inputTypeFilter, value: joined } (and
similarly for outputTypeFilter) only when that submenu has selections, so
FuseFilter.matches() can use its built-in OR behavior; keep the same
FuseFilterWithValue type and ensure no filters are added when the selection
arrays are empty.
---
Outside diff comments:
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue`:
- Line 44: The activeTypeFilters are being applied globally via activeNodes even
when the DropdownMenuRoot (submenu) is only shown for the All tab, causing
hidden filters to persist on Essentials/Blueprints; update the logic so filters
only affect nodes when selectedTab === 'all' (e.g. change the activeNodes
computed getter or filtering function to short-circuit and ignore/return
unfiltered results when selectedTab !== 'all'), or alternatively clear/reset
activeTypeFilters when selectedTab changes away from 'all' (add a watcher on
selectedTab that sets activeTypeFilters = [] when it becomes non-'all');
reference activeNodes, activeTypeFilters, selectedTab and DropdownMenuRoot when
making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 667e1bef-d093-446f-beb9-1f798d0a6da8
📒 Files selected for processing (2)
src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vuesrc/locales/en/main.json
Comment on lines
+145
to
+158
| <div class="px-3 pt-3 pb-1"> | ||
| <div | ||
| class="flex items-center gap-2 rounded-md border border-border-default bg-comfy-input px-2 py-1" | ||
| > | ||
| <i | ||
| class="icon-[lucide--search] size-3.5 shrink-0 text-muted-foreground" | ||
| /> | ||
| <input | ||
| v-model="inputTypeSearch" | ||
| type="text" | ||
| :placeholder="$t('g.search') + '...'" | ||
| class="w-full border-0 bg-transparent text-sm outline-none placeholder:text-muted-foreground" | ||
| @keydown.stop | ||
| /> |
Contributor
There was a problem hiding this comment.
Label the submenu search fields.
Both new <input> controls rely on placeholder text only. That leaves them without a proper accessible name, which is a regression for keyboard/screen-reader navigation in the filter menu.
💡 Minimal fix
<input
v-model="inputTypeSearch"
type="text"
+ :aria-label="$t('g.searchPlaceholder', { subject: $t('g.input') })"
:placeholder="$t('g.search') + '...'"
class="w-full border-0 bg-transparent text-sm outline-none placeholder:text-muted-foreground"
`@keydown.stop`
/> <input
v-model="outputTypeSearch"
type="text"
+ :aria-label="$t('g.searchPlaceholder', { subject: $t('g.output') })"
:placeholder="$t('g.search') + '...'"
class="w-full border-0 bg-transparent text-sm outline-none placeholder:text-muted-foreground"
`@keydown.stop`
/>Also applies to: 245-251
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue` around lines 145 -
158, The submenu search inputs in NodeLibrarySidebarTabV2.vue (the <input> with
v-model="inputTypeSearch" and the other input around lines 245-251) lack
accessible names; add explicit accessible labeling by providing either a visible
<label for="..."> tied to a unique id or an aria-label/aria-labelledby on each
input (e.g., :aria-label="$t('nodeLibrary.searchTypes')" or similar localized
string) so screen readers and keyboard users can identify each field; ensure the
id used in the label matches the input id and use the same pattern for both the
inputTypeSearch field and the other submenu input.
Comment on lines
+494
to
+505
| const activeTypeFilters = computed< | ||
| FuseFilterWithValue<ComfyNodeDefImpl, string>[] | ||
| >(() => { | ||
| const filters: FuseFilterWithValue<ComfyNodeDefImpl, string>[] = [] | ||
| const searchService = nodeDefStore.nodeSearchService | ||
| for (const type of selectedInputTypes.value) { | ||
| filters.push({ filterDef: searchService.inputTypeFilter, value: type }) | ||
| } | ||
| for (const type of selectedOutputTypes.value) { | ||
| filters.push({ filterDef: searchService.outputTypeFilter, value: type }) | ||
| } | ||
| return filters |
Contributor
There was a problem hiding this comment.
Group same-menu selections into one filter value.
Right now each checked type becomes its own filter entry, and activeNodes later applies every(...), so multi-select behaves like AND inside a submenu. For checkbox filters, users will expect OR within Input and within Output. FuseFilter.matches() already supports comma-separated OR values, so this should be collapsed here instead of emitting one filter per type.
💡 Minimal fix
const activeTypeFilters = computed<
FuseFilterWithValue<ComfyNodeDefImpl, string>[]
>(() => {
const filters: FuseFilterWithValue<ComfyNodeDefImpl, string>[] = []
const searchService = nodeDefStore.nodeSearchService
- for (const type of selectedInputTypes.value) {
- filters.push({ filterDef: searchService.inputTypeFilter, value: type })
- }
- for (const type of selectedOutputTypes.value) {
- filters.push({ filterDef: searchService.outputTypeFilter, value: type })
- }
+ if (selectedInputTypes.value.size > 0) {
+ filters.push({
+ filterDef: searchService.inputTypeFilter,
+ value: Array.from(selectedInputTypes.value).join(',')
+ })
+ }
+ if (selectedOutputTypes.value.size > 0) {
+ filters.push({
+ filterDef: searchService.outputTypeFilter,
+ value: Array.from(selectedOutputTypes.value).join(',')
+ })
+ }
return filters
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue` around lines 494 -
505, The activeTypeFilters computed currently emits one FuseFilterWithValue per
selected type, causing an unintended AND across the submenu; change
activeTypeFilters to emit at most one input-type filter and one output-type
filter by joining selectedInputTypes.value and selectedOutputTypes.value into
comma-separated strings and pushing a single { filterDef:
nodeDefStore.nodeSearchService.inputTypeFilter, value: joined } (and similarly
for outputTypeFilter) only when that submenu has selections, so
FuseFilter.matches() can use its built-in OR behavior; keep the same
FuseFilterWithValue type and ensure no filters are added when the selection
arrays are empty.
- Add 'Clear type filters' menu item below Output Type sub-menu - Disabled state when no type filters active (data-[disabled] for Reka UI) - Keep dropdown open on checkbox toggle for category filters - Replace DropdownMenuItemIndicator with fixed-size spans to prevent layout shift - Fix menu width to w-48 to prevent resize on check toggle Amp-Thread-ID: https://ampcode.com/threads/T-019d9399-6be0-76ab-aeb3-d5f688e5af43 Co-authored-by: Amp <amp@ampcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add Input Type and Output Type sub-menu filters to the existing filter dropdown in the Nodes panel sidebar.
Changes:
Files changed:
src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vuesrc/locales/en/main.json