Skip to content

Fix themeing of nodes#12712

Open
AustinMroz wants to merge 2 commits into
mainfrom
austin/blue-da-ba-de
Open

Fix themeing of nodes#12712
AustinMroz wants to merge 2 commits into
mainfrom
austin/blue-da-ba-de

Conversation

@AustinMroz

@AustinMroz AustinMroz commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

In moving styling to documentElement, #9516 introduced a regression preventing themes from styling nodes.

Before After
before after

Note: Some elements (like the app mode toggle) are themed using different variables (--secondary-background instead of --component-node-background). I think the sanest approach is to define --secondary-background to be --component-node-background by default, but that feels better handled as a followup PR

@AustinMroz AustinMroz requested a review from a team June 8, 2026 17:01
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR applies the dark-theme CSS class to the document root (<html>) instead of the body element, and adds a test that validates color palette switching updates node visual styles. The change improves theme styling scope while the test confirms palette mutations propagate to rendered components.

Changes

Theme Styling and Palette Integration

Layer / File(s) Summary
Dark theme class application target
src/views/GraphView.vue
The theme watcher now applies/removes the dark-theme class on document.documentElement rather than document.body, so dark-mode CSS variables apply at the document level.
Color palette mutation test
browser_tests/tests/colorPalette.spec.ts
New test loads a KSampler Vue node, captures its initial computed backgroundColor, switches the color palette to solarized, and polls to verify the background color changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

Possibly related PRs

  • Comfy-Org/ComfyUI_frontend#12455: The theme toggle now applies the dark-theme class to <html>, which directly affects the CSS variables used for Vue node widget styling.

Suggested labels

size:XS, core/1.44

Suggested reviewers

  • pythongosssss
  • dante01yoon

Poem

🐰 From body to root, the dark theme takes flight,
A palette of solarized hues shining bright,
Tests poll the colors with patience and care,
The document breathes with responsive flair! 🌙✨

🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description lacks the required template structure with Summary, Changes, and Review Focus sections. Reformat the description to follow the template: add a Summary section, clearly document What was changed in the Changes section, and add a Review Focus section addressing the regression and fix.
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix themeing of nodes' directly addresses the main change: restoring theme styling to nodes after a regression from moving styles to documentElement.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
End-To-End Regression Coverage For Fixes ✅ Passed PR contains bug-fix language and modifies src/; it also adds a Playwright test under browser_tests/ validating the theme fix, satisfying regression coverage requirements.
Adr Compliance For Entity/Litegraph Changes ✅ Passed No files under src/lib/litegraph/, src/ecs/, or graph entity files were modified. ADR compliance check does not apply to this PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch austin/blue-da-ba-de

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🎨 Storybook: ✅ Built — View Storybook

Details

⏰ Completed at: 06/08/2026, 05:02:54 PM UTC

Links

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🎭 Playwright: ❌ 1665 passed, 2 failed

❌ Failed Tests

📊 Browser Reports
  • chromium: View Report (✅ 1644 / ❌ 2 / ⚠️ 0 / ⏭️ 5)
  • chromium-2x: View Report (✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0)
  • chromium-0.5x: View Report (✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0)
  • mobile-chrome: View Report (✅ 18 / ❌ 0 / ⚠️ 0 / ⏭️ 0)

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/views/GraphView.vue 50.00% 1 Missing ⚠️
@@             Coverage Diff             @@
##             main   #12712       +/-   ##
===========================================
- Coverage   76.09%   61.33%   -14.77%     
===========================================
  Files        1559     1448      -111     
  Lines       95378    75006    -20372     
  Branches    25916    21170     -4746     
===========================================
- Hits        72580    46002    -26578     
- Misses      22042    28650     +6608     
+ Partials      756      354      -402     
Flag Coverage Δ
e2e ?
unit 61.33% <50.00%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/views/GraphView.vue 56.33% <50.00%> (-17.61%) ⬇️

... and 1147 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant