[style] Update error/subgraph node footer design with layered overlay approach#9360
Merged
christian-byrne merged 26 commits intomainfrom Mar 7, 2026
Merged
[style] Update error/subgraph node footer design with layered overlay approach#9360christian-byrne merged 26 commits intomainfrom
christian-byrne merged 26 commits intomainfrom
Conversation
jaeone94
added
enhancement
dosubot
bot
added
the
size:XL
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 553 passed, 0 failed · 3 flaky📊 Browser Reports
|
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (25)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new NodeFooter component and integrates overlay-driven selection/execution outlines in LGraphNode; updates unit and browser tests to locator-based selectors and new outline classes; adjusts enter-subgraph click positioning; removes Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LGraphNode
participant NodeFooter
participant GraphCanvas
User->>LGraphNode: click / select / drag
LGraphNode->>NodeFooter: render(props: isSubgraph, hasAnyError, shape, headerColor)
NodeFooter->>LGraphNode: emit enterSubgraph / openErrors / toggleAdvanced
LGraphNode->>GraphCanvas: update selection/execution state / navigate
GraphCanvas->>LGraphNode: notify execution/selection changes
LGraphNode->>NodeFooter: update overlays / outline classes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
⚡ Performance Report
Raw data{
"timestamp": "2026-03-07T01:15:52.362Z",
"gitSha": "74b0d0722562a243e0ab2891edf404d23c5f53a7",
"branch": "style/subgraph-error-node-design",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2017.0609999999556,
"styleRecalcs": 121,
"styleRecalcDurationMs": 15.827,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 360.076,
"heapDeltaBytes": -2473228
},
{
"name": "canvas-idle",
"durationMs": 2020.2370000000087,
"styleRecalcs": 123,
"styleRecalcDurationMs": 19.544999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 382.14300000000003,
"heapDeltaBytes": -2616296
},
{
"name": "canvas-idle",
"durationMs": 2011.3570000000323,
"styleRecalcs": 123,
"styleRecalcDurationMs": 18.238,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 381.333,
"heapDeltaBytes": -599784
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2041.5849999999978,
"styleRecalcs": 183,
"styleRecalcDurationMs": 51.504999999999995,
"layouts": 12,
"layoutDurationMs": 3.5719999999999996,
"taskDurationMs": 1003.3050000000001,
"heapDeltaBytes": -809712
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1904.291999999998,
"styleRecalcs": 170,
"styleRecalcDurationMs": 47.87,
"layouts": 12,
"layoutDurationMs": 3.165,
"taskDurationMs": 839.605,
"heapDeltaBytes": -1200000
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2024.19900000001,
"styleRecalcs": 180,
"styleRecalcDurationMs": 47.992,
"layouts": 12,
"layoutDurationMs": 2.902,
"taskDurationMs": 951.1800000000002,
"heapDeltaBytes": -795016
},
{
"name": "dom-widget-clipping",
"durationMs": 557.4970000000121,
"styleRecalcs": 42,
"styleRecalcDurationMs": 17.668000000000003,
"layouts": 1,
"layoutDurationMs": 0.29100000000000004,
"taskDurationMs": 359.24999999999994,
"heapDeltaBytes": 7547160
},
{
"name": "dom-widget-clipping",
"durationMs": 570.2739999999835,
"styleRecalcs": 44,
"styleRecalcDurationMs": 15.024000000000003,
"layouts": 1,
"layoutDurationMs": 0.201,
"taskDurationMs": 357.352,
"heapDeltaBytes": 8546632
},
{
"name": "dom-widget-clipping",
"durationMs": 572.6129999999898,
"styleRecalcs": 44,
"styleRecalcDurationMs": 16.061,
"layouts": 1,
"layoutDurationMs": 0.1730000000000001,
"taskDurationMs": 358.34700000000004,
"heapDeltaBytes": 8457296
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 574.4779999999992,
"styleRecalcs": 72,
"styleRecalcDurationMs": 13.783999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 391.276,
"heapDeltaBytes": -7497644
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 576.5799999999786,
"styleRecalcs": 72,
"styleRecalcDurationMs": 13.801,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 398.393,
"heapDeltaBytes": -7292304
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 565.0029999999902,
"styleRecalcs": 72,
"styleRecalcDurationMs": 14.434,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 392.604,
"heapDeltaBytes": 15961128
},
{
"name": "subgraph-idle",
"durationMs": 1998.4299999999848,
"styleRecalcs": 121,
"styleRecalcDurationMs": 18.566,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 358.452,
"heapDeltaBytes": -2861636
},
{
"name": "subgraph-idle",
"durationMs": 1991.0899999999856,
"styleRecalcs": 120,
"styleRecalcDurationMs": 19.144000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 364.54200000000003,
"heapDeltaBytes": -2783776
},
{
"name": "subgraph-idle",
"durationMs": 2007.0259999999962,
"styleRecalcs": 123,
"styleRecalcDurationMs": 21.996000000000002,
"layouts": 1,
"layoutDurationMs": 0.27099999999999996,
"taskDurationMs": 383.729,
"heapDeltaBytes": -822592
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1980.9359999999856,
"styleRecalcs": 172,
"styleRecalcDurationMs": 56.249,
"layouts": 16,
"layoutDurationMs": 4.515,
"taskDurationMs": 976.06,
"heapDeltaBytes": -4296632
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1989.9859999999876,
"styleRecalcs": 172,
"styleRecalcDurationMs": 52.689,
"layouts": 16,
"layoutDurationMs": 4.062,
"taskDurationMs": 940.8159999999999,
"heapDeltaBytes": -3139064
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1714.7079999999733,
"styleRecalcs": 155,
"styleRecalcDurationMs": 45.088,
"layouts": 16,
"layoutDurationMs": 4.179,
"taskDurationMs": 714.681,
"heapDeltaBytes": -5058772
}
]
} |
![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
🧹 Nitpick comments (3)
src/renderer/extensions/vueNodes/components/LGraphNode.vue (1)
185-187: HoistuseRightSidePanelStore()out of the template handler.Calling
useRightSidePanelStore()inline in the template hides the dependency and makes the event path harder to test. Prefer a setup-scoped store + named handler.♻️ Proposed refactor
- `@open-errors`="useRightSidePanelStore().openPanel('errors')" + `@open-errors`="handleOpenErrors"const rightSidePanelStore = useRightSidePanelStore() function handleOpenErrors() { rightSidePanelStore.openPanel('errors') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/vueNodes/components/LGraphNode.vue` around lines 185 - 187, Hoist the inline call to useRightSidePanelStore() out of the template by creating a setup-scoped variable (e.g., rightSidePanelStore = useRightSidePanelStore()) and a named handler (e.g., handleOpenErrors()) that calls rightSidePanelStore.openPanel('errors'); then replace the template binding `@open-errors`="useRightSidePanelStore().openPanel('errors')" with `@open-errors`="handleOpenErrors" so the dependency and event path are explicit and testable (ensure handleOpenErrors is returned from setup).src/renderer/extensions/vueNodes/components/NodeFooter.vue (2)
113-118: Use template$twhen i18n is template-only.
useI18nis only used to exposetfor template text. Prefer$tdirectly in template and remove the script import/destructure to reduce setup surface.Based on learnings: In Vue single-file components where the i18n
tfunction is only used within the template, prefer using built-in$tin the template instead of importinguseI18n.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/vueNodes/components/NodeFooter.vue` around lines 113 - 118, The code imports useI18n and destructures t in NodeFooter.vue but t is only used in the template; remove the import and the const { t } = useI18n() from the script setup, and change any template usages of t(...) to the global $t(...) call instead (keep component imports like Button, RenderShape, cn intact); this reduces setup surface by relying on Vue's built-in $t in the template rather than the useI18n symbol.
139-150: Follow the static string pattern from LGraphNode.vue for consistency.Lines 147 and 149 use runtime template literals (
rounded-br-${radius}) that make intent less clear. LGraphNode.vue (lines 573–585) handles the same logic with explicit static strings in a conditional, which is clearer and matches the codebase convention. Refactor to:const footerRadiusClass = computed(() => { const isExpanded = props.hasAnyError switch (props.shape) { case RenderShape.BOX: return 'rounded-none' case RenderShape.CARD: return isExpanded ? 'rounded-bl-none rounded-br-[20px]' : 'rounded-bl-none rounded-br-2xl' default: return isExpanded ? 'rounded-b-[20px]' : 'rounded-b-2xl' } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/vueNodes/components/NodeFooter.vue` around lines 139 - 150, Refactor the computed footerRadiusClass to avoid runtime template literals and follow the static string pattern used in LGraphNode.vue: remove the radius variable and use props.hasAnyError to choose between explicit static classes for each RenderShape case (for RenderShape.CARD return either 'rounded-bl-none rounded-br-[20px]' or 'rounded-bl-none rounded-br-2xl'; for default return either 'rounded-b-[20px]' or 'rounded-b-2xl'), keeping the RenderShape.BOX branch returning 'rounded-none' and leaving the computed name footerRadiusClass and props.shape/props.hasAnyError checks intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/extensions/vueNodes/components/LGraphNode.test.ts`:
- Around line 204-225: The tests in LGraphNode.test.ts are asserting Tailwind
utility classes (e.g., 'outline-node-component-outline',
'border-node-component-outline', 'border-3', 'outline-node-stroke-executing',
'border-node-stroke-executing') which couples behavior tests to styling; update
the two test cases that set mockData.mockExecuting and mountLGraphNode({
nodeData: mockNodeData }) to assert semantic behavior instead — for example
check for a data-state or aria attribute on the root node, or assert that the
progress indicator component/element exists (e.g., find by test id or component
name) and that the node's executing flag is reflected in emitted events or DOM
attributes, rather than checking Tailwind utility classes; reference the wrapper
returned by mountLGraphNode, mockNodeData/mockExecuting, and the overlay query
so you replace those class-based expects with attribute/component-based expects.
---
Nitpick comments:
In `@src/renderer/extensions/vueNodes/components/LGraphNode.vue`:
- Around line 185-187: Hoist the inline call to useRightSidePanelStore() out of
the template by creating a setup-scoped variable (e.g., rightSidePanelStore =
useRightSidePanelStore()) and a named handler (e.g., handleOpenErrors()) that
calls rightSidePanelStore.openPanel('errors'); then replace the template binding
`@open-errors`="useRightSidePanelStore().openPanel('errors')" with
`@open-errors`="handleOpenErrors" so the dependency and event path are explicit
and testable (ensure handleOpenErrors is returned from setup).
In `@src/renderer/extensions/vueNodes/components/NodeFooter.vue`:
- Around line 113-118: The code imports useI18n and destructures t in
NodeFooter.vue but t is only used in the template; remove the import and the
const { t } = useI18n() from the script setup, and change any template usages of
t(...) to the global $t(...) call instead (keep component imports like Button,
RenderShape, cn intact); this reduces setup surface by relying on Vue's built-in
$t in the template rather than the useI18n symbol.
- Around line 139-150: Refactor the computed footerRadiusClass to avoid runtime
template literals and follow the static string pattern used in LGraphNode.vue:
remove the radius variable and use props.hasAnyError to choose between explicit
static classes for each RenderShape case (for RenderShape.CARD return either
'rounded-bl-none rounded-br-[20px]' or 'rounded-bl-none rounded-br-2xl'; for
default return either 'rounded-b-[20px]' or 'rounded-b-2xl'), keeping the
RenderShape.BOX branch returning 'rounded-none' and leaving the computed name
footerRadiusClass and props.shape/props.hasAnyError checks intact.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/renderer/extensions/vueNodes/components/LGraphNode.test.tssrc/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/vueNodes/components/NodeFooter.vuesrc/utils/colorUtil.ts
💤 Files with no reviewable changes (1)
- src/utils/colorUtil.ts
📦 Bundle: 4.53 MB gzip 🔴 +1.13 kBDetailsSummary
Category Glance App Entry Points — 28.9 kB (baseline 28.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 947 kB (baseline 940 kB) • 🔴 +6.62 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.4 kB (baseline 72.4 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 436 kB (baseline 436 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 — 76.6 kB (baseline 76.6 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 50.9 kB (baseline 50.9 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.76 MB (baseline 2.76 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 57.3 kB (baseline 57.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.88 MB (baseline 8.88 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.93 MB (baseline 7.93 MB) • ⚪ 0 BBundles that do not match a named category
|
jaeone94
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
github-actions
bot
removed
the
New Browser Test Expectations
- Add pointer-events-auto to NodeFooter tab buttons so Playwright can reliably click absolutely-positioned footer elements - Update error.spec.ts: target inner wrapper div and check ring-destructive-background instead of root div border-node-stroke-error - Update bypass.spec.ts: target inner wrapper div for before:bg-bypass/60 class check, reflecting the class moving from root to inner div
dosubot
bot
added
size:M
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
browser_tests/tests/vueNodes/nodeStates/error.spec.ts (1)
21-24:⚠️ Potential issue | 🟠 MajorSame selector brittleness issue as bypass tests—please centralize.
This repeats the
[data-node-id]+nth-child(2)pattern, which is brittle under DOM changes and should be replaced with centralized TestIds/shared selector helpers.As per coding guidelines, "Use the centralized TestIds from
browser_tests/fixtures/selectors.tsfor test element selection."Also applies to: 35-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/tests/vueNodes/nodeStates/error.spec.ts` around lines 21 - 24, The locator construction for unknownNode (and the similar selector at lines 35-38) uses a brittle '[data-node-id]' + '> div:nth-child(2)' pattern; replace these ad-hoc selectors with the centralized TestIds/selector helpers from browser_tests/fixtures/selectors.ts (e.g., use the exported test id constant for UNKNOWN_NODE and the helper locator function provided by comfyPage or the selectors module) so the test references a stable test id instead of positional child selectors; update the unknownNode assignment to use the selectors.ts constant/helper and do the same for the other occurrence.
🧹 Nitpick comments (2)
src/renderer/extensions/vueNodes/components/NodeFooter.vue (2)
113-119: Optional: use template$twhen translations are template-only.
useI18nis only used to exposetfor template bindings; this can be simplified by using$tdirectly in the template and removing the script wiring.Based on learnings, in Vue components where translation is used only in template, prefer template
$tover importinguseI18n.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/vueNodes/components/NodeFooter.vue` around lines 113 - 119, The component imports and calls useI18n and destructures t at module-level (useI18n, t in NodeFooter.vue) only for template translations; remove the useI18n import and the const { t } = useI18n() line and update the template to call $t(...) instead of t(...) so the component uses the global template helper, reducing script wiring and avoiding unnecessary composition API usage.
84-87: Make advanced footer radius shape-aware for consistency.The advanced footer hardcodes
rounded-b-2xl, while other footer cases derive radius fromshapeviafooterRadiusClass. This can produce inconsistent corners for non-default shapes.♻️ Proposed refactor
- <div - v-else-if="showAdvancedInputsButton || showAdvancedState" - class="flex w-full h-7 rounded-b-2xl -z-1 text-xs rounded-t-none overflow-hidden divide-x divide-component-node-border relative -mt-5" - > + <div + v-else-if="showAdvancedInputsButton || showAdvancedState" + :class=" + cn( + 'flex w-full h-7 -z-1 text-xs rounded-t-none overflow-hidden divide-x divide-component-node-border relative -mt-5', + footerRadiusClass + ) + " + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/vueNodes/components/NodeFooter.vue` around lines 84 - 87, The advanced footer div is using a hardcoded rounded-b-2xl which can conflict with shape-specific radii; update the element rendered when v-else-if="showAdvancedInputsButton || showAdvancedState" to use the computed footerRadiusClass (same one other footer branches use) instead of the static rounded-b-2xl so the corner radius honors the node shape; locate the div in NodeFooter.vue containing that v-else-if and replace the hardcoded class with footerRadiusClass while preserving the other classes (flex w-full h-7 ... divide-x divide-component-node-border relative -mt-5).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts`:
- Around line 25-29: The locator uses fragile DOM-indexed traversal (the '>
div:nth-child(2)' chain) to find the "Load Checkpoint" node; replace this with
the centralized TestIds selector from browser_tests/fixtures/selectors.ts and
the shared helper used for node selection so tests don't depend on child
ordering: stop using the inline locator chain that produces checkpointNode and
instead call the shared helper (the project’s node-selection helper) with the
TestId for nodes and the visible label "Load Checkpoint", then assert that the
returned element has class BYPASS_CLASS (keep BYPASS_CLASS as-is); apply the
same replacement for the duplicate case around lines 46-53.
---
Duplicate comments:
In `@browser_tests/tests/vueNodes/nodeStates/error.spec.ts`:
- Around line 21-24: The locator construction for unknownNode (and the similar
selector at lines 35-38) uses a brittle '[data-node-id]' + '> div:nth-child(2)'
pattern; replace these ad-hoc selectors with the centralized TestIds/selector
helpers from browser_tests/fixtures/selectors.ts (e.g., use the exported test id
constant for UNKNOWN_NODE and the helper locator function provided by comfyPage
or the selectors module) so the test references a stable test id instead of
positional child selectors; update the unknownNode assignment to use the
selectors.ts constant/helper and do the same for the other occurrence.
---
Nitpick comments:
In `@src/renderer/extensions/vueNodes/components/NodeFooter.vue`:
- Around line 113-119: The component imports and calls useI18n and destructures
t at module-level (useI18n, t in NodeFooter.vue) only for template translations;
remove the useI18n import and the const { t } = useI18n() line and update the
template to call $t(...) instead of t(...) so the component uses the global
template helper, reducing script wiring and avoiding unnecessary composition API
usage.
- Around line 84-87: The advanced footer div is using a hardcoded rounded-b-2xl
which can conflict with shape-specific radii; update the element rendered when
v-else-if="showAdvancedInputsButton || showAdvancedState" to use the computed
footerRadiusClass (same one other footer branches use) instead of the static
rounded-b-2xl so the corner radius honors the node shape; locate the div in
NodeFooter.vue containing that v-else-if and replace the hardcoded class with
footerRadiusClass while preserving the other classes (flex w-full h-7 ...
divide-x divide-component-node-border relative -mt-5).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 18e3803a-a0f8-453f-b6dd-7ce8d7364346
⛔ Files ignored due to path filters (28)
browser_tests/tests/interaction.spec.ts-snapshots/dragged-node1-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/loadWorkflowInMedia.spec.ts-snapshots/no-workflow-webp-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/nodeSearchBox.spec.ts-snapshots/added-node-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/nodeSearchBox.spec.ts-snapshots/added-node-no-connection-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/saveImageAndWebp.spec.ts-snapshots/save-image-and-webm-preview-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-create-group-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-fit-to-contents-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts-snapshots/vue-nodes-paned-with-touch-mobile-chrome-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/canvas/zoom.spec.ts-snapshots/zoomed-in-ctrl-shift-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-dragging-link-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-ctrl-alt-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-reuses-origin-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-input-drag-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-output-shift-drag-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-shift-output-multi-link-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-node-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-slot-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts-snapshots/bring-to-front-overlapped-after-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts-snapshots/bring-to-front-overlapped-before-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts-snapshots/bring-to-front-widget-overlapped-after-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts-snapshots/bring-to-front-widget-overlapped-before-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts-snapshots/vue-node-multiple-promoted-previews-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-color-blue-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-dark-all-colors-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-light-all-colors-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/nodeStates/mute.spec.ts-snapshots/vue-node-muted-state-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (3)
browser_tests/tests/vueNodes/nodeStates/bypass.spec.tsbrowser_tests/tests/vueNodes/nodeStates/error.spec.tssrc/renderer/extensions/vueNodes/components/NodeFooter.vue
- Add data-testid='node-inner-wrapper' to LGraphNode's main content div
to enable stable targeting regardless of selection state
- Update bypass.spec.ts: replace fragile > div:nth-child(2) with
getByTestId('node-inner-wrapper') to handle DOM order change when
node is selected (SelectionOutline div is inserted before the wrapper)
- Update error.spec.ts: replace nth-child(2) with getByTestId and update
ERROR_CLASS to ring-destructive-background (moved from root to inner div)
- Use { force: true } in VueNodeHelpers.enterSubgraph() to bypass
Playwright hit-test interception from overlapping node body div
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
browser_tests/fixtures/VueNodeHelpers.ts (1)
175-175: Removeforce: truefrom the sharedenterSubgraphhelper to catch visibility regressions.The subgraph enter button should be clickable without forcing actionability checks. Shared test helpers should be strict to avoid masking real issues. The force click currently applies to every
enterSubgraphcall (used in 2 tests), which can hide overlay or visibility regressions. If specific test scenarios require force, apply it at the call site instead.Proposed change
async enterSubgraph(nodeId?: string): Promise<void> { const locator = nodeId ? this.getNodeLocator(nodeId) : this.page const editButton = locator.getByTestId(TestIds.widgets.subgraphEnterButton) - await editButton.click({ force: true }) + const target = editButton.first() + await target.waitFor({ state: 'visible' }) + await target.click() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/fixtures/VueNodeHelpers.ts` at line 175, The shared helper enterSubgraph currently forces clicks and can mask visibility regressions; in the enterSubgraph helper remove the { force: true } option from the await editButton.click(...) call so the click goes through normal actionability/visibility checks, and if any specific test truly needs a forced click, apply { force: true } at that test's enterSubgraph call site instead; locate the editButton.click invocation inside the enterSubgraph function and delete the force option to make the helper strict.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/extensions/vueNodes/components/LGraphNode.vue`:
- Around line 576-602: rootBorderShapeClass and selectionShapeClass only handle
RenderShape.BOX and RenderShape.CARD and rely on the default for other
RenderShape variants, causing overlay geometry mismatches; update both computed
properties (rootBorderShapeClass and selectionShapeClass in LGraphNode.vue) to
include explicit cases for RenderShape.ROUND, RenderShape.CIRCLE,
RenderShape.HollowCircle, RenderShape.ARROW, and RenderShape.GRID, returning the
correct tailorable class strings that match the canvas rendering logic used
elsewhere (e.g., the hollow-circle and circle radii/rounding used by
litegraphService.ts and default ROUND behavior), so each shape has a
deterministic overlay class instead of falling back to the generic default.
---
Nitpick comments:
In `@browser_tests/fixtures/VueNodeHelpers.ts`:
- Line 175: The shared helper enterSubgraph currently forces clicks and can mask
visibility regressions; in the enterSubgraph helper remove the { force: true }
option from the await editButton.click(...) call so the click goes through
normal actionability/visibility checks, and if any specific test truly needs a
forced click, apply { force: true } at that test's enterSubgraph call site
instead; locate the editButton.click invocation inside the enterSubgraph
function and delete the force option to make the helper strict.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00bc9f7e-3ec2-4773-b1b4-11ba0b54108d
📒 Files selected for processing (4)
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/nodeStates/bypass.spec.tsbrowser_tests/tests/vueNodes/nodeStates/error.spec.tssrc/renderer/extensions/vueNodes/components/LGraphNode.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts
- browser_tests/tests/vueNodes/nodeStates/error.spec.ts
jaeone94
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
…h main layout size
jaeone94
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
dosubot
bot
added
size:M
AustinMroz
previously approved these changes
Mar 6, 2026
Collaborator
AustinMroz
left a comment
AustinMroz
left a comment
There was a problem hiding this comment.
Looks like test failures are flakes
viva-jinyi
previously approved these changes
Mar 6, 2026
dosubot
bot
added
size:XL
jaeone94
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
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
5 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
Refactors the error and subgraph node footer UI by extracting a dedicated
NodeFootercomponent and replacing the CSSoutlineapproach with a layered border overlay for selection/executing state indicators.Changes
NodeFooter.vuefromLGraphNode.vueto encapsulate the footer tab logic (subgraph enter, error, advanced inputs). Replaced CSSoutlinewith an absolutely-positioned border overlay div for selection and executing state. Added a separate root border overlay div for the node body border. Removed unusedisTransparentfunction fromcolorUtil.ts.Review Focus
absolute -inset-[3px] border-3) for selection/executing outlines vs the previousoutline-3approach — ensures the outline renders outside the node bounds correctly including the footer areaNodeFooterhandles 4 cases: subgraph+error (dual tabs), error only, subgraph only, advanced inputs — verify edge cases render correctlyhasFooter)Screenshots
┆Issue is synchronized with this Notion page by Unito