Skip to content

fix: layer collapsed nodes behind expanded nodes - #14156

Closed
r-vage wants to merge 2 commits into
Comfy-Org:mainfrom
r-vage:fix/collapsed-node-stacking
Closed

fix: layer collapsed nodes behind expanded nodes#14156
r-vage wants to merge 2 commits into
Comfy-Org:mainfrom
r-vage:fix/collapsed-node-stacking

Conversation

@r-vage

@r-vage r-vage commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Layer inactive collapsed nodes behind expanded nodes in the Vue node renderer, while temporarily promoting selected collapsed nodes so they remain easy to interact with.

Changes

  • What: Adds a stable three-tier DOM paint order for inactive collapsed, expanded, and selected collapsed nodes.
  • Preserves persisted z-order within each tier, including stable ties and nodes without layout data.
  • Uses keyed sibling order for rendering and pointer hits without mutating layout state or remounting nodes.
  • Keeps legacy canvas rendering unchanged.

Review Focus

  • Selected and multi-selected collapsed nodes move to the foreground tier.
  • Deselecting or changing collapse state updates the order immediately.
  • Existing bring-to-front behavior remains intact within each tier.

Testing

  • Targeted Vitest: 5 passed
  • Playwright collapsed-stacking interaction: 1 passed
  • Existing Playwright bring-to-front interactions: 2 passed
  • pnpm typecheck
  • pnpm typecheck:browser
  • Lint suite: Stylelint, Oxlint, and ESLint
  • pnpm format:check
  • pnpm knip
  • E2E regression coverage for this fix: browser_tests/tests/vueNodes/interactions/node/collapsedStacking.spec.ts

Screenshots

Before After
before
before
after
after

@r-vage
r-vage requested a review from a team July 27, 2026 21:02
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 27, 2026
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

🎨 Storybook: 🚧 Building...

🎭 Playwright: ⏳ Running...

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 71b3652d-70bf-48e1-bbc1-ea9162524dce

📥 Commits

Reviewing files that changed from the base of the PR and between d33a228 and 8bd3bd5.

📒 Files selected for processing (8)
  • browser_tests/assets/vueNodes/collapsed-stacking.json
  • browser_tests/fixtures/VueNodeHelpers.ts
  • browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
  • browser_tests/tests/vueNodes/interactions/node/collapsedStacking.spec.ts
  • src/components/graph/GraphCanvas.vue
  • src/renderer/extensions/vueNodes/components/LGraphNode.vue
  • src/renderer/extensions/vueNodes/utils/nodePaintOrder.test.ts
  • src/renderer/extensions/vueNodes/utils/nodePaintOrder.ts

📝 Walkthrough

Walkthrough

Vue node rendering now computes paint order from collapse state, selection, and layout z-index. Node components stop applying inline z-index styles, while browser helpers and E2E coverage validate overlapping and collapsed-node interactions.

Changes

Vue node stacking

Layer / File(s) Summary
Painting-order utility and tests
src/renderer/extensions/vueNodes/utils/nodePaintOrder.ts, src/renderer/extensions/vueNodes/utils/nodePaintOrder.test.ts
Adds tiered, z-index-aware node ordering with stable tie handling and unit tests for collapsed, selected, expanded, and missing-layout cases.
Layout-driven graph rendering
src/components/graph/GraphCanvas.vue, src/renderer/extensions/vueNodes/components/LGraphNode.vue
Synchronizes layout changes and selection state, renders displayedNodes in computed paint order, removes inline z-index styling, and cleans up the layout watcher.
Stacking interaction coverage
browser_tests/assets/vueNodes/collapsed-stacking.json, browser_tests/fixtures/VueNodeHelpers.ts, browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts, browser_tests/tests/vueNodes/interactions/node/collapsedStacking.spec.ts
Adds a collapsed-node workflow, shared center and paint-order helpers, updated bring-to-front assertions, and E2E coverage for overlap selection and collapse behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: christian-byrne, austinmroz

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: layering collapsed nodes behind expanded nodes.
Description check ✅ Passed The description covers Summary, Changes, Review Focus, Testing, and Screenshots and matches the template well.
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 The PR has bug-fix language, but it also adds browser_tests regression coverage and the description explicitly cites an E2E test, so the fail conditions aren’t all met.
Adr Compliance For Entity/Litegraph Changes ✅ Passed Changed files are node-renderer/test assets only; no litegraph/ECS mutations, new command APIs, or extension callbacks were added.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@r-vage

r-vage commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 21 minutes.

@r-vage

r-vage commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

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

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant