-
Notifications
You must be signed in to change notification settings - Fork 440
feat: bring node to front when clicking on any widget #7202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adds a capture-phase pointerdown handler to NodeWidgets that calls bringNodeToFront whenever any widget is clicked, improving UX by ensuring the interacted node is always visible on top.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (3)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds capture-phase pointerdown in node widgets and calls a z-index composable from selection handlers so unpinned nodes are moved to the front on pointer interaction; includes end-to-end tests validating bring-to-front behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NodeWidget as NodeWidgets.vue
participant Handlers as useNodeEventHandlers.ts
participant ZIndex as useNodeZIndex
User->>NodeWidget: pointerdown (capture)
NodeWidget->>ZIndex: bringNodeToFront(nodeId)
ZIndex-->>NodeWidget: stacking updated
Note over User,Handlers: pointerup / selection flow
User->>NodeWidget: pointerup
NodeWidget->>Handlers: handleNodeSelect / toggleNodeSelectionAfterPointerUp
Handlers->>Handlers: check pinned?
alt not pinned
Handlers->>ZIndex: bringNodeToFront(nodeId)
ZIndex-->>Handlers: stacking updated
end
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/11/2025, 08:50:13 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results✅ All tests passed! ⏰ Completed at: 12/11/2025, 08:55:40 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.24 MB (baseline 3.23 MB) • 🔴 +747 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 988 kB (baseline 988 kB) • 🟢 -249 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 178 kB (baseline 178 kB) • ⚪ 0 BReusable component library chunks
Status: 7 added / 7 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 3.18 kB (baseline 3.18 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 18 added / 18 removed |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
@Myestery Want to add a small Playwright test for this? |
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
Selection now automatically brings the node to front (unless pinned), ensuring selected nodes are always visible above other nodes.
Verifies overlapping nodes come to front when: - Clicking on the node body - Clicking on a widget
|
Updating Playwright Expectations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts-snapshots/bring-to-front-overlapped-after-chromium-darwin.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts-snapshots/bring-to-front-overlapped-before-chromium-darwin.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts-snapshots/bring-to-front-widget-overlapped-after-chromium-darwin.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts-snapshots/bring-to-front-widget-overlapped-before-chromium-darwin.pngis excluded by!**/*.png
📒 Files selected for processing (1)
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}: Test user workflows in browser tests
Use Playwright fixtures for browser tests
Follow naming conventions for browser tests
Check assets/ directory for test data when writing tests
Prefer specific selectors in browser tests
Test across multiple viewports
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
browser_tests/**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write E2E tests with Playwright; optional tags like
@mobileand@2xare respected by config
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
🧠 Learnings (10)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test across multiple viewports
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Follow naming conventions for browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{js,ts,jsx,tsx} : When adding features, always write vitest unit tests using cursor rules in @.cursor
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing tests dependent on non-behavioral features like utility classes or styles
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
🧬 Code graph analysis (1)
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts (2)
tests-ui/tests/litegraph/core/fixtures/testExtensions.ts (1)
test(35-67)browser_tests/helpers/fitToView.ts (1)
fitToViewInstant(15-104)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
- GitHub Check: setup
🔇 Additional comments (1)
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts (1)
99-107: Remove the unusedtextWidgetvariable and use the widget's bounding box for the click instead of hard-coded coordinatesThe second test has two issues that should be fixed:
Unused variable:
const textWidget = clipNode.locator('textarea').first()is declared but never used.Hard-coded widget coordinates: Clicking at
clipBox.x + 170, clipBox.y + 80assumes a fixed internal layout and doesn't guarantee the click targets the textarea element itself. This can break if node styles change.The textWidget locator is already available; use its bounding box to calculate the click position:
- const clipNode = comfyPage.vueNodes.getNodeByTitle('CLIP Text Encode') - const clipBox = await clipNode.boundingBox() - const textWidget = clipNode.locator('textarea').first() - if (!clipBox) throw new Error('CLIP node not found') - await comfyPage.page.mouse.click(clipBox.x + 170, clipBox.y + 80) + const clipNode = comfyPage.vueNodes.getNodeByTitle('CLIP Text Encode') + const textWidget = clipNode.locator('textarea').first() + const widgetBox = await textWidget.boundingBox() + if (!widgetBox) throw new Error('CLIP text widget not found') + await comfyPage.page.mouse.click( + widgetBox.x + widgetBox.width / 2, + widgetBox.y + widgetBox.height / 2 + )This removes dead code, ties the interaction to the actual widget bounds, and aligns with the pattern used in
getNodeCenterelsewhere in the test.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts (1)
53-57: Reduce brittleness from hard-coded drag/click offsets; prefer locators where possibleBoth tests rely on magic offsets for drag and click targets (
+50/+10,+30/+10,+170/+80), which assumes a very particular node layout and widget positioning. That increases the chance of flakiness if styles, fonts, or viewports change.Consider tightening this by:
- Deriving drag/click coordinates from both nodes’ bounding boxes (e.g. choose a point guaranteed to be inside CLIP but outside the overlapping node), rather than fixed offsets; and/or
- For the widget case, scoping to a specific widget locator inside the CLIP node (e.g. a text input/textarea within
clipNode) and calling.click()on that element instead of raw page coordinates, aligning with the “prefer specific selectors” guidance.This would better express intent and make the tests more resilient to layout tweaks. Based on learnings, prefer specific selectors in browser tests.
Also applies to: 76-78, 100-103, 116-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-create-group-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (1)
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}: Test user workflows in browser tests
Use Playwright fixtures for browser tests
Follow naming conventions for browser tests
Check assets/ directory for test data when writing tests
Prefer specific selectors in browser tests
Test across multiple viewports
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
browser_tests/**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write E2E tests with Playwright; optional tags like
@mobileand@2xare respected by config
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
🧠 Learnings (15)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test across multiple viewports
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{js,ts,jsx,tsx} : When adding features, always write vitest unit tests using cursor rules in @.cursor
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing tests dependent on non-behavioral features like utility classes or styles
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Follow naming conventions for browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Use Playwright fixtures for browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Always prefer vitest mock functions over writing verbose manual mocks
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing redundant tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: collect
- GitHub Check: test
- GitHub Check: lint-and-format
🔇 Additional comments (1)
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts (1)
20-41: Helper utilities for z-index and centers are clear and reusable
getNodeZIndexandgetNodeCenternicely encapsulate the DOM probing you need with properComfyPagetyping and simple error handling, which keeps the test bodies focused on behavior rather than locator details. No changes needed here.
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
Outdated
Show resolved
Hide resolved
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
Outdated
Show resolved
Hide resolved
Makes assertions more robust by comparing both nodes' z-indices after the click action rather than comparing to stale pre-action values.
…:Comfy-Org/ComfyUI_frontend into feat/bring-node-to-front-on-widget-click
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
browser_tests/tests/interaction.spec.ts-snapshots/prompt-dialog-closed-text-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/nodeSearchBox.spec.ts-snapshots/added-node-no-connection-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-create-group-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (1)
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}: Test user workflows in browser tests
Use Playwright fixtures for browser tests
Follow naming conventions for browser tests
Check assets/ directory for test data when writing tests
Prefer specific selectors in browser tests
Test across multiple viewports
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
browser_tests/**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write E2E tests with Playwright; optional tags like
@mobileand@2xare respected by config
Files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
🧠 Learnings (17)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test across multiple viewports
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Follow naming conventions for browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{js,ts,jsx,tsx} : When adding features, always write vitest unit tests using cursor rules in @.cursor
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to browser_tests/**/*.spec.ts : Write E2E tests with Playwright; optional tags like `mobile` and `2x` are respected by config
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing tests dependent on non-behavioral features like utility classes or styles
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Use Playwright fixtures for browser tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Always prefer vitest mock functions over writing verbose manual mocks
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing redundant tests
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Check tests-ui/README.md for test guidelines
Applied to files:
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: setup
- GitHub Check: collect
🔇 Additional comments (3)
browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts (3)
1-6: LGTM - Clean type import addresses previous feedback.Importing
ComfyPagetype directly from the fixture module (line 5) is cleaner than the previously suggested local alias and eliminates the verbose inline type annotations. This makes the helper signatures readable while maintaining type safety.
80-83: LGTM - Z-index assertion correctly compares post-action state.Comparing both nodes' z-indices after the click action (lines 81-82) makes the test robust to z-index normalization or rebasing strategies. This addresses the previous review feedback and focuses the assertion on the actual behavior: "CLIP is above KSampler after clicking CLIP."
124-127: LGTM - Z-index assertion correctly compares post-action state.Comparing both nodes' z-indices after the widget click (lines 125-126) properly validates that CLIP moves above VAE regardless of absolute z-index values. This mirrors the correct pattern from the first test and addresses previous review feedback.
Summary
bringNodeToFrontwhenever any widget is clickedfix #7131
Before
Screen.Recording.2025-12-06.at.04.39.39.mov
After
Screen.Recording.2025-12-06.at.04.40.05.mov
Test plan