fix(vueNodes): decrease default size of reroute nodes#8734
Open
christian-byrne wants to merge 4 commits intomainfrom
Open
fix(vueNodes): decrease default size of reroute nodes#8734christian-byrne wants to merge 4 commits intomainfrom
christian-byrne wants to merge 4 commits intomainfrom
Conversation
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/14/2026, 03:00:58 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Playwright: ❌ 523 passed, 3 failed · 1 flaky ❌ Failed Tests📊 Browser Reports
|
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:
📝 WalkthroughWalkthroughDisables UI resizing for reroute nodes, tightens their computed size, exempts them from default min-width/min-height and certain padding in the Vue renderer, and adds unit and E2E tests verifying smaller sizing and absence of resize handles. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 21.7 kB (baseline 21.7 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 880 kB (baseline 879 kB) • 🔴 +400 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.9 kB (baseline 68.9 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 427 kB (baseline 427 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Editors & Dialogs — 785 B (baseline 785 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 36.6 kB (baseline 36.6 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.15 MB (baseline 2.15 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 237 kB (baseline 237 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.69 MB (baseline 8.69 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.31 MB (baseline 7.31 MB) • ⚪ 0 BBundles that do not match a named category
|
christian-byrne
added
the
preview-cpu
christian-byrne
force-pushed
the
fix/vue-nodes/reroute-default-size
branch
from
2e7d670 to
90bea80
Compare
![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: 1
🤖 Fix all issues with AI agents
In `@browser_tests/tests/vueNodes/rerouteNodeSize.spec.ts`:
- Around line 68-69: The assertion is checking for zero resize handles but uses
.last(), which is misleading; update the locator usage by removing .last() so
that resizeHandle is obtained via
rerouteEl.locator('[role="button"][aria-label]') and then assert await
expect(resizeHandle).toHaveCount(0) (keep the existing selector fallback),
ensuring the test clearly asserts that no resize handle elements exist.
🧹 Nitpick comments (1)
browser_tests/tests/vueNodes/rerouteNodeSize.spec.ts (1)
16-20: Consider extracting the repeated reroute node lookup into a helper.The logic to find the reroute node ID via
page.evaluateis duplicated across all four tests. Extracting this into a helper function would reduce duplication and improve maintainability.♻️ Suggested helper extraction
async function getRerouteNodeId(page: Page): Promise<string | null> { return page.evaluate(() => { const graph = window.app!.graph! const rerouteNode = graph.nodes.find((n) => n.type === 'Reroute') return rerouteNode ? String(rerouteNode.id) : null }) }Then in each test:
const rerouteNodeId = await getRerouteNodeId(comfyPage.page) expect(rerouteNodeId).not.toBeNull()Also applies to: 33-41, 59-63, 75-79
dosubot
bot
added
the
size:L
Member
|
In litegraph mode the node is very hard to move as the clickable area for the slots takes up most of the node |
christian-byrne
force-pushed
the
fix/vue-nodes/reroute-default-size
branch
2 times, most recently
from
93692fb to
a88a5e6
Compare
- Skip min-w-[225px] and min-h constraints for reroute nodes in Vue - Render bare NodeSlots without body wrapper for compact layout - Hide footer Button and resize handles for reroute nodes - All changes are Vue-only; no modifications to shared litegraph code - Add screenshot baseline test for visual regression Fixes #4704 Amp-Thread-ID: https://ampcode.com/threads/T-019c5893-c9d2-77cb-9e90-a9a0631220db
christian-byrne
force-pushed
the
fix/vue-nodes/reroute-default-size
branch
from
a88a5e6 to
9925d27
Compare
Contributor
Author
|
I simplified it 
|
Contributor
Author
|
Test here: https://pr-2369.testenvs.comfy.org/ |
Reroute nodes without min-h caused ResizeObserver to measure a tiny DOM height. After removeNodeTitleHeight subtracted 30px the stored height became 0, making the node invisible when switching to litegraph mode. Use h-(--node-height) so the DOM height always matches computeSize() after the title-height normalization. Amp-Thread-ID: https://ampcode.com/threads/T-019c59fb-447e-7191-ba36-b85f723346e1
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
3 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
Remove the 225px minimum width constraint from reroute nodes so they render at their intended ~75×26px size.
Changes
min-w-[225px]CSS constraint and bottom padding applied to regular nodes.resizable: falseis set on the RerouteNode constructor to hide the resize handle. AnisRerouteNodecomputed inLGraphNode.vuegates these behaviors by checkingnodeData.type === "Reroute".Review Focus
type === "Reroute"(explicit) rather thantitleMode === NO_TITLE(semantic but too broad). See PR fix(vue-nodes): hide slot labels for reroute nodes with empty names #8574 as prior art for reroute-specific conditionals.LGraphNode.tspos/size setters but does not touchLGraphNode.vuetemplate or resize callback — no conflict expected.Fixes #4704
Screenshots (if applicable)
Reroute nodes now render at ~75px wide instead of being forced to 225px minimum.
┆Issue is synchronized with this Notion page by Unito