Allow Vue nodes to have their colors changed from selection toolbox #5720

christian-byrne · 2025-09-22T04:38:24Z

Summary

Fixes #5680 by allowing Vue nodes to properly synchronize color changes with LiteGraph nodes, implementing header darkening and light theme adjustments.

Changes

What: Implemented color property synchronization between LiteGraph and Vue node rendering systems
Core Fix: Added nodeData.color and nodeData.bgcolor to v-memo dependencies to trigger re-renders on color changes
Color Logic: Added header darkening using memoized color adjustments to match LiteGraph's ColorOption system
Event System: Enhanced property change instrumentation in LGraphNode.setColorOption to emit color/bgcolor events

Review Focus

Vue component reactivity timing - the v-memo fix was critical for immediate color updates. Verify light theme color adjustments match the drawNode monkey patch behavior in app.ts.

Technical Details

graph TD
    A[User Sets Color] --> B[LGraphNode.setColorOption]
    B --> C[Sets node.color & node.bgcolor]
    C --> D[Triggers property:changed events]
    D --> E[Vue Node Manager Updates]
    E --> F[v-memo Detects Change]
    F --> G[NodeHeader Re-renders]
    G --> H[Header Darkening Applied]

    style A fill:#f9f9f9,stroke:#333,color:#000
    style H fill:#f9f9f9,stroke:#333,color:#000

┆Issue is synchronized with this Notion page by Unito

github-actions · 2025-09-22T04:38:52Z

🎭 Playwright Test Results

⚠️ Tests passed with flaky tests

⏰ Completed at: 09/27/2025, 10:04:55 PM UTC

📈 Summary

Total Tests: 471
Passed: 438 ✅
Failed: 0
Flaky: 4 ⚠️
Skipped: 29 ⏭️

📊 Test Reports by Browser

✅ chromium: View Report • ✅ 431 / ❌ 0 / ⚠️ 4 / ⏭️ 29
✅ chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
✅ chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
✅ mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

christian-byrne · 2025-09-22T04:40:02Z

src/renderer/extensions/vueNodes/components/NodeHeader.vue

+    headerColor = adjustColor(nodeData.color, { lightness: -0.15 })
+  }
+
+  // Apply light theme lightening on top of base darkening (same as drawNode monkey patch)
+  if (colorPaletteStore.completedActivePalette.light_theme) {
+    headerColor = adjustColor(headerColor, { lightness: 0.5 })


Is there a better way to do this with CSS? adjustColor is memoized though.

color-mix() with white ?

Like this? https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_colors/Relative_colors

It's safe to assume that would be more performant right? Not sure performance different is measurable though, so maybe other pros/cons should be considered. Thoughts?

If we were able to bind to a CSS variable and derive from it a lighter and a darker shade, we could eliminate all of this logic

We want to support any arbitrary color set by extensions though.

It would work, the css variable can be dynamic.

TY, let me try tomorrow

The logic is a bit convulated when trying to do in CSS especially since we need to serialize color in the workflow, decided against it for this PR.

browser_tests/tests/vueNodes/nodeStates/colors.spec.ts

Myestery · 2025-09-22T05:14:09Z

src/renderer/extensions/vueNodes/components/LGraphNode.vue

        transform: `translate(${layoutPosition.x ?? position?.x ?? 0}px, ${(layoutPosition.y ?? position?.y ?? 0) - LiteGraph.NODE_TITLE_HEIGHT}px)`,
-        zIndex: zIndex
+        zIndex: zIndex,
+        // Explicitly set backgroundColor to ensure reset works properly


Suggested change

// Explicitly set backgroundColor to ensure reset works properly

color: nodeData.color || ''

Where are we setting color?

The naming in Litegraph is poor.
They're both surface colors, one for the header, one for the body, neither for the text.

Myestery · 2025-09-22T05:15:06Z

browser_tests/tests/vueNodes/nodeStates/colors.spec.ts

+      .click()
+
+    await expect(comfyPage.canvas).toHaveScreenshot(
+      'vue-node-custom-color-blue.png'


Aside from the screenshot, can we have another test that reads the color or backgroundColor prop from the node?

What additional signal would that provide?

src/lib/litegraph/src/LGraphNode.ts

…view feedback Adds CSS background-color property validation in addition to screenshot comparison to verify header and body colors are correctly applied to DOM elements. Co-authored-by: Myestery <[email protected]>

…resses review feedback Moves color and bgcolor property change tracking to the standard LGraphNodeProperties system instead of manual event firing in setColorOption. This reduces custom code and leverages the existing property instrumentation infrastructure. Co-authored-by: DrJKL <[email protected]>

The :style binding was accidentally removed, causing TypeScript error about unused headerStyle computed property. This binding is needed for Vue node header color functionality.

Now correctly implements all aspects of the LiteGraph drawNode monkey patch: 1. Header colors: Apply opacity + lightness adjustments like LiteGraph 2. Body colors: Apply same adjustments to background as LiteGraph 3. Opacity setting: Support 'Comfy.Node.Opacity' setting from user preferences 4. Light theme: Apply lightness=0.5 to both header and body in light theme This ensures Vue nodes have pixel-perfect color matching with LiteGraph nodes across all themes and opacity settings.

christian-byrne added area:customization area:vue-migration labels Sep 22, 2025

dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 22, 2025

christian-byrne commented Sep 22, 2025

View reviewed changes

browser_tests/tests/vueNodes/nodeStates/colors.spec.ts Outdated Show resolved Hide resolved

christian-byrne requested review from DrJKL, Myestery and benceruleanlu and removed request for DrJKL September 22, 2025 04:42

Myestery reviewed Sep 22, 2025

View reviewed changes

DrJKL previously approved these changes Sep 22, 2025

View reviewed changes

DrJKL reviewed Sep 23, 2025

View reviewed changes

src/lib/litegraph/src/LGraphNode.ts Outdated Show resolved Hide resolved

christian-byrne dismissed DrJKL’s stale review via d3c15b0 September 26, 2025 00:10

christian-byrne force-pushed the vue-node/fix/color-change-node branch from d3c15b0 to b4413e7 Compare September 27, 2025 17:07

christian-byrne and others added 7 commits September 27, 2025 11:21

fix vue node color change

7414b94

[fix] restore headerStyle binding in NodeHeader template

f0c3f63

The :style binding was accidentally removed, causing TypeScript error about unused headerStyle computed property. This binding is needed for Vue node header color functionality.

set w-full on node header

e83ffad

handle opacity setting

481aa82

christian-byrne force-pushed the vue-node/fix/color-change-node branch from 30e010a to 481aa82 Compare September 27, 2025 18:22

christian-byrne added the New Browser Test Expectations New browser test screenshot should be set by github action label Sep 27, 2025

github-actions and others added 3 commits September 27, 2025 19:06

Update test expectations [skip ci]

8603941

refactor

543c656

change tests from fail to normal (should work now)

de497da

christian-byrne merged commit a25d898 into main Sep 27, 2025
21 checks passed

christian-byrne deleted the vue-node/fix/color-change-node branch September 27, 2025 22:12

	// Explicitly set backgroundColor to ensure reset works properly
	color: nodeData.color \|\| ''

Allow Vue nodes to have their colors changed from selection toolbox #5720

Allow Vue nodes to have their colors changed from selection toolbox #5720

Uh oh!

Conversation

christian-byrne commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Review Focus

Technical Details

Uh oh!

github-actions bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎭 Playwright Test Results

📈 Summary

📊 Test Reports by Browser

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

christian-byrne commented Sep 22, 2025 •

edited

Loading

github-actions bot commented Sep 22, 2025 •

edited

Loading