feat: Node Library sidebar and V2 Search dialog UI/UX updates#9085
Merged
feat: Node Library sidebar and V2 Search dialog UI/UX updates#9085
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-node bookmarking, reorganizes node library into Blueprints/Partner/Comfy/Extensions/Recents with new filter UI and sortOrder flow, removes provided sidebar containerRef/panelRef from preview/drag logic (preview position now uses element rect), updates types, tests, locales, and several sidebar/panel components. Changes
Sequence DiagramsequenceDiagram
participant User
participant TreeNode as TreeExplorerV2Node
participant BookmarkStore as nodeBookmarkStore
participant Preview as useNodePreviewAndDrag
participant Sidebar as NodeLibrarySidebarTabV2
participant Service as NodeOrganizationService
User->>TreeNode: hover / focus node
TreeNode->>Preview: handleMouseEnter(el)
Preview->>Preview: compute preview pos from el.getBoundingClientRect()
Preview->>User: show preview
User->>TreeNode: click bookmark
TreeNode->>BookmarkStore: toggleBookmark(nodeId)
BookmarkStore-->>TreeNode: updated isBookmarked
TreeNode->>User: update bookmark icon
User->>Sidebar: open filters / change sort
Sidebar->>Service: request sections(filtered, sorted)
Service-->>Sidebar: sections (blueprints, partner, comfy, extensions)
Sidebar->>User: render panels (Blueprints/Essentials/All)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
christian-byrne
changed the title
![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: 5
🧹 Nitpick comments (14)
src/components/searchbox/v2/NodeSearchInput.test.ts (3)
56-73: Type test props withComponentProps<typeof NodeSearchInput>for stronger safety.♻️ Suggested refactor
import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { ComponentProps } from 'vue-component-type-helpers' function createWrapper( - props: Partial<{ - filters: FuseFilterWithValue<ComfyNodeDefImpl, string>[] - activeFilter: FilterChip | null - searchQuery: string - filterQuery: string - }> = {} + props: Partial<ComponentProps<typeof NodeSearchInput>> = {} ) { return mount(NodeSearchInput, { - props: { + props: { filters: [], activeFilter: null, searchQuery: '', filterQuery: '', ...props - }, + } as ComponentProps<typeof NodeSearchInput>, global: { plugins: [testI18n] } }) }Based on learnings: In test files, for helpers like mountComponent, type the props parameter as Partial<ComponentProps> and cast to ComponentProps when passing to mount.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchInput.test.ts` around lines 56 - 73, The test helper createWrapper should type its props parameter as Partial<ComponentProps<typeof NodeSearchInput>> and when passing props to mount cast the merged props to ComponentProps<typeof NodeSearchInput>; update the function signature and the props spread in the mount call (referencing createWrapper and NodeSearchInput) so TypeScript enforces correct prop shapes in tests.
26-47: Avoidas unknown asfor filter mocks; usesatisfiesfor shape validation.This keeps the helper aligned with type drift and avoids unsafe assertions.
♻️ Suggested refactor
+type FilterDefStub = Pick< + FuseFilter<ComfyNodeDefImpl, string>, + 'id' | 'matches' +> + function createFilter( id: string, value: string ): FuseFilterWithValue<ComfyNodeDefImpl, string> { return { - filterDef: { - id, - matches: vi.fn(() => true) - } as unknown as FuseFilter<ComfyNodeDefImpl, string>, + filterDef: { + id, + matches: vi.fn<FilterDefStub['matches']>(() => true) + } satisfies FilterDefStub, value } } function createActiveFilter(label: string): FilterChip { return { key: label.toLowerCase(), label, filter: { id: label.toLowerCase(), - matches: vi.fn(() => true) - } as unknown as FuseFilter<ComfyNodeDefImpl, string> + matches: vi.fn<FilterDefStub['matches']>(() => true) + } satisfies FilterDefStub } }Based on learnings: In test files at src/**/*.test.ts, when creating test helper functions that construct mock objects implementing an interface, use
satisfies InterfaceTypefor shape validation instead of type assertions likeas anyor double-casts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchInput.test.ts` around lines 26 - 47, The helpers createFilter and createActiveFilter use unsafe double-casts ("as unknown as") for FuseFilterWithValue/ FuseFilter and FilterChip; replace those casts by having the returned object literals use TypeScript's "satisfies" operator to validate shape against the proper interfaces (e.g., have the object in createFilter satisfy FuseFilterWithValue<ComfyNodeDefImpl, string> and the object in createActiveFilter satisfy FilterChip or have the nested filter satisfy FuseFilter<ComfyNodeDefImpl, string>), keep the vi.fn() implementations for matches and preserve return types so the compiler enforces shape instead of silencing checks with casts.
111-135: Prefer accessible selectors overdata-testidwhen possible.If the chips/cancel button have accessible text/role, use those selectors to better reflect user behavior (e.g., role + name). Keep
data-testidonly when no accessible alternative exists.Based on learnings: In test files, prefer selecting or asserting on accessible properties (text content, aria-label, role, accessible name) over data-testid attributes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchInput.test.ts` around lines 111 - 135, Tests in NodeSearchInput.test.ts use data-testid selectors for filter chips and the cancel button; update the three assertions that call wrapper.findAll('[data-testid="filter-chip"]') and wrapper.find('[data-testid="cancel-filter"]') to use accessible selectors instead (e.g., find/findAll by role + accessible name or by visible text) and only fall back to data-testid if the component has no accessible text/role; ensure the component exposes an accessible role/aria-label for the chips and the cancel button if needed, then change the tests that use createWrapper/createFilter/createActiveFilter to assert on wrapper.findAll(/* role or text selector for chip */) and wrapper.find(/* role or name selector for cancel button */) so tests reflect user-facing accessibility instead of data-testid.src/components/searchbox/v2/NodeSearchCategoryTreeNode.vue (1)
2-18: Replace raw<button>with the shared Button component.This keeps styling and behavior consistent with the design system used across
src/components.Based on learnings: In the ComfyUI_frontend Vue codebase, replace raw
<button>HTML elements with the shared Button component located atsrc/components/ui/button/Button.vue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchCategoryTreeNode.vue` around lines 2 - 18, Replace the raw <button> in NodeSearchCategoryTreeNode.vue with the shared Button component (Button from src/components/ui/button/Button.vue): import and register the Button component, render <Button> instead of <button> and pass through the same attributes and behavior — bind data-testid to `category-${node.key}`, set aria-current to (selectedCategory === node.key ? selectedCategory === node.key : undefined), apply the same inline style for paddingLeft (`{ paddingLeft: `${0.75 + depth * 1.25}rem` }`), compute the class using cn with CATEGORY_SELECTED_CLASS / CATEGORY_UNSELECTED_CLASS as before, and keep the `@click` handler to $emit('select', node.key); ensure node.label is rendered as the Button slot/children so visual/behavioral parity is preserved.src/components/node/NodePreviewCard.vue (1)
118-124: Consider using reactive binding instead of direct DOM manipulation.The current approach directly sets
style.heighton the DOM element. While functional, using a reactive ref bound to the template would be more idiomatic Vue:♻️ Alternative reactive approach
+const containerHeight = ref<string>('auto') + useResizeObserver(previewWrapperRef, (entries) => { const entry = entries[0] - if (entry && previewContainerRef.value) { + if (entry) { const scaledHeight = entry.contentRect.height * SCALE_FACTOR - previewContainerRef.value.style.height = `${scaledHeight + PREVIEW_CONTAINER_PADDING_PX}px` + containerHeight.value = `${scaledHeight + PREVIEW_CONTAINER_PADDING_PX}px` } })Then in template:
<div ref="previewContainerRef" class="overflow-hidden p-3" :style="{ height: containerHeight }">This keeps height changes within Vue's reactivity system.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/node/NodePreviewCard.vue` around lines 118 - 124, Replace direct DOM style mutation inside the useResizeObserver callback with a reactive ref for height: create a ref like containerHeight and compute/set containerHeight.value = `${scaledHeight + PREVIEW_CONTAINER_PADDING_PX}px` inside the callback (useResizeObserver, previewWrapperRef, SCALE_FACTOR, PREVIEW_CONTAINER_PADDING_PX are where to change), remove previewContainerRef.value.style.height assignment, and bind the ref in the template on the container element (previewContainerRef) via :style="{ height: containerHeight }" so Vue's reactivity drives the height updates.src/components/sidebar/tabs/nodeLibrary/EssentialNodesPanel.test.ts (2)
135-146: Consider reducing redundant await calls.The test uses multiple
await nextTick()andawait flushPromises()calls. While this works, a singleawait flushPromises()typically handles both microtasks and DOM updates.♻️ Simplify async waiting
it('should expand all folders by default when expandedKeys is empty', async () => { const wrapper = mountComponent(createMockRoot(), []) - await nextTick() await flushPromises() - await nextTick() const roots = wrapper.findAll('.collapsible-root')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/nodeLibrary/EssentialNodesPanel.test.ts` around lines 135 - 146, The test for default expansion in EssentialNodesPanel uses redundant awaits (multiple nextTick() and flushPromises()); simplify by replacing the sequence of await nextTick(); await flushPromises(); await nextTick() with a single await flushPromises() (or a single await nextTick() if preferred) after mounting via mountComponent(createMockRoot(), []), ensuring the DOM settles before querying '.collapsible-root'—update the test body in the 'should expand all folders by default when expandedKeys is empty' spec to call only one async wait (flushPromises) and then perform the existing expects.
36-43: Consider usingsatisfiesfor mock type validation.Per repository conventions, when creating mock objects in test helpers, prefer
satisfiesfor shape validation over type assertions.♻️ Optional improvement
function createMockNode( name: string ): RenderedTreeExplorerNode<ComfyNodeDefImpl> { return { key: `node-${name}`, label: name, icon: 'icon-[comfy--node]', type: 'node', totalLeaves: 1, data: { name, display_name: name - } as ComfyNodeDefImpl + } as unknown as ComfyNodeDefImpl } }Note: Since
ComfyNodeDefImplis a class with many required fields, a fullsatisfiespattern would require more fields. The current cast is acceptable for test purposes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/nodeLibrary/EssentialNodesPanel.test.ts` around lines 36 - 43, Replace the type assertion with a TS "satisfies" check for the mock node object to validate its shape without forcing a cast: locate the mock where `data: { name, display_name: name } as ComfyNodeDefImpl` in EssentialNodesPanel.test.ts and change the pattern to use `satisfies ComfyNodeDefImpl`; if the class requires extra fields for full satisfaction, either provide minimal stubbed fields on the mock or use a Partial-like shape that still uses `satisfies` for shape validation instead of `as`.src/composables/node/useNodeDragToCanvas.ts (1)
67-87: Consider using VueUse'suseEventListenerfor automatic cleanup.The manual listener management with
listenersSetupflag works, but VueUse'suseEventListenerprovides automatic cleanup on component unmount and reduces boilerplate.♻️ Suggested approach using VueUse
If the composable is used within component lifecycle,
useEventListenerfrom@vueuse/corewould simplify listener management:import { useEventListener } from '@vueuse/core' // Inside useNodeDragToCanvas: useEventListener(document, 'pointermove', updatePosition) useEventListener(document, 'pointerup', endDrag, true) useEventListener(document, 'keydown', handleKeydown)However, since this is a singleton pattern with explicit setup/cleanup functions, the current approach may be intentional for finer control. Based on learnings: "Leverage VueUse functions for performance-enhancing composables."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodeDragToCanvas.ts` around lines 67 - 87, Replace manual add/removeEventListener logic in setupGlobalListeners and cleanupGlobalListeners by using useEventListener from `@vueuse/core`: import useEventListener, call const stopPointerMove = useEventListener(document, 'pointermove', updatePosition), const stopPointerUp = useEventListener(document, 'pointerup', endDrag, true), const stopKeydown = useEventListener(document, 'keydown', handleKeydown) inside setupGlobalListeners (maintain listenersSetup guard), store the returned stop functions in module-scoped variables, and in cleanupGlobalListeners call those stop functions instead of removeEventListener (then keep the existing isDragging.value && dragMode.value === 'click' check and call cancelDrag() as before).src/components/searchbox/v2/NodeSearchContent.vue (1)
254-256: Consider simplifying watch dependency for filters prop.The watch uses
() => filtersas a dependency, which works but is unconventional for a prop. Sincefiltersis a prop (destructured from defineProps), you can reference it directly in the array.♻️ Suggested simplification
-watch([selectedCategory, searchQuery, () => filters], () => { +watch([selectedCategory, searchQuery, filters], () => { selectedIndex.value = 0 })Note: This requires verifying that the filters prop maintains referential stability. If the parent passes a new array reference on updates, both patterns work correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchContent.vue` around lines 254 - 256, The watch dependency list is using an unnecessary function wrapper (() => filters); update the watcher to reference the prop directly by replacing the dependency entry with filters so the array becomes [selectedCategory, searchQuery, filters]; keep the watcher body (setting selectedIndex.value = 0) unchanged and verify filters is the prop from defineProps to ensure referential behavior remains correct (symbols: selectedCategory, searchQuery, filters, selectedIndex).src/services/nodeOrganizationService.ts (2)
160-201: Consider extracting shared grouping logic between 'custom' and 'all' tabs.Lines 164-180 and 184-200 implement nearly identical grouping patterns, differing only in:
- The node filter (CustomNodes vs all)
- The default main_category value ('custom_extensions' vs 'basics')
This is acceptable for now but could be consolidated if the pattern emerges elsewhere.
♻️ Potential extraction
function groupByMainCategory( nodes: ComfyNodeDefImpl[], defaultCategory: string, pathExtractor: (n: ComfyNodeDefImpl) => string[] ): NodeSection[] { const grouped = new Map<string, ComfyNodeDefImpl[]>() for (const node of nodes) { const mainCategory = node.main_category ?? defaultCategory if (!grouped.has(mainCategory)) { grouped.set(mainCategory, []) } grouped.get(mainCategory)!.push(node) } return Array.from(grouped.entries()).map(([cat, categoryNodes]) => ({ title: upperCase(cat), tree: buildNodeDefTree(categoryNodes, { pathExtractor }) })) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/nodeOrganizationService.ts` around lines 160 - 201, The switch cases for 'custom' and 'all' in nodeOrganizationService.ts duplicate grouping logic; extract a helper (e.g., groupByMainCategory) that accepts nodes, a defaultCategory string ('custom_extensions' or 'basics'), and the pathExtractor callback, performs the Map grouping by node.main_category ?? defaultCategory, and returns the mapped NodeSection array using upperCase and buildNodeDefTree; then replace both case blocks to call this helper (filtering by NodeSourceType.CustomNodes for 'custom' before calling it) to remove the duplication.
135-144: Hardcoded folder order for essentials tab.The
folderOrderarray hardcodes the display order for essential node categories. This is acceptable for initial implementation but consider externalizing to configuration if the order needs to change frequently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/nodeOrganizationService.ts` around lines 135 - 144, The hardcoded folderOrder array in nodeOrganizationService.ts (symbol: folderOrder) fixes the display order for the essentials tab and should be externalized so it can be changed without code edits; move folderOrder into a configurable source (e.g., a config file, environment-driven settings, or a centralized constants/config module) and update the code that references folderOrder to read from that config (or provide a default fallback) so the order can be modified at runtime or via deployments.src/components/searchbox/v2/NodeSearchInput.vue (1)
16-24: Use the shared Button component for the cancel action.Swap the raw
<button>for the design-system Button component (icon-only variant) to keep styling/behavior consistent across the app.Based on learnings "replace raw HTML elements with the shared Button component located at src/components/ui/button/Button.vue."♻️ Proposed change
- <button - type="button" - data-testid="cancel-filter" - class="cursor-pointer border-none bg-transparent text-muted-foreground hover:text-base-foreground rounded-full aspect-square" - :aria-label="$t('g.remove')" - `@click`="emit('cancelFilter')" - > - <i class="pi pi-times text-xs" /> - </button> + <Button + variant="ghost" + size="icon" + data-testid="cancel-filter" + class="text-muted-foreground hover:text-base-foreground" + :aria-label="$t('g.remove')" + `@click`="emit('cancelFilter')" + > + <i class="pi pi-times text-xs" /> + </Button>+import Button from '@/components/ui/button/Button.vue'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchInput.vue` around lines 16 - 24, Replace the raw <button> in NodeSearchInput.vue with the shared Button component (src/components/ui/button/Button.vue): import/register the Button, use its icon-only variant and pass the same attributes (data-testid="cancel-filter", :aria-label="$t('g.remove')") and the click handler that emits 'cancelFilter' (emit('cancelFilter')). Ensure visual parity by mapping any class or appearance props the Button API expects (e.g., icon-only, rounded, size/text classes) so styling and behavior match the design system.src/components/searchbox/v2/NodeSearchContent.test.ts (1)
35-38: TypecreateWrapperprops withComponentPropsfor safety.This avoids implicit
anyand keeps the test helper aligned with the component’s real prop shape.Based on learnings "type the props parameter as Partial> ... cast as ComponentProps instead of using Record with a double-cast through unknown."♻️ Proposed change
-import type { VueWrapper } from '@vue/test-utils' +import type { VueWrapper } from '@vue/test-utils' +import type { ComponentProps } from 'vue-component-type-helpers'- async function createWrapper(props = {}) { + async function createWrapper( + props: Partial<ComponentProps<typeof NodeSearchContent>> = {} + ) { const wrapper = mount(NodeSearchContent, { - props: { filters: [], ...props }, + props: { filters: [], ...props } as ComponentProps<typeof NodeSearchContent>,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchContent.test.ts` around lines 35 - 38, Update the createWrapper helper to type its props parameter and cast the merged props for mount: change its signature to async function createWrapper(props: Partial<ComponentProps<typeof NodeSearchContent>> = {}) (import ComponentProps as a type), and when calling mount pass props: { filters: [], ...props } as ComponentProps<typeof NodeSearchContent>; this ensures the helper matches the actual component prop shape and avoids implicit any.src/components/searchbox/v2/NodeSearchCategorySidebar.vue (1)
98-120: Consider extractingmapNodeoutside the computed for clarity.The recursive
mapNodefunction inside the computed property works correctly, but extracting it as a standalone function would improve readability and make the computed's intent clearer.♻️ Optional refactor to extract mapNode
+function mapNode(node: TreeNode): CategoryNode { + const stripRootPrefix = (key: string) => key.replace(/^root\//, '') + const children = node.children + ?.filter((child): child is TreeNode => !child.leaf) + .map(mapNode) + return { + key: stripRootPrefix(node.key as string), + label: node.label, + ...(children?.length ? { children } : {}) + } +} + const categoryTree = computed<CategoryNode[]>(() => { const tree = nodeOrganizationService.organizeNodes( nodeDefStore.visibleNodeDefs, { groupBy: 'category' } ) - - const stripRootPrefix = (key: string) => key.replace(/^root\//, '') - - function mapNode(node: TreeNode): CategoryNode { - const children = node.children - ?.filter((child): child is TreeNode => !child.leaf) - .map(mapNode) - return { - key: stripRootPrefix(node.key as string), - label: node.label, - ...(children?.length ? { children } : {}) - } - } return (tree.children ?? []) .filter((node): node is TreeNode => !node.leaf) .map(mapNode) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchCategorySidebar.vue` around lines 98 - 120, Extract the inline recursive mapper out of the computed by moving the mapNode function (and helper stripRootPrefix) to a sibling/top-level function so categoryTree only builds the tree and maps root-level nodes; specifically, create a standalone function mapNode(node: TreeNode, stripRootPrefix: (s: string) => string) or keep stripRootPrefix as a small exported helper, then in computed categoryTree call nodeOrganizationService.organizeNodes(nodeDefStore.visibleNodeDefs, { groupBy: 'category' }) and map the resulting (tree.children ?? []).filter(...).map(mapNode) to produce CategoryNode[] — ensure the moved mapNode still performs the same recursion, filters leaf children, and returns { key: stripRootPrefix(node.key as string), label: node.label, ...(children?.length ? { children } : {}) }.
🤖 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/common/TreeExplorerV2.vue`:
- Around line 4-6: The current binding passes a defensive copy to TreeRoot
(":expanded='[...expandedKeys]'") so TreeRoot cannot emit "update:expanded" to
update the original ref; change the binding to use v-model:expanded and pass the
expandedKeys ref directly (i.e., replace the spread copy with
v-model:expanded="expandedKeys") so TreeRoot and the parent stay in sync and
update:expanded will mutate the same reactive ref.
In `@src/components/searchbox/v2/NodeSearchListItem.vue`:
- Around line 127-134: The component NodeSearchListItem.vue currently defines a
local scoped style for the .highlight selector; move this styling into Tailwind
by creating a shared utility class (e.g., .highlight or .hl) in the global CSS
where you can `@apply` the equivalent Tailwind utilities (background tint,
font-semibold, rounded-md, px-0.5, negative margins) and replace the local
<style> block with that class usage in the template so the SFC has no <style>
block and follows the Tailwind 4 guideline.
- Around line 10-16: The markup returned by highlightQuery
(packages/shared-frontend-utils/src/formatUtil.ts) is being bound with v-html in
NodeSearchListItem.vue (the two v-html uses for nodeDef.display_name and
nodeDef.name) and is not sanitized; update the code to sanitize or escape HTML
before insertion: either (A) import and run DOMPurify.sanitize(...) on the
result of highlightQuery in NodeSearchListItem.vue before passing it to v-html,
or (B) change highlightQuery to first HTML-escape its inputs (text and query)
and then build the highlighted span markup so the returned string is safe;
reference the highlightQuery function and the two v-html bindings when applying
the fix.
In `@src/components/sidebar/tabs/nodeLibrary/EssentialNodeCard.vue`:
- Around line 23-26: The Teleport preview ref callback is assigning to the ref
variable itself (previewRef) instead of its .value, which throws at mount;
update the ref usage so the template assigns to previewRef.value (or use a
normal template ref via ref="previewRef") and ensure previewRef is created as a
Vue ref (e.g., previewRef = ref<HTMLElement | null>(null)) in the setup; change
the :ref callback in the Teleport block (currently "(el) => (previewRef = el as
HTMLElement)") to assign to previewRef.value or switch to the simpler template
ref approach to avoid runtime errors and ensure nodePreviewStyle still uses the
same ref.
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue`:
- Around line 236-237: The ref searchBoxRef is currently untyped (ref())
producing Ref<any>; change it to a concrete DOM/input ref such as const
searchBoxRef = ref<HTMLInputElement | null>(null) (or the appropriate element
type that exposes focus) so TypeScript knows the API (focus) and avoids implicit
any; update any usages to handle null (e.g., searchBoxRef.value?.focus()) if
needed.
---
Nitpick comments:
In `@src/components/node/NodePreviewCard.vue`:
- Around line 118-124: Replace direct DOM style mutation inside the
useResizeObserver callback with a reactive ref for height: create a ref like
containerHeight and compute/set containerHeight.value = `${scaledHeight +
PREVIEW_CONTAINER_PADDING_PX}px` inside the callback (useResizeObserver,
previewWrapperRef, SCALE_FACTOR, PREVIEW_CONTAINER_PADDING_PX are where to
change), remove previewContainerRef.value.style.height assignment, and bind the
ref in the template on the container element (previewContainerRef) via :style="{
height: containerHeight }" so Vue's reactivity drives the height updates.
In `@src/components/searchbox/v2/NodeSearchCategorySidebar.vue`:
- Around line 98-120: Extract the inline recursive mapper out of the computed by
moving the mapNode function (and helper stripRootPrefix) to a sibling/top-level
function so categoryTree only builds the tree and maps root-level nodes;
specifically, create a standalone function mapNode(node: TreeNode,
stripRootPrefix: (s: string) => string) or keep stripRootPrefix as a small
exported helper, then in computed categoryTree call
nodeOrganizationService.organizeNodes(nodeDefStore.visibleNodeDefs, { groupBy:
'category' }) and map the resulting (tree.children ??
[]).filter(...).map(mapNode) to produce CategoryNode[] — ensure the moved
mapNode still performs the same recursion, filters leaf children, and returns {
key: stripRootPrefix(node.key as string), label: node.label,
...(children?.length ? { children } : {}) }.
In `@src/components/searchbox/v2/NodeSearchCategoryTreeNode.vue`:
- Around line 2-18: Replace the raw <button> in NodeSearchCategoryTreeNode.vue
with the shared Button component (Button from
src/components/ui/button/Button.vue): import and register the Button component,
render <Button> instead of <button> and pass through the same attributes and
behavior — bind data-testid to `category-${node.key}`, set aria-current to
(selectedCategory === node.key ? selectedCategory === node.key : undefined),
apply the same inline style for paddingLeft (`{ paddingLeft: `${0.75 + depth *
1.25}rem` }`), compute the class using cn with CATEGORY_SELECTED_CLASS /
CATEGORY_UNSELECTED_CLASS as before, and keep the `@click` handler to
$emit('select', node.key); ensure node.label is rendered as the Button
slot/children so visual/behavioral parity is preserved.
In `@src/components/searchbox/v2/NodeSearchContent.test.ts`:
- Around line 35-38: Update the createWrapper helper to type its props parameter
and cast the merged props for mount: change its signature to async function
createWrapper(props: Partial<ComponentProps<typeof NodeSearchContent>> = {})
(import ComponentProps as a type), and when calling mount pass props: { filters:
[], ...props } as ComponentProps<typeof NodeSearchContent>; this ensures the
helper matches the actual component prop shape and avoids implicit any.
In `@src/components/searchbox/v2/NodeSearchContent.vue`:
- Around line 254-256: The watch dependency list is using an unnecessary
function wrapper (() => filters); update the watcher to reference the prop
directly by replacing the dependency entry with filters so the array becomes
[selectedCategory, searchQuery, filters]; keep the watcher body (setting
selectedIndex.value = 0) unchanged and verify filters is the prop from
defineProps to ensure referential behavior remains correct (symbols:
selectedCategory, searchQuery, filters, selectedIndex).
In `@src/components/searchbox/v2/NodeSearchInput.test.ts`:
- Around line 56-73: The test helper createWrapper should type its props
parameter as Partial<ComponentProps<typeof NodeSearchInput>> and when passing
props to mount cast the merged props to ComponentProps<typeof NodeSearchInput>;
update the function signature and the props spread in the mount call
(referencing createWrapper and NodeSearchInput) so TypeScript enforces correct
prop shapes in tests.
- Around line 26-47: The helpers createFilter and createActiveFilter use unsafe
double-casts ("as unknown as") for FuseFilterWithValue/ FuseFilter and
FilterChip; replace those casts by having the returned object literals use
TypeScript's "satisfies" operator to validate shape against the proper
interfaces (e.g., have the object in createFilter satisfy
FuseFilterWithValue<ComfyNodeDefImpl, string> and the object in
createActiveFilter satisfy FilterChip or have the nested filter satisfy
FuseFilter<ComfyNodeDefImpl, string>), keep the vi.fn() implementations for
matches and preserve return types so the compiler enforces shape instead of
silencing checks with casts.
- Around line 111-135: Tests in NodeSearchInput.test.ts use data-testid
selectors for filter chips and the cancel button; update the three assertions
that call wrapper.findAll('[data-testid="filter-chip"]') and
wrapper.find('[data-testid="cancel-filter"]') to use accessible selectors
instead (e.g., find/findAll by role + accessible name or by visible text) and
only fall back to data-testid if the component has no accessible text/role;
ensure the component exposes an accessible role/aria-label for the chips and the
cancel button if needed, then change the tests that use
createWrapper/createFilter/createActiveFilter to assert on wrapper.findAll(/*
role or text selector for chip */) and wrapper.find(/* role or name selector for
cancel button */) so tests reflect user-facing accessibility instead of
data-testid.
In `@src/components/searchbox/v2/NodeSearchInput.vue`:
- Around line 16-24: Replace the raw <button> in NodeSearchInput.vue with the
shared Button component (src/components/ui/button/Button.vue): import/register
the Button, use its icon-only variant and pass the same attributes
(data-testid="cancel-filter", :aria-label="$t('g.remove')") and the click
handler that emits 'cancelFilter' (emit('cancelFilter')). Ensure visual parity
by mapping any class or appearance props the Button API expects (e.g.,
icon-only, rounded, size/text classes) so styling and behavior match the design
system.
In `@src/components/sidebar/tabs/nodeLibrary/EssentialNodesPanel.test.ts`:
- Around line 135-146: The test for default expansion in EssentialNodesPanel
uses redundant awaits (multiple nextTick() and flushPromises()); simplify by
replacing the sequence of await nextTick(); await flushPromises(); await
nextTick() with a single await flushPromises() (or a single await nextTick() if
preferred) after mounting via mountComponent(createMockRoot(), []), ensuring the
DOM settles before querying '.collapsible-root'—update the test body in the
'should expand all folders by default when expandedKeys is empty' spec to call
only one async wait (flushPromises) and then perform the existing expects.
- Around line 36-43: Replace the type assertion with a TS "satisfies" check for
the mock node object to validate its shape without forcing a cast: locate the
mock where `data: { name, display_name: name } as ComfyNodeDefImpl` in
EssentialNodesPanel.test.ts and change the pattern to use `satisfies
ComfyNodeDefImpl`; if the class requires extra fields for full satisfaction,
either provide minimal stubbed fields on the mock or use a Partial-like shape
that still uses `satisfies` for shape validation instead of `as`.
In `@src/composables/node/useNodeDragToCanvas.ts`:
- Around line 67-87: Replace manual add/removeEventListener logic in
setupGlobalListeners and cleanupGlobalListeners by using useEventListener from
`@vueuse/core`: import useEventListener, call const stopPointerMove =
useEventListener(document, 'pointermove', updatePosition), const stopPointerUp =
useEventListener(document, 'pointerup', endDrag, true), const stopKeydown =
useEventListener(document, 'keydown', handleKeydown) inside setupGlobalListeners
(maintain listenersSetup guard), store the returned stop functions in
module-scoped variables, and in cleanupGlobalListeners call those stop functions
instead of removeEventListener (then keep the existing isDragging.value &&
dragMode.value === 'click' check and call cancelDrag() as before).
In `@src/services/nodeOrganizationService.ts`:
- Around line 160-201: The switch cases for 'custom' and 'all' in
nodeOrganizationService.ts duplicate grouping logic; extract a helper (e.g.,
groupByMainCategory) that accepts nodes, a defaultCategory string
('custom_extensions' or 'basics'), and the pathExtractor callback, performs the
Map grouping by node.main_category ?? defaultCategory, and returns the mapped
NodeSection array using upperCase and buildNodeDefTree; then replace both case
blocks to call this helper (filtering by NodeSourceType.CustomNodes for 'custom'
before calling it) to remove the duplication.
- Around line 135-144: The hardcoded folderOrder array in
nodeOrganizationService.ts (symbol: folderOrder) fixes the display order for the
essentials tab and should be externalized so it can be changed without code
edits; move folderOrder into a configurable source (e.g., a config file,
environment-driven settings, or a centralized constants/config module) and
update the code that references folderOrder to read from that config (or provide
a default fallback) so the order can be modified at runtime or via deployments.
Comment on lines
+23
to
+26
| <Teleport v-if="showPreview" to="body"> | ||
| <div | ||
| :ref="(el) => (previewRef = el as HTMLElement)" | ||
| :style="nodePreviewStyle" |
Contributor
There was a problem hiding this comment.
Fix preview ref assignment to avoid runtime errors.
The ref callback assigns to the previewRef constant instead of previewRef.value, which will throw and leave the preview ref unset when the Teleport mounts. Use a normal template ref or assign to .value.
🐛 Proposed fix
- :ref="(el) => (previewRef = el as HTMLElement)"
+ ref="previewRef"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/sidebar/tabs/nodeLibrary/EssentialNodeCard.vue` around lines
23 - 26, The Teleport preview ref callback is assigning to the ref variable
itself (previewRef) instead of its .value, which throws at mount; update the ref
usage so the template assigns to previewRef.value (or use a normal template ref
via ref="previewRef") and ensure previewRef is created as a Vue ref (e.g.,
previewRef = ref<HTMLElement | null>(null)) in the setup; change the :ref
callback in the Teleport block (currently "(el) => (previewRef = el as
HTMLElement)") to assign to previewRef.value or switch to the simpler template
ref approach to avoid runtime errors and ensure nodePreviewStyle still uses the
same ref.
Collaborator
There was a problem hiding this comment.
It's not part of this change.
Contributor
There was a problem hiding this comment.
@Yourz, you're absolutely right — that pattern was already in the code before your changes. I shouldn't have flagged it as part of this PR since you're only updating the composable usage and removing the panel context dependency. My apologies for the confusion!
🧠 Learnings used
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: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize Vue 3's Teleport component when needed
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-27T20:51:47.936Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid `ts-expect-error` - fix the underlying type issue instead
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-23T21:34:45.361Z
Learning: Applies to src/**/*.ts : Minimize addition of new refs or other state; if possible to accomplish with just a prop, don't add a `ref`; if possible to use ref/prop directly, don't add a `computed`; if possible to use `computed` for derived values, don't use a `watch`
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-23T21:34:45.361Z
Learning: Applies to src/**/*.ts : Avoid using a `ref` and a `watch` if a `computed` would work instead
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:12.361Z
Learning: In Vue single-file components, allow inline Tailwind CSS class strings for static classes and avoid extracting them into computed properties solely for readability. Prefer keeping static class names inline for simplicity and performance. For dynamic or conditional classes, use Vue bindings (e.g., :class) to compose classes.
Applies to all Vue files in the repository (e.g., src/**/*.vue) where Tailwind utilities are used for static styling.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.vue:17-17
Timestamp: 2025-12-16T22:26:49.463Z
Learning: In Vue 3.5+ with <script setup>, when using defineProps<Props>() with partial destructuring (e.g., const { as = 'button', class: customClass = '' } = defineProps<Props>() ), props that are not destructured (e.g., variant, size) stay accessible by name in the template scope. This pattern is valid: you can destructure only a subset of props for convenience while referencing the remaining props directly in template expressions. Apply this guideline to Vue components across the codebase (all .vue files).
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7598
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:131-131
Timestamp: 2025-12-18T02:07:38.870Z
Learning: Tailwind CSS v4 safe utilities (e.g., items-center-safe, justify-*-safe, place-*-safe) are allowed in Vue components under src/ and in story files. Do not flag these specific safe variants as invalid when reviewing code in src/**/*.vue or related stories.
Learnt from: henrikvilhelmberglund
Repo: Comfy-Org/ComfyUI_frontend PR: 7617
File: src/components/actionbar/ComfyActionbar.vue:301-308
Timestamp: 2025-12-18T16:03:02.066Z
Learning: In the ComfyUI frontend queue system, useQueuePendingTaskCountStore().count indicates the number of tasks in the queue, where count = 1 means a single active/running task and count > 1 means there are pending tasks in addition to the active task. Therefore, in src/components/actionbar/ComfyActionbar.vue, enable the 'Clear Pending Tasks' button only when count > 1 to avoid clearing the currently running task. The active task should be canceled using the 'Cancel current run' button instead. This rule should be enforced via a conditional check on the queue count, with appropriate disabled/aria-disabled states for accessibility, and tests should verify behavior for count = 1 and count > 1.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7603
File: src/components/queue/QueueOverlayHeader.vue:49-59
Timestamp: 2025-12-18T21:15:46.862Z
Learning: In the ComfyUI_frontend repository, for Vue components, do not add aria-label to buttons that have visible text content (e.g., buttons containing <span> text). The visible text provides the accessible name. Use aria-label only for elements without visible labels (e.g., icon-only buttons). If a button has no visible label, provide a clear aria-label or associate with an aria-labelledby describing its action.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/components/graph/selectionToolbox/ColorPickerButton.vue:15-18
Timestamp: 2025-12-21T01:06:02.786Z
Learning: In Comfy-Org/ComfyUI_frontend, in Vue component files, when a filled icon is required (e.g., 'pi pi-circle-fill'), you may mix PrimeIcons with Lucide icons since Lucide lacks filled variants. This mixed usage is acceptable when one icon library does not provide an equivalent filled icon. Apply consistently across Vue components in the src directory where icons are used, and document the rationale when a mixed approach is chosen.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/platform/cloud/subscription/components/PricingTable.vue:185-201
Timestamp: 2025-12-22T21:36:08.369Z
Learning: In Vue components, avoid creating single-use variants for common UI components (e.g., Button and other shared components). Aim for reusable variants that cover multiple use cases. It’s acceptable to temporarily mix variant props with inline Tailwind classes when a styling need is unique to one place, but plan and consolidate into shared, reusable variants as patterns emerge across the codebase.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7893
File: src/components/button/IconGroup.vue:5-6
Timestamp: 2026-01-08T02:26:18.357Z
Learning: In components that use the cn utility from '@/utils/tailwindUtil' with tailwind-merge, rely on the behavior that conflicting Tailwind classes are resolved by keeping the last one. For example, cn('base-classes bg-default', propClass) will have any conflicting background class from propClass override bg-default. This additive pattern is intentional and aligns with the shadcn-ui convention; ensure you document or review expectations accordingly in Vue components.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8195
File: src/platform/assets/components/MediaAssetFilterBar.vue:16-16
Timestamp: 2026-01-21T01:28:27.626Z
Learning: In Vue templates (Vue 3.4+ with the build step), when binding to data or props that are camelCase (e.g., mediaTypeFilters), you can use kebab-case in the template bindings (e.g., :media-type-filters). This is acceptable and will resolve to the corresponding camelCase variable. Do not require CamelCase in template bindings.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8090
File: src/platform/assets/components/modelInfo/ModelInfoField.vue:8-11
Timestamp: 2026-01-22T02:28:58.105Z
Learning: In Vue 3 script setup, props defined with defineProps are automatically available by name in the template without destructuring. Destructuring the result of defineProps inside script can break reactivity; prefer accessing props by name in the template. If you need to use props in the script, reference them via the defined props object rather than destructuring, or use toRefs when you intend to destructure while preserving reactivity.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8497
File: src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue:223-236
Timestamp: 2026-02-01T21:10:29.567Z
Learning: In Vue single-file components, do not review or require edits to comments. Favor self-documenting code through clear naming, strong types, and explicit APIs. If a comment is misleading or outdated, consider removing it, but avoid suggesting adding or fixing comments. This guideline aligns with preferring code clarity over comment maintenance across all .vue files.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8592
File: src/components/topbar/WorkflowExecutionIndicator.vue:28-28
Timestamp: 2026-02-03T21:35:40.889Z
Learning: In Vue single-file components where the i18n t function is only used within the template, prefer using the built-in $t in the template instead of importing useI18n and destructuring t in the script. This avoids unnecessary imports when t is not used in the script. If you need i18n in the script (Composition API), only then use useI18n and access t from its returned object. Ensure this pattern applies to all Vue components with template-only i18n usage.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 8548
File: src/components/common/TreeExplorerV2.vue:2-89
Timestamp: 2026-02-07T14:46:21.179Z
Learning: In Vue components (e.g., TreeExplorerV2.vue and similar Recka UI components) avoid wrapping ContextMenuRoot with a dynamic <component :is="..."> and rendering alternative branches. Use v-if / v-else to render separate ContextMenuRoot and non-ContextMenuRoot branches to ensure correct behavior with Reka UI, even if it duplicates some tree structure. This guideline applies to Vue components under src/components and can be checked by ensuring there is a dedicated render path for the ContextMenuRoot without dynamic component switching.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 8548
File: src/components/common/TreeExplorerV2Node.vue:9-47
Timestamp: 2026-02-07T14:47:06.751Z
Learning: In all Vue files that render Reka UI context menu components (ContextMenuRoot, ContextMenuTrigger, etc.), avoid dynamic component wrapping with <component :is="...">. Use conditional rendering with v-if / v-else to render separate branches, even if it results in some template duplication. This improves readability and maintainability across files that use these components.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 8548
File: src/components/common/TreeExplorerV2.vue:3-5
Timestamp: 2026-02-08T16:04:58.463Z
Learning: In TreeExplorerV2.vue and similar Re- ka UI Tree components, avoid defensive copies in bindings like :expanded="[...expandedKeys]". Use v-model:expanded="expandedKeys" instead. TreeRoot follows Vue's controlled component pattern and emits update:expanded with the new array; it does not mutate the passed array. Defensive copies break reactivity and can desynchronize state. Ensure components rely on update:expanded and do not mutate input arrays.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8753
File: src/renderer/extensions/vueNodes/widgets/components/WidgetDOM.vue:17-28
Timestamp: 2026-02-09T03:24:47.113Z
Learning: When destructuring reactive properties from Pinia stores in Vue components, use storeToRefs() to preserve reactivity. Example: const store = useCanvasStore(); const { canvas } = storeToRefs(store); access as canvas.value (e.g., canvas.value). Ensure you import storeToRefs from 'pinia' and use it wherever you destructure store properties in the setup function.
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 8761
File: src/components/dialog/content/MissingModelsWarning.vue:21-27
Timestamp: 2026-02-10T00:58:07.904Z
Learning: In the ComfyUI_frontend Vue codebase, replace raw <button> HTML elements with the shared Button component located at src/components/ui/button/Button.vue. Import and use it with appropriate variants (e.g., variant="link") to align with the design system. Apply this pattern across Vue components under src/components, ensuring consistent styling and behavior instead of ad-hoc button markup.
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI_frontend PR: 8775
File: src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue:62-68
Timestamp: 2026-02-10T17:59:25.893Z
Learning: In Vue 3 single-file components, code defined outside the script setup scope cannot access variables or helpers defined inside setup. If a default function (e.g., defineModel) is hoisted outside script setup, it will not be able to call setup helpers or reference setup-scoped variables. Place such logic inside setup or expose necessary values via returns/defineExpose to be accessible.
55f38b8 to
b62e7c6
Compare
🎭 Playwright: ✅ 549 passed, 0 failed · 4 flaky📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
📦 Bundle: 4.46 MB gzip 🔴 +1.46 kBDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.03 MB (baseline 1.03 MB) • 🔴 +702 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 10 added / 10 removed Panels & Settings — 435 kB (baseline 435 kB) • 🟢 -1 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Status: 11 added / 11 removed Data & Services — 2.56 MB (baseline 2.56 MB) • 🔴 +8.34 kBStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 13 added / 13 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • 🔴 +2.21 kBExternal libraries and shared vendor chunks
Status: 2 added / 2 removed Other — 7.87 MB (baseline 7.87 MB) • 🔴 +558 BBundles that do not match a named category
Status: 71 added / 71 removed |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/searchbox/v2/NodeSearchContent.vue (1)
195-240: Consider consistency in filter application forrecentscase.The
recentscase (lines 210-211) returnssearchResults.valuedirectly, bypassing thematchesFilterscheck that other category cases apply at line 239. This means applied filters won't affect the recents list, which may be intentional given the placeholder nature, but could cause unexpected behavior when filters are active.If filters should apply to recents:
♻️ Suggested fix
case 'recents': - return searchResults.value + results = searchResults.value + break🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchContent.vue` around lines 195 - 240, The recents branch in the computed displayedResults currently returns searchResults.value directly and skips the shared filter application; to ensure active filters affect recents, change the 'recents' case in the displayedResults switch (using effectiveCategory.value / searchResults.value) to assign results = searchResults.value (instead of returning) so the final line "return filters.length > 0 ? results.filter(matchesFilters) : results" will apply matchesFilters, or alternatively add an explicit filter pass like results = searchResults.value.filter(matchesFilters) if you want to apply filters immediately; update the 'recents' case in the displayedResults computed accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/searchbox/v2/NodeSearchContent.vue`:
- Around line 195-240: The recents branch in the computed displayedResults
currently returns searchResults.value directly and skips the shared filter
application; to ensure active filters affect recents, change the 'recents' case
in the displayedResults switch (using effectiveCategory.value /
searchResults.value) to assign results = searchResults.value (instead of
returning) so the final line "return filters.length > 0 ?
results.filter(matchesFilters) : results" will apply matchesFilters, or
alternatively add an explicit filter pass like results =
searchResults.value.filter(matchesFilters) if you want to apply filters
immediately; update the 'recents' case in the displayedResults computed
accordingly.
b62e7c6 to
3953b14
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vue (1)
22-24: Avoid destructuringdefinePropsto keepsortOrderreactive.
Per repo guidance, destructuring can break prop reactivity. Consider keeping a props object and derivingsortOrdervia a computed default so header visibility reacts to parent updates.♻️ Suggested refactor
-const { fillNodeInfo, sortOrder = 'original' } = defineProps<{ +const props = defineProps<{ sections: NodeLibrarySection<ComfyNodeDefImpl>[] fillNodeInfo: (node: TreeNode) => RenderedTreeExplorerNode<ComfyNodeDefImpl> sortOrder?: string }>() + +const sortOrder = computed(() => props.sortOrder ?? 'original')-const favoritesRoot = computed(() => - fillNodeInfo(nodeBookmarkStore.bookmarkedRoot) -) +const favoritesRoot = computed(() => + props.fillNodeInfo(nodeBookmarkStore.bookmarkedRoot) +)Based on learnings: “In Vue 3 script setup, props defined with defineProps are automatically available by name in the template without destructuring. Destructuring the result of defineProps inside script can break reactivity; prefer accessing props by name in the template. If you need to use props in the script, reference them via the defined props object rather than destructuring, or use toRefs when you intend to destructure while preserving reactivity.”
Also applies to: 51-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vue` around lines 22 - 24, The template's reactivity is broken by destructuring defineProps; stop destructuring and keep the props object returned from defineProps (e.g., const props = defineProps(...)), then expose a reactive sortOrder via a computed default (e.g., const sortOrder = computed(() => props.sortOrder ?? 'defaultValue') or use toRefs(props) if you must destructure) so the v-if="section.title && sortOrder !== 'alphabetical'" header and the other occurrences (lines ~51-55) react to parent updates; update references in the script to use props.sortOrder or the computed sortOrder instead of a destructured variable.
🤖 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 45-50: Replace the raw <button> in NodeLibrarySidebarTabV2.vue
with the shared Button component (import the Button component from
src/components/ui/button/Button.vue into the NodeLibrarySidebarTabV2 component),
pass the aria-label ($t('sideToolbar.nodeLibraryTab.filter')) and existing
classes/attributes through the Button’s props/attrs, and keep the icon (<i
class="icon-[lucide--list-filter] size-4" />) as the Button’s child so the
visual/behavioral contract matches the rest of the app.
---
Nitpick comments:
In `@src/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vue`:
- Around line 22-24: The template's reactivity is broken by destructuring
defineProps; stop destructuring and keep the props object returned from
defineProps (e.g., const props = defineProps(...)), then expose a reactive
sortOrder via a computed default (e.g., const sortOrder = computed(() =>
props.sortOrder ?? 'defaultValue') or use toRefs(props) if you must destructure)
so the v-if="section.title && sortOrder !== 'alphabetical'" header and the other
occurrences (lines ~51-55) react to parent updates; update references in the
script to use props.sortOrder or the computed sortOrder instead of a
destructured variable.
Comment on lines
+45
to
+50
| <button | ||
| :aria-label="$t('sideToolbar.nodeLibraryTab.filter')" | ||
| class="flex size-10 shrink-0 cursor-pointer items-center justify-center rounded-lg bg-comfy-input hover:bg-comfy-input-hover border-none" | ||
| > | ||
| <i class="icon-[lucide--list-filter] size-4" /> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Use the shared Button component for the new filter trigger.
Raw <button> usage in src/components should be replaced with the shared Button component for consistent styling/behavior.
♻️ Suggested update
+import Button from '@/components/ui/button/Button.vue'- <button
- :aria-label="$t('sideToolbar.nodeLibraryTab.filter')"
- class="flex size-10 shrink-0 cursor-pointer items-center justify-center rounded-lg bg-comfy-input hover:bg-comfy-input-hover border-none"
- >
+ <Button
+ :aria-label="$t('sideToolbar.nodeLibraryTab.filter')"
+ class="flex size-10 shrink-0 cursor-pointer items-center justify-center rounded-lg bg-comfy-input hover:bg-comfy-input-hover border-none"
+ >
<i class="icon-[lucide--list-filter] size-4" />
- </button>
+ </Button>Based on learnings: In the ComfyUI_frontend Vue codebase, replace raw HTML elements with the shared Button component located at src/components/ui/button/Button.vue.
📝 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
| <button | |
| :aria-label="$t('sideToolbar.nodeLibraryTab.filter')" | |
| class="flex size-10 shrink-0 cursor-pointer items-center justify-center rounded-lg bg-comfy-input hover:bg-comfy-input-hover border-none" | |
| > | |
| <i class="icon-[lucide--list-filter] size-4" /> | |
| </button> | |
| import Button from '@/components/ui/button/Button.vue' |
Suggested change
| <button | |
| :aria-label="$t('sideToolbar.nodeLibraryTab.filter')" | |
| class="flex size-10 shrink-0 cursor-pointer items-center justify-center rounded-lg bg-comfy-input hover:bg-comfy-input-hover border-none" | |
| > | |
| <i class="icon-[lucide--list-filter] size-4" /> | |
| </button> | |
| <Button | |
| :aria-label="$t('sideToolbar.nodeLibraryTab.filter')" | |
| class="flex size-10 shrink-0 cursor-pointer items-center justify-center rounded-lg bg-comfy-input hover:bg-comfy-input-hover border-none" | |
| > | |
| <i class="icon-[lucide--list-filter] size-4" /> | |
| </Button> |
🤖 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 45 -
50, Replace the raw <button> in NodeLibrarySidebarTabV2.vue with the shared
Button component (import the Button component from
src/components/ui/button/Button.vue into the NodeLibrarySidebarTabV2 component),
pass the aria-label ($t('sideToolbar.nodeLibraryTab.filter')) and existing
classes/attributes through the Button’s props/attrs, and keep the icon (<i
class="icon-[lucide--list-filter] size-4" />) as the Button’s child so the
visual/behavioral contract matches the rest of the app.
Collaborator
There was a problem hiding this comment.
The raw elements are deliberate here — they're used inside with specific styling that doesn't map to any existing Button variant. Wrapping them would either break the visual design or add unnecessary complexity. Both buttons in this file follow the same pattern consistently.
Contributor
There was a problem hiding this comment.
@Yourz, that makes sense — the as-child pattern in Reka UI is designed for composition, allowing you to pass behavior to custom trigger elements while retaining accessibility. Creating a Button variant for this specific styling would introduce a single-use variant without broader reuse, and the raw <button> approach keeps the styling inline where it's contextually needed.
I'll note this pattern for future reviews. Thanks for the clarification!
✏️ Learnings added
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 9085
File: src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue:45-50
Timestamp: 2026-02-27T09:39:47.145Z
Learning: In Vue components using Reka UI primitives (DropdownMenuTrigger, SelectTrigger, etc.) with `as-child`, raw `<button>` elements are acceptable when the styling is context-specific and doesn't map to existing Button component variants. The `as-child` pattern allows composition while maintaining accessibility. Avoid creating single-use Button variants for these cases; inline styling with raw buttons is preferred for clarity and to avoid unnecessary abstraction.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 8761
File: src/components/dialog/content/MissingModelsWarning.vue:21-27
Timestamp: 2026-02-10T00:58:07.904Z
Learning: In the ComfyUI_frontend Vue codebase, replace raw <button> HTML elements with the shared Button component located at src/components/ui/button/Button.vue. Import and use it with appropriate variants (e.g., variant="link") to align with the design system. Apply this pattern across Vue components under src/components, ensuring consistent styling and behavior instead of ad-hoc button markup.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/platform/cloud/subscription/components/PricingTable.vue:185-201
Timestamp: 2025-12-22T21:36:08.369Z
Learning: In Vue components, avoid creating single-use variants for common UI components (e.g., Button and other shared components). Aim for reusable variants that cover multiple use cases. It’s acceptable to temporarily mix variant props with inline Tailwind classes when a styling need is unique to one place, but plan and consolidate into shared, reusable variants as patterns emerge across the codebase.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7603
File: src/components/queue/QueueOverlayHeader.vue:49-59
Timestamp: 2025-12-18T21:15:46.862Z
Learning: In the ComfyUI_frontend repository, for Vue components, do not add aria-label to buttons that have visible text content (e.g., buttons containing <span> text). The visible text provides the accessible name. Use aria-label only for elements without visible labels (e.g., icon-only buttons). If a button has no visible label, provide a clear aria-label or associate with an aria-labelledby describing its action.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 8548
File: src/components/common/TreeExplorerV2Node.vue:9-47
Timestamp: 2026-02-07T14:47:06.751Z
Learning: In all Vue files that render Reka UI context menu components (ContextMenuRoot, ContextMenuTrigger, etc.), avoid dynamic component wrapping with <component :is="...">. Use conditional rendering with v-if / v-else to render separate branches, even if it results in some template duplication. This improves readability and maintainability across files that use these components.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/ui/AGENTS.md:0-0
Timestamp: 2026-01-22T17:18:23.734Z
Learning: Applies to src/components/ui/**/*.vue : Use Iconify icons: <i class="icon-[lucide--check]" /> instead of other icon libraries
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 8548
File: src/components/common/TreeExplorerV2.vue:2-89
Timestamp: 2026-02-07T14:46:21.179Z
Learning: In Vue components (e.g., TreeExplorerV2.vue and similar Recka UI components) avoid wrapping ContextMenuRoot with a dynamic <component :is="..."> and rendering alternative branches. Use v-if / v-else to render separate ContextMenuRoot and non-ContextMenuRoot branches to ensure correct behavior with Reka UI, even if it duplicates some tree structure. This guideline applies to Vue components under src/components and can be checked by ensuring there is a dedicated render path for the ContextMenuRoot without dynamic component switching.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/ui/AGENTS.md:0-0
Timestamp: 2026-01-22T17:18:23.734Z
Learning: Applies to src/components/ui/**/*.vue : Use design tokens: bg-secondary-background, text-muted-foreground, border-border-default
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7459
File: src/lib/litegraph/src/canvas/LinkConnector.ts:875-881
Timestamp: 2026-02-23T19:49:55.224Z
Learning: ComfyUI_frontend TypeScript review preference: Prefer user-defined type guards (e.g., function isMovingInputLink(x): x is MovingInputLink) over inline type assertions when narrowing union types like RenderLinkUnion in src/lib/litegraph/**/*.ts. Apply especially in LinkConnector and related canvas classes to avoid casts and improve reuse.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: tests-ui/tests/platform/cloud/subscription/components/SubscriptionPanel.test.ts:189-194
Timestamp: 2025-12-22T21:36:46.909Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: Do not stub primitive UI components or customized primitive components (e.g., Button). Instead, import and register the real components in test setup. This ensures tests accurately reflect production behavior and component API usage.
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:12.361Z
Learning: In Vue single-file components, allow inline Tailwind CSS class strings for static classes and avoid extracting them into computed properties solely for readability. Prefer keeping static class names inline for simplicity and performance. For dynamic or conditional classes, use Vue bindings (e.g., :class) to compose classes.
Applies to all Vue files in the repository (e.g., src/**/*.vue) where Tailwind utilities are used for static styling.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.vue:17-17
Timestamp: 2025-12-16T22:26:49.463Z
Learning: In Vue 3.5+ with <script setup>, when using defineProps<Props>() with partial destructuring (e.g., const { as = 'button', class: customClass = '' } = defineProps<Props>() ), props that are not destructured (e.g., variant, size) stay accessible by name in the template scope. This pattern is valid: you can destructure only a subset of props for convenience while referencing the remaining props directly in template expressions. Apply this guideline to Vue components across the codebase (all .vue files).
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7598
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:131-131
Timestamp: 2025-12-18T02:07:38.870Z
Learning: Tailwind CSS v4 safe utilities (e.g., items-center-safe, justify-*-safe, place-*-safe) are allowed in Vue components under src/ and in story files. Do not flag these specific safe variants as invalid when reviewing code in src/**/*.vue or related stories.
Learnt from: henrikvilhelmberglund
Repo: Comfy-Org/ComfyUI_frontend PR: 7617
File: src/components/actionbar/ComfyActionbar.vue:301-308
Timestamp: 2025-12-18T16:03:02.066Z
Learning: In the ComfyUI frontend queue system, useQueuePendingTaskCountStore().count indicates the number of tasks in the queue, where count = 1 means a single active/running task and count > 1 means there are pending tasks in addition to the active task. Therefore, in src/components/actionbar/ComfyActionbar.vue, enable the 'Clear Pending Tasks' button only when count > 1 to avoid clearing the currently running task. The active task should be canceled using the 'Cancel current run' button instead. This rule should be enforced via a conditional check on the queue count, with appropriate disabled/aria-disabled states for accessibility, and tests should verify behavior for count = 1 and count > 1.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/components/graph/selectionToolbox/ColorPickerButton.vue:15-18
Timestamp: 2025-12-21T01:06:02.786Z
Learning: In Comfy-Org/ComfyUI_frontend, in Vue component files, when a filled icon is required (e.g., 'pi pi-circle-fill'), you may mix PrimeIcons with Lucide icons since Lucide lacks filled variants. This mixed usage is acceptable when one icon library does not provide an equivalent filled icon. Apply consistently across Vue components in the src directory where icons are used, and document the rationale when a mixed approach is chosen.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7893
File: src/components/button/IconGroup.vue:5-6
Timestamp: 2026-01-08T02:26:18.357Z
Learning: In components that use the cn utility from '@/utils/tailwindUtil' with tailwind-merge, rely on the behavior that conflicting Tailwind classes are resolved by keeping the last one. For example, cn('base-classes bg-default', propClass) will have any conflicting background class from propClass override bg-default. This additive pattern is intentional and aligns with the shadcn-ui convention; ensure you document or review expectations accordingly in Vue components.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8195
File: src/platform/assets/components/MediaAssetFilterBar.vue:16-16
Timestamp: 2026-01-21T01:28:27.626Z
Learning: In Vue templates (Vue 3.4+ with the build step), when binding to data or props that are camelCase (e.g., mediaTypeFilters), you can use kebab-case in the template bindings (e.g., :media-type-filters). This is acceptable and will resolve to the corresponding camelCase variable. Do not require CamelCase in template bindings.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8090
File: src/platform/assets/components/modelInfo/ModelInfoField.vue:8-11
Timestamp: 2026-01-22T02:28:58.105Z
Learning: In Vue 3 script setup, props defined with defineProps are automatically available by name in the template without destructuring. Destructuring the result of defineProps inside script can break reactivity; prefer accessing props by name in the template. If you need to use props in the script, reference them via the defined props object rather than destructuring, or use toRefs when you intend to destructure while preserving reactivity.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8497
File: src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue:223-236
Timestamp: 2026-02-01T21:10:29.567Z
Learning: In Vue single-file components, do not review or require edits to comments. Favor self-documenting code through clear naming, strong types, and explicit APIs. If a comment is misleading or outdated, consider removing it, but avoid suggesting adding or fixing comments. This guideline aligns with preferring code clarity over comment maintenance across all .vue files.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8592
File: src/components/topbar/WorkflowExecutionIndicator.vue:28-28
Timestamp: 2026-02-03T21:35:40.889Z
Learning: In Vue single-file components where the i18n t function is only used within the template, prefer using the built-in $t in the template instead of importing useI18n and destructuring t in the script. This avoids unnecessary imports when t is not used in the script. If you need i18n in the script (Composition API), only then use useI18n and access t from its returned object. Ensure this pattern applies to all Vue components with template-only i18n usage.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 8548
File: src/components/common/TreeExplorerV2.vue:3-5
Timestamp: 2026-02-08T16:04:58.463Z
Learning: In TreeExplorerV2.vue and similar Re- ka UI Tree components, avoid defensive copies in bindings like :expanded="[...expandedKeys]". Use v-model:expanded="expandedKeys" instead. TreeRoot follows Vue's controlled component pattern and emits update:expanded with the new array; it does not mutate the passed array. Defensive copies break reactivity and can desynchronize state. Ensure components rely on update:expanded and do not mutate input arrays.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8753
File: src/renderer/extensions/vueNodes/widgets/components/WidgetDOM.vue:17-28
Timestamp: 2026-02-09T03:24:47.113Z
Learning: When destructuring reactive properties from Pinia stores in Vue components, use storeToRefs() to preserve reactivity. Example: const store = useCanvasStore(); const { canvas } = storeToRefs(store); access as canvas.value (e.g., canvas.value). Ensure you import storeToRefs from 'pinia' and use it wherever you destructure store properties in the setup function.
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI_frontend PR: 8775
File: src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue:62-68
Timestamp: 2026-02-10T17:59:25.893Z
Learning: In Vue 3 single-file components, code defined outside the script setup scope cannot access variables or helpers defined inside setup. If a default function (e.g., defineModel) is hoisted outside script setup, it will not be able to call setup helpers or reference setup-scoped variables. Place such logic inside setup or expose necessary values via returns/defineExpose to be accessible.
3953b14 to
f336725
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/common/BadgePill.vue (1)
21-31:⚠️ Potential issue | 🟠 MajorAvoid destructuring
definePropshere to keep prop reactivity.
borderStyleandfilledare destructured fromdefineProps, which can desynctextColorClass/customStyleif those props change. Use a props object ortoRefsinstead.🔧 Suggested fix (preserve reactivity)
-const { borderStyle, filled } = defineProps<{ +const props = defineProps<{ text?: string icon?: string iconClass?: string borderStyle?: string filled?: boolean }>() -const textColorClass = computed(() => - borderStyle && filled ? '' : 'text-foreground' -) +const textColorClass = computed(() => + props.borderStyle && props.filled ? '' : 'text-foreground' +) -const customStyle = computed(() => { - if (!borderStyle) { +const customStyle = computed(() => { + if (!props.borderStyle) { return { borderColor: 'var(--border-color)' } } - const isGradient = borderStyle.includes('linear-gradient') + const isGradient = props.borderStyle.includes('linear-gradient') if (isGradient) { return { borderColor: 'transparent', - backgroundImage: `linear-gradient(var(--base-background), var(--base-background)), ${borderStyle}`, + backgroundImage: `linear-gradient(var(--base-background), var(--base-background)), ${props.borderStyle}`, backgroundOrigin: 'border-box', backgroundClip: 'padding-box, border-box' } } - if (filled) { + if (props.filled) { return { - borderColor: borderStyle, - backgroundColor: `${borderStyle}33`, - color: borderStyle + borderColor: props.borderStyle, + backgroundColor: `${props.borderStyle}33`, + color: props.borderStyle } } - return { borderColor: borderStyle } + return { borderColor: props.borderStyle } })Based on learnings: "In Vue 3 script setup, props defined with defineProps are automatically available by name in the template without destructuring. Destructuring the result of defineProps inside script can break reactivity; prefer accessing props by name in the template. If you need to use props in the script, reference them via the defined props object rather than destructuring, or use toRefs when you intend to destructure while preserving reactivity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/BadgePill.vue` around lines 21 - 31, The props were destructured from defineProps causing loss of reactivity for computed values like textColorClass (and any customStyle); change to keep the props reactive by not destructuring: call defineProps into a single props object (or use toRefs) and update computed to reference props.borderStyle and props.filled (or their refs) instead of the standalone variables; locate the defineProps call and the computed textColorClass (and any related customStyle) and switch their accesses to the reactive props object/toRefs to restore reactivity.
🧹 Nitpick comments (3)
src/components/common/TreeExplorerV2Node.vue (1)
27-45: Addtype="button"to prevent unintended form submissions.The bookmark button is missing
type="button". Without it, the button defaults totype="submit"and could trigger form submission if this component is ever rendered inside a form.Also, this uses PrimeIcons (
pi pi-bookmark/pi pi-bookmark-fill) while guidelines prefer Iconify. This is acceptable per prior decisions when Lucide lacks filled variants, but consider checking if Iconify has a filled bookmark icon available (e.g.,icon-[mdi--bookmark]/icon-[mdi--bookmark-outline]).Proposed fix for type attribute
<button + type="button" :class=" cn(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/TreeExplorerV2Node.vue` around lines 27 - 45, The bookmark button in TreeExplorerV2Node.vue is missing an explicit type and may submit enclosing forms; update the button element (the one using cn(...) and `@click.stop`="toggleBookmark" and conditional classes based on isBookmarked) to include type="button". While here, optionally replace the PrimeIcons classes ('pi pi-bookmark' / 'pi pi-bookmark-fill') with Iconify equivalents (e.g., icon-[mdi--bookmark] / icon-[mdi--bookmark-outline]) if available, keeping the isBookmarked conditional logic in place.src/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vue (1)
51-55: Consider usingSortingStrategyIdtype for thesortOrderprop.The
sortOrderprop is typed as a genericstring, but the parent component passes values of typeSortingStrategyId. Using the specific type improves type safety and ensures consistency with the string literal comparison on line 22.♻️ Suggested type improvement
+import type { SortingStrategyId } from '@/types/nodeOrganizationTypes' import type { NodeLibrarySection, RenderedTreeExplorerNode, TreeNode } from '@/types/treeExplorerTypes' -const { fillNodeInfo, sortOrder = 'original' } = defineProps<{ +const { fillNodeInfo, sortOrder = 'original' as SortingStrategyId } = defineProps<{ sections: NodeLibrarySection<ComfyNodeDefImpl>[] fillNodeInfo: (node: TreeNode) => RenderedTreeExplorerNode<ComfyNodeDefImpl> - sortOrder?: string + sortOrder?: SortingStrategyId }>()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vue` around lines 51 - 55, The sortOrder prop is currently typed as a generic string in the defineProps call (const { fillNodeInfo, sortOrder = 'original' } = defineProps<...>()), but the parent passes SortingStrategyId values; change the sortOrder prop type from string to SortingStrategyId and update the default to the appropriate SortingStrategyId literal so comparisons (e.g., where sortOrder === 'original') are type-safe—import or reference the SortingStrategyId type and use it in the defineProps generic for the sortOrder property.src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue (1)
415-427: Remove unnecessary type casts for consistency.The
TabIdtype is defined as'essentials' | 'all' | 'custom', and all three values in the array are valid union members. Since the array is explicitly typed asArray<{ value: TabId; label: string }>, TypeScript can infer the types correctly without explicit casts. The inconsistency (no cast for'all'but casts for'essentials'and'custom') should be resolved by removing all casts.♻️ Suggested cleanup
const tabs = computed(() => { const allTabs: Array<{ value: TabId; label: string }> = [ { value: 'all', label: t('sideToolbar.nodeLibraryTab.allNodes') }, { - value: 'essentials' as TabId, + value: 'essentials', label: t('sideToolbar.nodeLibraryTab.essentials') }, { - value: 'custom' as TabId, + value: 'custom', label: t('sideToolbar.nodeLibraryTab.blueprints') } ] return flags.nodeLibraryEssentialsEnabled ? allTabs : [allTabs[0], allTabs[2]] })🤖 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 415 - 427, Remove the unnecessary "as TabId" casts in the allTabs array: the array is already typed as Array<{ value: TabId; label: string }>, so TypeScript will infer the union members for 'essentials' and 'custom' without explicit casts; update the allTabs declaration (and any similar entries) to use plain string literals for value and keep the conditional return using flags.nodeLibraryEssentialsEnabled unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/common/BadgePill.vue`:
- Around line 21-31: The props were destructured from defineProps causing loss
of reactivity for computed values like textColorClass (and any customStyle);
change to keep the props reactive by not destructuring: call defineProps into a
single props object (or use toRefs) and update computed to reference
props.borderStyle and props.filled (or their refs) instead of the standalone
variables; locate the defineProps call and the computed textColorClass (and any
related customStyle) and switch their accesses to the reactive props
object/toRefs to restore reactivity.
---
Duplicate comments:
In `@src/components/sidebar/tabs/nodeLibrary/EssentialNodeCard.vue`:
- Around line 23-26: The Teleport ref callback currently assigns to the const
previewRef instead of the Vue Ref value, causing runtime errors; update the
Teleport element to either use a template ref (ref="previewRef") or change the
callback to assign to previewRef.value (e.g., (el) => (previewRef.value = el as
HTMLElement)), and ensure the composable useNodePreviewAndDrag also treats
previewRef as a Vue ref and only accesses/sets previewRef.value throughout the
component and composable so the preview is correctly set/unset without throwing.
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue`:
- Around line 45-50: Replace the raw <button> in NodeLibrarySidebarTabV2.vue
used as the filter trigger with the shared Button component: keep the
:aria-label binding ($t('sideToolbar.nodeLibraryTab.filter')), preserve the
intent of the classes/variant (map existing classes/props into the shared
Button's props such as size, variant, rounded, and any class attribute), and
move the <i class="icon-[lucide--list-filter] size-4" /> icon into the Button's
default slot; ensure any event handlers or attributes on the original button are
forwarded to the shared Button so behavior remains identical.
- Line 250: The ref searchBoxRef is currently untyped causing an implicit any;
change its declaration to a properly typed Vue Ref (e.g., Ref<HTMLInputElement |
null>) and initialize with null, and ensure the Ref type is imported from 'vue'
if not already; update the symbol searchBoxRef to use ref<HTMLInputElement |
null>(null) so template/TS checks know it's an HTML input element (or pick the
correct element type if different).
---
Nitpick comments:
In `@src/components/common/TreeExplorerV2Node.vue`:
- Around line 27-45: The bookmark button in TreeExplorerV2Node.vue is missing an
explicit type and may submit enclosing forms; update the button element (the one
using cn(...) and `@click.stop`="toggleBookmark" and conditional classes based on
isBookmarked) to include type="button". While here, optionally replace the
PrimeIcons classes ('pi pi-bookmark' / 'pi pi-bookmark-fill') with Iconify
equivalents (e.g., icon-[mdi--bookmark] / icon-[mdi--bookmark-outline]) if
available, keeping the isBookmarked conditional logic in place.
In `@src/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vue`:
- Around line 51-55: The sortOrder prop is currently typed as a generic string
in the defineProps call (const { fillNodeInfo, sortOrder = 'original' } =
defineProps<...>()), but the parent passes SortingStrategyId values; change the
sortOrder prop type from string to SortingStrategyId and update the default to
the appropriate SortingStrategyId literal so comparisons (e.g., where sortOrder
=== 'original') are type-safe—import or reference the SortingStrategyId type and
use it in the defineProps generic for the sortOrder property.
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue`:
- Around line 415-427: Remove the unnecessary "as TabId" casts in the allTabs
array: the array is already typed as Array<{ value: TabId; label: string }>, so
TypeScript will infer the union members for 'essentials' and 'custom' without
explicit casts; update the allTabs declaration (and any similar entries) to use
plain string literals for value and keep the conditional return using
flags.nodeLibraryEssentialsEnabled unchanged.
dosubot
bot
added
the
size:XL
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/types/nodeSource.ts (1)
62-65:⚠️ Potential issue | 🟠 MajorGuard the
custom_nodesparse path against missing module names.
moduleNamecan beundefined(e.g. input'custom_nodes'), andmoduleName.split('@')will throw. ReturnUNKNOWN_NODE_SOURCEwhen the second segment is absent.Suggested fix
} else if (modules[0] === 'custom_nodes') { const moduleName = modules[1] + if (!moduleName) { + return UNKNOWN_NODE_SOURCE + } const customNodeName = moduleName.split('@')[0] const displayName = shortenNodeName(customNodeName) return {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/nodeSource.ts` around lines 62 - 65, The custom_nodes branch assumes modules[1] exists and calls moduleName.split('@'), which throws when the input is just 'custom_nodes'; update the branch in the function handling node source parsing to check modules[1] (moduleName) before using it, and if missing return UNKNOWN_NODE_SOURCE; when present, proceed to compute customNodeName = moduleName.split('@')[0] and call shortenNodeName(customNodeName) as before.
♻️ Duplicate comments (2)
src/components/common/TreeExplorerV2.vue (1)
4-6:v-model:expandedfix is correctly applied.The defensive copy
[...expandedKeys]has been replaced withv-model:expanded="expandedKeys", enabling proper two-way synchronization viaupdate:expanded. This resolves the earlier desync issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/TreeExplorerV2.vue` around lines 4 - 6, The previous defensive copy removed two-way binding for expanded state; ensure TreeExplorerV2.vue now uses v-model:expanded="expandedKeys" (as shown) and confirm the TreeRoot component emits the corresponding "update:expanded" event (or uses defineModel) so updates flow back to the parent; if TreeRoot mutates the array internally, adjust it to emit a new array (e.g., emit("update:expanded", [...newExpanded]) ) rather than modifying a prop in-place to keep reactivity working with expandedKeys.src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue (1)
45-50: Previously flagged: filter trigger is still a raw<button>insrc/components.This appears unchanged from prior feedback; please use the shared
Buttoncomponent for consistency with the design system.Based on learnings: "In the ComfyUI_frontend Vue codebase, replace raw HTML elements with the shared Button component located at src/components/ui/button/Button.vue."
🤖 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 45 - 50, Replace the raw <button> with the shared Button component: import and use Button (src/components/ui/button/Button.vue) in the NodeLibrarySidebarTabV2 component, pass the aria-label via :aria-label="$t('sideToolbar.nodeLibraryTab.filter')" and transfer the utility classes (or map them to the Button's variant/size props if available), and render the icon <i class="icon-[lucide--list-filter] size-4" /> inside the Button slot so the Button handles accessibility, styling, and behavior consistently with other UI controls (update any event handlers or attributes to the Button API).
🧹 Nitpick comments (2)
src/components/common/TreeExplorerV2.vue (1)
22-27: Redundant type annotations in template inline handler.
emitis already typed viadefineEmits; repeatingRenderedTreeExplorerNode<ComfyNodeDefImpl>andMouseEventin the template arrow function adds noise without adding safety.♻️ Proposed simplification
- `@node-click`=" - ( - node: RenderedTreeExplorerNode<ComfyNodeDefImpl>, - e: MouseEvent - ) => emit('nodeClick', node, e) - " + `@node-click`="(node, e) => emit('nodeClick', node, e)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/TreeExplorerV2.vue` around lines 22 - 27, The inline template handler for the node-click binding redundantly repeats type annotations (RenderedTreeExplorerNode<ComfyNodeDefImpl> and MouseEvent); remove the explicit parameter types and simplify the arrow function to just "(node, e) => emit('nodeClick', node, e)" so it relies on defineEmits typing instead—update the node-click attribute in TreeExplorerV2.vue to use untyped parameters while keeping the emit call and event name unchanged.src/services/nodeOrganizationService.ts (1)
203-217: Extract blueprint partitioning into a shared helper to avoid drift.The blueprint/non-blueprint classification loop is duplicated across two branches with nearly identical logic.
As per coding guidelines "When writing new code, ask if there is a simpler way to introduce the same functionality; choose the simpler approach if one exists."♻️ Refactor sketch
+function partitionNodeSources(nodes: ComfyNodeDefImpl[]) { + const myBlueprints: ComfyNodeDefImpl[] = [] + const comfyBlueprints: ComfyNodeDefImpl[] = [] + const partnerNodes: ComfyNodeDefImpl[] = [] + const comfyNodes: ComfyNodeDefImpl[] = [] + const extensions: ComfyNodeDefImpl[] = [] + + for (const node of nodes) { + if ( + node.nodeSource.type === NodeSourceType.Blueprint || + node.python_module?.startsWith('blueprint') + ) { + if (node.isGlobal) comfyBlueprints.push(node) + else myBlueprints.push(node) + } else if (node.api_node || node.category?.startsWith('api node')) { + partnerNodes.push(node) + } else if ( + node.nodeSource.type === NodeSourceType.Core || + node.nodeSource.type === NodeSourceType.Essentials + ) { + comfyNodes.push(node) + } else { + extensions.push(node) + } + } + + return { myBlueprints, comfyBlueprints, partnerNodes, comfyNodes, extensions } +}Also applies to: 239-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/nodeOrganizationService.ts` around lines 203 - 217, Extract the duplicated blueprint partitioning logic into a shared helper (e.g., partitionBlueprints) that accepts the nodes array and returns two arrays { myBlueprints, comfyBlueprints }; the helper should encapsulate the predicate used in the case 'blueprints' branch (check nodeDef.nodeSource.type === NodeSourceType.Blueprint OR nodeDef.python_module?.startsWith('blueprint')) and the isGlobal check to decide which output bucket to push into, then replace the duplicated loops in both locations (the case 'blueprints' block and the similar block around lines 239-265) to call this helper and use its returned myBlueprints and comfyBlueprints.
🤖 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/services/nodeOrganizationService.ts`:
- Around line 320-333: Replace the hardcoded English labels ('My Blueprints',
'Comfy Blueprints') emitted by buildBlueprintTree/buildNodeDefTree with vue-i18n
translation keys: call the i18n translator (e.g., useI18n().t or the shared
i18n.t instance used in this service) when constructing the node objects so
children.push uses t('...') instead of string literals; add corresponding keys
to src/locales/en/main.json (for example nodeTree.myBlueprints and
nodeTree.comfyBlueprints). Ensure the service imports/uses the same i18n object
pattern used across src (do not leave raw strings) and update any other places
where buildBlueprintTree emits user-facing labels.
In `@src/utils/treeUtil.ts`:
- Around line 47-52: The unwrap condition currently treats an empty array as
truthy because it checks tree.children[0].children; update the conditional in
the unwrap logic so it requires at least one grandchild by checking
tree.children[0].children?.length > 0 (or >= 1) instead of truthiness; keep the
rest of the block (the return { ...tree, children: tree.children[0].children })
intact so only non-empty single-child wrappers are unwrapped.
---
Outside diff comments:
In `@src/types/nodeSource.ts`:
- Around line 62-65: The custom_nodes branch assumes modules[1] exists and calls
moduleName.split('@'), which throws when the input is just 'custom_nodes';
update the branch in the function handling node source parsing to check
modules[1] (moduleName) before using it, and if missing return
UNKNOWN_NODE_SOURCE; when present, proceed to compute customNodeName =
moduleName.split('@')[0] and call shortenNodeName(customNodeName) as before.
---
Duplicate comments:
In `@src/components/common/TreeExplorerV2.vue`:
- Around line 4-6: The previous defensive copy removed two-way binding for
expanded state; ensure TreeExplorerV2.vue now uses
v-model:expanded="expandedKeys" (as shown) and confirm the TreeRoot component
emits the corresponding "update:expanded" event (or uses defineModel) so updates
flow back to the parent; if TreeRoot mutates the array internally, adjust it to
emit a new array (e.g., emit("update:expanded", [...newExpanded]) ) rather than
modifying a prop in-place to keep reactivity working with expandedKeys.
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue`:
- Around line 45-50: Replace the raw <button> with the shared Button component:
import and use Button (src/components/ui/button/Button.vue) in the
NodeLibrarySidebarTabV2 component, pass the aria-label via
:aria-label="$t('sideToolbar.nodeLibraryTab.filter')" and transfer the utility
classes (or map them to the Button's variant/size props if available), and
render the icon <i class="icon-[lucide--list-filter] size-4" /> inside the
Button slot so the Button handles accessibility, styling, and behavior
consistently with other UI controls (update any event handlers or attributes to
the Button API).
---
Nitpick comments:
In `@src/components/common/TreeExplorerV2.vue`:
- Around line 22-27: The inline template handler for the node-click binding
redundantly repeats type annotations (RenderedTreeExplorerNode<ComfyNodeDefImpl>
and MouseEvent); remove the explicit parameter types and simplify the arrow
function to just "(node, e) => emit('nodeClick', node, e)" so it relies on
defineEmits typing instead—update the node-click attribute in TreeExplorerV2.vue
to use untyped parameters while keeping the emit call and event name unchanged.
In `@src/services/nodeOrganizationService.ts`:
- Around line 203-217: Extract the duplicated blueprint partitioning logic into
a shared helper (e.g., partitionBlueprints) that accepts the nodes array and
returns two arrays { myBlueprints, comfyBlueprints }; the helper should
encapsulate the predicate used in the case 'blueprints' branch (check
nodeDef.nodeSource.type === NodeSourceType.Blueprint OR
nodeDef.python_module?.startsWith('blueprint')) and the isGlobal check to decide
which output bucket to push into, then replace the duplicated loops in both
locations (the case 'blueprints' block and the similar block around lines
239-265) to call this helper and use its returned myBlueprints and
comfyBlueprints.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
knip.config.tspackages/design-system/src/css/style.csssrc/components/common/TreeExplorerV2.vuesrc/components/searchbox/v2/NodeSearchListItem.vuesrc/components/sidebar/tabs/NodeLibrarySidebarTabV2.test.tssrc/components/sidebar/tabs/NodeLibrarySidebarTabV2.vuesrc/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vuesrc/components/sidebar/tabs/nodeLibrary/BlueprintsPanel.vuesrc/components/sidebar/tabs/nodeLibrary/EssentialNodesPanel.test.tssrc/components/sidebar/tabs/nodeLibrary/EssentialNodesPanel.vuesrc/locales/en/main.jsonsrc/services/nodeOrganizationService.tssrc/stores/nodeDefStore.tssrc/types/nodeOrganizationTypes.tssrc/types/nodeSource.test.tssrc/types/nodeSource.tssrc/types/treeExplorerTypes.tssrc/utils/treeUtil.ts
💤 Files with no reviewable changes (1)
- src/components/searchbox/v2/NodeSearchListItem.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- src/types/treeExplorerTypes.ts
- src/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vue
Yourz
force-pushed
the
library-improvements
branch
from
d759555 to
cfb3de1
Compare
Yourz
added
the
preview-cpu
Yourz
force-pushed
the
library-improvements
branch
from
3a9088c to
bbef03f
Compare
⚡ Performance Report
Raw data{
"timestamp": "2026-02-28T14:29:45.003Z",
"gitSha": "11658051743b9ccff6f24e50c2ecf11d66b7ed7b",
"branch": "library-improvements",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2048.852000000011,
"styleRecalcs": 123,
"styleRecalcDurationMs": 25.149999999999995,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 516.1790000000001,
"heapDeltaBytes": -2434704
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1891.2529999999492,
"styleRecalcs": 174,
"styleRecalcDurationMs": 57.701,
"layouts": 12,
"layoutDurationMs": 3.575,
"taskDurationMs": 867.153,
"heapDeltaBytes": -3802256
},
{
"name": "dom-widget-clipping",
"durationMs": 562.3850000000061,
"styleRecalcs": 42,
"styleRecalcDurationMs": 13.081000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 356.414,
"heapDeltaBytes": 6236484
}
]
} |
Yourz
force-pushed
the
library-improvements
branch
from
bbef03f to
e82f245
Compare
Yourz
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
github-actions
bot
removed
the
New Browser Test Expectations
Yourz
force-pushed
the
library-improvements
branch
from
384437a to
8cc7be8
Compare
Yourz
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
Yourz
force-pushed
the
library-improvements
branch
2 times, most recently
from
5e914e4 to
0bc8ddb
Compare
Yourz
force-pushed
the
library-improvements
branch
from
0bc8ddb to
2fe4da2
Compare
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
Yourz
force-pushed
the
library-improvements
branch
from
2fe4da2 to
4094300
Compare
Yourz
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
DrJKL
pushed a commit
that referenced
this pull request
Feb 28, 2026
## Summary Implement 11 Figma design discrepancies for the Node Library sidebar and V2 Node Search dialog, aligning the UI with the [Toolbox Figma design](https://www.figma.com/design/xMFxCziXJe6Denz4dpDGTq/Toolbox?node-id=2074-21394&m=dev). ## Changes - **What**: Sidebar: reorder tabs (All/Essentials/Blueprints), rename Custom→Blueprints, uppercase section headers, chevron-left of folder icon, bookmark-on-hover for node rows, filter dropdown with checkbox items, sort labels (Categorized/A-Z) with label-left/check-right layout, hide section headers in A-Z mode. Search dialog: expand filter chips from 3→6, add Recents and source categories to sidebar, remove "Filter by" label. Pull foundation V2 components from merged PR #8548. - **Dependencies**: Depends on #8987 (V2 Node Search) and #8548 (NodeLibrarySidebarTabV2) ## Review Focus - Filter dropdown (`filterOptions`) is UI-scaffolded but not yet wired to filtering logic (pending V2 integration) - "Recents" category currently returns frequency-based results as placeholder until a usage-tracking store is implemented - Pre-existing type errors from V2 PR dependencies not in the base commit (SearchBoxV2, usePerTabState, TextTicker, getProviderIcon, getLinkTypeColor, SidebarContainerKey) are expected and will resolve when rebased onto main after parent PRs land ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9085-feat-Node-Library-sidebar-and-V2-Search-dialog-Figma-design-improvements-30f6d73d36508175bf72d716f5904476) by [Unito](https://www.unito.io) --------- Co-authored-by: Yourz <crazilou@vip.qq.com> Co-authored-by: github-actions <github-actions@github.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
2 participants
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.
Summary
Implement 11 Figma design discrepancies for the Node Library sidebar and V2 Node Search dialog, aligning the UI with the Toolbox Figma design.
Changes
Review Focus
filterOptions) is UI-scaffolded but not yet wired to filtering logic (pending V2 integration)┆Issue is synchronized with this Notion page by Unito