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

🕵🏻 No test results found

⏰ Completed at: 09/27/2025, 05:08:47 PM UTC

📊 Test Reports by Browser

❌ chromium: Deployment failed
❌ chromium-2x: Deployment failed
❌ chromium-0.5x: Deployment failed
❌ mobile-chrome: Deployment failed

🎉 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

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

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

+  // TODO: implement loading node colors from workflow in Vue system
+  test.fail('should load node colors from workflow', async ({ comfyPage }) => {
+    await comfyPage.loadWorkflow('nodes/every_node_color')
+    await expect(comfyPage.canvas).toHaveScreenshot(
+      'vue-node-custom-colors-dark-all-colors.png'
+    )
+  })
+
+  // TODO: implement loading node colors from workflow in Vue system
+  test.fail(


Expect these to fail until loading custom colors from workflow is implemented.

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?

DrJKL · 2025-09-23T06:42:09Z

src/lib/litegraph/src/LGraphNode.ts

+      if (oldColor !== newColor) {
+        this.graph.trigger('node:property:changed', {
+          nodeId: this.id,
+          property: 'color',
+          oldValue: oldColor,
+          newValue: newColor
+        })
+      }
+      if (oldBgcolor !== newBgcolor) {
+        this.graph.trigger('node:property:changed', {
+          nodeId: this.id,
+          property: 'bgcolor',
+          oldValue: oldBgcolor,
+          newValue: newBgcolor
+        })
+      }


I think if you add color and bgcolor here you can remove this section.

…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 <Myestery@users.noreply.github.com>

…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 <DrJKL@users.noreply.github.com>

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

christian-byrne requested review from DrJKL, benceruleanlu and Myestery 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

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

christian-byrne and others added 3 commits September 27, 2025 10:06

fix vue node color change

d4bdde6

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

christian-byrne commented Sep 22, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 22, 2025 •

edited

Loading

Uh oh!

christian-byrne Sep 22, 2025

Uh oh!

Myestery Sep 22, 2025

Uh oh!

DrJKL Sep 22, 2025

Uh oh!

christian-byrne Sep 22, 2025

Uh oh!

DrJKL Sep 22, 2025

Uh oh!

christian-byrne Sep 22, 2025

Uh oh!

DrJKL Sep 23, 2025

Uh oh!

christian-byrne Sep 23, 2025

Uh oh!

christian-byrne Sep 22, 2025

Uh oh!

Myestery Sep 22, 2025

Uh oh!

DrJKL Sep 22, 2025

Uh oh!

Myestery Sep 22, 2025

Uh oh!

DrJKL Sep 22, 2025

Uh oh!

DrJKL Sep 23, 2025

Uh oh!

Uh oh!

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

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

Are you sure you want to change the base?

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

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

📊 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!

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!

christian-byrne commented Sep 22, 2025 •

edited

Loading

github-actions bot commented Sep 22, 2025 •

edited

Loading