fix: selection bounding box not encompassing node footer overlay#10640
Open
fix: selection bounding box not encompassing node footer overlay#10640
Conversation
Extend node boundingRect via onBounding to include footer overlay height (32px expanded, 34px collapsed) and override collapsed width to MIN_NODE_WIDTH since Vue nodes lack canvas ctx for measure(). Regression from #9360 which changed footer from inside-node to overlay approach (absolute top-full).
dosubot
bot
added
the
size:L
Contributor
📝 WalkthroughWalkthroughAdds a JSON test asset and Playwright test for verifying canvas selection bounding boxes, introduces helper methods to position and collapse nodes in browser tests, and updates the Vue LGraphNode component to adjust LiteGraph node bounding via a reactive watchEffect when footers or collapsed state apply. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Playwright Test
participant App as window.app
participant Node as LGraphNode (Vue)
participant Canvas as Renderer/DOM
Test->>App: load graph JSON (with fixed positions)
App->>Node: instantiate/render node components
Note right of Node: watchEffect installs/updates node.onBounding
Node->>Canvas: provide adjusted bounding during layout pass
App->>Canvas: render nodes with updated bounds
Test->>Canvas: trigger selection (Ctrl+A) and read DOM rects, ds.scale, ds.offset
Test->>App: compute selection bounding box from canvas items
Test->>Test: assert selection bounds contain node visual bounds
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🎭 Playwright: ✅ 744 passed, 0 failed · 4 flaky📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
📦 Bundle: 5.09 MB gzip 🟢 -75 BDetailsSummary
Category Glance App Entry Points — 22.3 kB (baseline 22.3 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.14 MB (baseline 1.14 MB) • 🔴 +633 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 76.6 kB (baseline 76.6 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 2 unchanged Panels & Settings — 484 kB (baseline 484 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed / 12 unchanged User & Accounts — 17.1 kB (baseline 17.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed / 2 unchanged Editors & Dialogs — 109 kB (baseline 109 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 60.3 kB (baseline 60.3 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 8 unchanged Data & Services — 2.96 MB (baseline 2.96 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed / 4 unchanged Utilities & Hooks — 334 kB (baseline 334 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 13 added / 13 removed / 12 unchanged Vendor & Third-Party — 9.8 MB (baseline 9.8 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 8.43 MB (baseline 8.43 MB) • ⚪ 0 BBundles that do not match a named category
Status: 55 added / 55 removed / 79 unchanged ⚡ Performance Report
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-03-28T09:44:30.714Z",
"gitSha": "2ed65a1bb6f4e3c7d3a00f4a72565b670cf954da",
"branch": "fix/selection-bounding-box-footer-overlay",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 1999.4509999999934,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.081999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 405.97099999999995,
"heapDeltaBytes": 20838132,
"heapUsedBytes": 64988488,
"domNodes": 20,
"jsHeapTotalBytes": 22806528,
"scriptDurationMs": 24.274000000000004,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.680000000000017
},
{
"name": "canvas-idle",
"durationMs": 2012.943000000007,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.857,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 425.798,
"heapDeltaBytes": 20019652,
"heapUsedBytes": 63032084,
"domNodes": 19,
"jsHeapTotalBytes": 22806528,
"scriptDurationMs": 24.970999999999997,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "canvas-idle",
"durationMs": 2033.0350000000408,
"styleRecalcs": 12,
"styleRecalcDurationMs": 11.869,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 445.90399999999994,
"heapDeltaBytes": 20125408,
"heapUsedBytes": 64317724,
"domNodes": 23,
"jsHeapTotalBytes": 23068672,
"scriptDurationMs": 29.357000000000003,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000073
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1817.766000000006,
"styleRecalcs": 75,
"styleRecalcDurationMs": 37.271,
"layouts": 12,
"layoutDurationMs": 3.979999999999999,
"taskDurationMs": 800.6560000000001,
"heapDeltaBytes": 15796068,
"heapUsedBytes": 58347308,
"domNodes": 58,
"jsHeapTotalBytes": 23068672,
"scriptDurationMs": 142.87999999999997,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1804.0659999999775,
"styleRecalcs": 72,
"styleRecalcDurationMs": 37.795,
"layouts": 12,
"layoutDurationMs": 3.92,
"taskDurationMs": 802.2119999999999,
"heapDeltaBytes": 16338596,
"heapUsedBytes": 59317652,
"domNodes": 53,
"jsHeapTotalBytes": 22544384,
"scriptDurationMs": 133.89800000000002,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2017.5930000000335,
"styleRecalcs": 82,
"styleRecalcDurationMs": 44.869,
"layouts": 12,
"layoutDurationMs": 3.4799999999999995,
"taskDurationMs": 1006.411,
"heapDeltaBytes": 15862240,
"heapUsedBytes": 59568200,
"domNodes": 68,
"jsHeapTotalBytes": 23330816,
"scriptDurationMs": 142.536,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1750.267000000008,
"styleRecalcs": 31,
"styleRecalcDurationMs": 26.104999999999997,
"layouts": 6,
"layoutDurationMs": 0.7309999999999999,
"taskDurationMs": 345.49600000000004,
"heapDeltaBytes": 24650156,
"heapUsedBytes": 66987364,
"domNodes": 80,
"jsHeapTotalBytes": 19660800,
"scriptDurationMs": 31.088000000000005,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1724.775999999963,
"styleRecalcs": 31,
"styleRecalcDurationMs": 20.326999999999998,
"layouts": 6,
"layoutDurationMs": 0.743,
"taskDurationMs": 357.809,
"heapDeltaBytes": 24820680,
"heapUsedBytes": 67584844,
"domNodes": 80,
"jsHeapTotalBytes": 20971520,
"scriptDurationMs": 32.338,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1738.1199999999808,
"styleRecalcs": 32,
"styleRecalcDurationMs": 20.650000000000002,
"layouts": 6,
"layoutDurationMs": 0.735,
"taskDurationMs": 376.126,
"heapDeltaBytes": 24615376,
"heapUsedBytes": 67388132,
"domNodes": 79,
"jsHeapTotalBytes": 20447232,
"scriptDurationMs": 36.032000000000004,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "dom-widget-clipping",
"durationMs": 595.1739999999859,
"styleRecalcs": 13,
"styleRecalcDurationMs": 9.344000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 361.813,
"heapDeltaBytes": 6194896,
"heapUsedBytes": 48918916,
"domNodes": 22,
"jsHeapTotalBytes": 13631488,
"scriptDurationMs": 67.427,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "dom-widget-clipping",
"durationMs": 559.1039999999907,
"styleRecalcs": 13,
"styleRecalcDurationMs": 9.213,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 346.35900000000004,
"heapDeltaBytes": 6864736,
"heapUsedBytes": 49261480,
"domNodes": 22,
"jsHeapTotalBytes": 12845056,
"scriptDurationMs": 66.509,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "dom-widget-clipping",
"durationMs": 589.1079999998965,
"styleRecalcs": 13,
"styleRecalcDurationMs": 10.890999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 376.865,
"heapDeltaBytes": -2158000,
"heapUsedBytes": 48768776,
"domNodes": 21,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 70.494,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "large-graph-idle",
"durationMs": 2037.4889999999937,
"styleRecalcs": 11,
"styleRecalcDurationMs": 10.454999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 613.399,
"heapDeltaBytes": 19949464,
"heapUsedBytes": 71555948,
"domNodes": -254,
"jsHeapTotalBytes": 15462400,
"scriptDurationMs": 110.43700000000001,
"eventListeners": -125,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000073
},
{
"name": "large-graph-idle",
"durationMs": 2031.926999999996,
"styleRecalcs": 11,
"styleRecalcDurationMs": 12.164000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 624.5959999999999,
"heapDeltaBytes": 4327564,
"heapUsedBytes": 54851328,
"domNodes": -258,
"jsHeapTotalBytes": 15405056,
"scriptDurationMs": 114.758,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "large-graph-idle",
"durationMs": 2047.2469999999703,
"styleRecalcs": 9,
"styleRecalcDurationMs": 9.815999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 694.7700000000001,
"heapDeltaBytes": 3774524,
"heapUsedBytes": 56151868,
"domNodes": -261,
"jsHeapTotalBytes": 16191488,
"scriptDurationMs": 123.18199999999999,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "large-graph-pan",
"durationMs": 2149.7130000000197,
"styleRecalcs": 69,
"styleRecalcDurationMs": 16.177,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1189.1979999999999,
"heapDeltaBytes": 17303648,
"heapUsedBytes": 69137824,
"domNodes": -258,
"jsHeapTotalBytes": 19017728,
"scriptDurationMs": 434.168,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "large-graph-pan",
"durationMs": 2144.9340000000348,
"styleRecalcs": 68,
"styleRecalcDurationMs": 15.806999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1177.6740000000002,
"heapDeltaBytes": 6764916,
"heapUsedBytes": 62890248,
"domNodes": -261,
"jsHeapTotalBytes": 19517440,
"scriptDurationMs": 414.933,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000073
},
{
"name": "large-graph-pan",
"durationMs": 2191.833000000088,
"styleRecalcs": 68,
"styleRecalcDurationMs": 16.854999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1250.638,
"heapDeltaBytes": 15348564,
"heapUsedBytes": 68771484,
"domNodes": -263,
"jsHeapTotalBytes": 17784832,
"scriptDurationMs": 448.171,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "large-graph-zoom",
"durationMs": 3257.4670000000197,
"styleRecalcs": 65,
"styleRecalcDurationMs": 17.166,
"layouts": 60,
"layoutDurationMs": 7.646,
"taskDurationMs": 1430.272,
"heapDeltaBytes": 9841020,
"heapUsedBytes": 64895732,
"domNodes": -266,
"jsHeapTotalBytes": 15929344,
"scriptDurationMs": 530.339,
"eventListeners": -123,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "large-graph-zoom",
"durationMs": 3170.5069999999864,
"styleRecalcs": 66,
"styleRecalcDurationMs": 17.547,
"layouts": 60,
"layoutDurationMs": 7.562,
"taskDurationMs": 1414.8100000000002,
"heapDeltaBytes": 12634540,
"heapUsedBytes": 68539056,
"domNodes": -264,
"jsHeapTotalBytes": 15986688,
"scriptDurationMs": 511.76200000000006,
"eventListeners": -125,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.659999999999947
},
{
"name": "large-graph-zoom",
"durationMs": 3216.1009999999806,
"styleRecalcs": 64,
"styleRecalcDurationMs": 16.429,
"layouts": 60,
"layoutDurationMs": 7.841000000000001,
"taskDurationMs": 1475.05,
"heapDeltaBytes": 9491388,
"heapUsedBytes": 64520468,
"domNodes": -269,
"jsHeapTotalBytes": 15929344,
"scriptDurationMs": 548.217,
"eventListeners": -123,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000073
},
{
"name": "minimap-idle",
"durationMs": 2014.4640000000322,
"styleRecalcs": 9,
"styleRecalcDurationMs": 9.485000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 598.7040000000001,
"heapDeltaBytes": 2025684,
"heapUsedBytes": 55258212,
"domNodes": -264,
"jsHeapTotalBytes": 15929344,
"scriptDurationMs": 107.30600000000001,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.659999999999947
},
{
"name": "minimap-idle",
"durationMs": 2041.2089999999807,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.405999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 641.0519999999999,
"heapDeltaBytes": 4670836,
"heapUsedBytes": 57653616,
"domNodes": -258,
"jsHeapTotalBytes": 15437824,
"scriptDurationMs": 118.438,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "minimap-idle",
"durationMs": 2065.303999999969,
"styleRecalcs": 11,
"styleRecalcDurationMs": 11.754000000000005,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 715.815,
"heapDeltaBytes": 4705724,
"heapUsedBytes": 57428044,
"domNodes": -260,
"jsHeapTotalBytes": 15405056,
"scriptDurationMs": 128.189,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 577.7820000000133,
"styleRecalcs": 48,
"styleRecalcDurationMs": 16.951,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 376.073,
"heapDeltaBytes": 6558352,
"heapUsedBytes": 49320956,
"domNodes": 22,
"jsHeapTotalBytes": 13631488,
"scriptDurationMs": 125.31899999999999,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.65999999999999
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 575.1859999999738,
"styleRecalcs": 48,
"styleRecalcDurationMs": 12.040000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 380.422,
"heapDeltaBytes": 6499420,
"heapUsedBytes": 49505272,
"domNodes": 22,
"jsHeapTotalBytes": 13369344,
"scriptDurationMs": 128.46500000000003,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000027
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 633.0769999999575,
"styleRecalcs": 49,
"styleRecalcDurationMs": 13.565,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 404.245,
"heapDeltaBytes": 6754540,
"heapUsedBytes": 49353732,
"domNodes": 23,
"jsHeapTotalBytes": 13107200,
"scriptDurationMs": 132.27,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000027
},
{
"name": "subgraph-idle",
"durationMs": 2002.1490000000028,
"styleRecalcs": 12,
"styleRecalcDurationMs": 12.078,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 401.372,
"heapDeltaBytes": 19933420,
"heapUsedBytes": 62608212,
"domNodes": 23,
"jsHeapTotalBytes": 22806528,
"scriptDurationMs": 25.958000000000002,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "subgraph-idle",
"durationMs": 1997.3790000000236,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.118,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 418.90100000000007,
"heapDeltaBytes": 11085072,
"heapUsedBytes": 62315996,
"domNodes": 20,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 24.804,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
},
{
"name": "subgraph-idle",
"durationMs": 2019.0099999999802,
"styleRecalcs": 11,
"styleRecalcDurationMs": 9.771000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 456.268,
"heapDeltaBytes": 10642512,
"heapUsedBytes": 62319344,
"domNodes": 19,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 24.808000000000003,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1973.875000000021,
"styleRecalcs": 84,
"styleRecalcDurationMs": 54.378,
"layouts": 16,
"layoutDurationMs": 5.5809999999999995,
"taskDurationMs": 1029.903,
"heapDeltaBytes": 12247744,
"heapUsedBytes": 55189780,
"domNodes": 72,
"jsHeapTotalBytes": 23068672,
"scriptDurationMs": 112.646,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1700.538999999992,
"styleRecalcs": 76,
"styleRecalcDurationMs": 39.506,
"layouts": 16,
"layoutDurationMs": 4.984999999999999,
"taskDurationMs": 749.937,
"heapDeltaBytes": 2194720,
"heapUsedBytes": 53926272,
"domNodes": 63,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 109.271,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000073
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1692.6650000000336,
"styleRecalcs": 76,
"styleRecalcDurationMs": 39.907,
"layouts": 16,
"layoutDurationMs": 4.295999999999999,
"taskDurationMs": 743.9739999999999,
"heapDeltaBytes": 11204336,
"heapUsedBytes": 54198320,
"domNodes": 62,
"jsHeapTotalBytes": 23592960,
"scriptDurationMs": 109.41199999999999,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000036
},
{
"name": "vue-large-graph-idle",
"durationMs": 12472.185000000025,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12456.855000000001,
"heapDeltaBytes": -35834472,
"heapUsedBytes": 158070360,
"domNodes": -8331,
"jsHeapTotalBytes": 16867328,
"scriptDurationMs": 623.749,
"eventListeners": -16462,
"totalBlockingTimeMs": 0,
"frameDurationMs": 18.339999999999783
},
{
"name": "vue-large-graph-idle",
"durationMs": 12520.179999999982,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12505.845000000001,
"heapDeltaBytes": -30883376,
"heapUsedBytes": 165208668,
"domNodes": -8331,
"jsHeapTotalBytes": 27353088,
"scriptDurationMs": 612.842,
"eventListeners": -16466,
"totalBlockingTimeMs": 0,
"frameDurationMs": 18.340000000000146
},
{
"name": "vue-large-graph-idle",
"durationMs": 12740.56900000005,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12725.999000000002,
"heapDeltaBytes": -30241956,
"heapUsedBytes": 166058484,
"domNodes": -8331,
"jsHeapTotalBytes": 27615232,
"scriptDurationMs": 612.8919999999999,
"eventListeners": -16466,
"totalBlockingTimeMs": 0,
"frameDurationMs": 18.340000000000146
},
{
"name": "vue-large-graph-pan",
"durationMs": 14790.38300000002,
"styleRecalcs": 69,
"styleRecalcDurationMs": 17.586999999999964,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 14765.925000000003,
"heapDeltaBytes": -46075116,
"heapUsedBytes": 148084656,
"domNodes": -8331,
"jsHeapTotalBytes": -3317760,
"scriptDurationMs": 863.8199999999999,
"eventListeners": -16492,
"totalBlockingTimeMs": 50,
"frameDurationMs": 18.329999999999927
},
{
"name": "vue-large-graph-pan",
"durationMs": 14498.443000000008,
"styleRecalcs": 64,
"styleRecalcDurationMs": 14.803999999999984,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 14472.999999999998,
"heapDeltaBytes": -21833848,
"heapUsedBytes": 173322804,
"domNodes": -8331,
"jsHeapTotalBytes": 23597056,
"scriptDurationMs": 873.0400000000001,
"eventListeners": -16458,
"totalBlockingTimeMs": 0,
"frameDurationMs": 18.329999999999927
},
{
"name": "vue-large-graph-pan",
"durationMs": 14954.773999999928,
"styleRecalcs": 69,
"styleRecalcDurationMs": 16.139999999999986,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 14933.820000000002,
"heapDeltaBytes": -32985188,
"heapUsedBytes": 160595792,
"domNodes": -8331,
"jsHeapTotalBytes": -3403776,
"scriptDurationMs": 969.144,
"eventListeners": -16456,
"totalBlockingTimeMs": 0,
"frameDurationMs": 19.990000000000144
},
{
"name": "workflow-execution",
"durationMs": 462.35699999999724,
"styleRecalcs": 18,
"styleRecalcDurationMs": 28.2,
"layouts": 5,
"layoutDurationMs": 1.451,
"taskDurationMs": 140.88500000000005,
"heapDeltaBytes": 4714704,
"heapUsedBytes": 49232840,
"domNodes": 169,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 32.37,
"eventListeners": 71,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.65999999999999
},
{
"name": "workflow-execution",
"durationMs": 451.9040000000132,
"styleRecalcs": 15,
"styleRecalcDurationMs": 21.019000000000002,
"layouts": 4,
"layoutDurationMs": 1.272,
"taskDurationMs": 114.17700000000004,
"heapDeltaBytes": 4412760,
"heapUsedBytes": 49020812,
"domNodes": 150,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 25.673,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000027
},
{
"name": "workflow-execution",
"durationMs": 461.0600000000886,
"styleRecalcs": 19,
"styleRecalcDurationMs": 27.252000000000002,
"layouts": 5,
"layoutDurationMs": 1.312,
"taskDurationMs": 132.823,
"heapDeltaBytes": 4489856,
"heapUsedBytes": 48470592,
"domNodes": 158,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 32.044000000000004,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998
}
]
} |
![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: 2
🤖 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/assets/selection/subgraph-with-regular-node.json`:
- Line 36: The node size in the test asset JSON currently uses "size": [315,
106], which is below the required minimum for browser tests; update the "size"
property in subgraph-with-regular-node.json (the object with "size": [315, 106])
to at least [400, 200] to conform to the guideline for
browser_tests/assets/**/*.json and prevent navigateIntoSubgraph timeouts.
In `@src/renderer/extensions/vueNodes/components/LGraphNode.vue`:
- Around line 786-809: The current watchEffect replaces node.onBounding and
clears it on cleanup, which clobbers other callbacks; instead capture the
existing node.onBounding (e.g., prevOnBounding = node.onBounding) inside the
watchEffect, set node.onBounding to a composed function that calls the existing
prevOnBounding(out) first (if present) and then applies this component's
footer/collapsed adjustments (use hasFooter.value, isCollapsed.value,
FOOTER_BOUNDING_OFFSET[_COLLAPSED], MIN_NODE_WIDTH), and in onCleanup restore
node.onBounding back to prevOnBounding; ensure the else branch still sets
node.onBounding to prevOnBounding (not undefined) so you don't drop other
extensions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0dbc84e9-9bf9-4e7d-91b4-0b38b5833d7a
📒 Files selected for processing (3)
browser_tests/assets/selection/subgraph-with-regular-node.jsonbrowser_tests/tests/selectionBoundingBox.spec.tssrc/renderer/extensions/vueNodes/components/LGraphNode.vue
Capture and call any pre-existing node.onBounding before applying footer/collapsed adjustments, and restore it on cleanup.
jaeone94
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
github-actions
bot
removed
the
New Browser Test Expectations
jaeone94
added
needs-backport
Contributor
There was a problem hiding this comment.
Should we add some of these to shared/centralized fixtures/helpers/page objects?
Collaborator
Author
There was a problem hiding this comment.
Done !
Replaced waitForSelectedCount and waitForNodeLayout with existing fixture helpers (nodeOps.getSelectedGraphNodesCount via expect.poll, vueNodes.getNodeLocator.waitFor).
Extracted loadWithPositions and setCollapsed to shared fixtures:
NodeOperationsHelper.loadWithPositions— serialize-modify-reload pattern for placing nodes at specific positions without separate asset filesNodeReference.setCollapsed(collapsed)— deterministic setter usingnode.collapse()API with current state check (vstoggleCollapse()which depends on current state)
The remaining measureBounds is kept as a local helper — it combines multi-selection bounds with DOM visual bounds including footer measurement, too specialized to justify sharing.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
browser_tests/tests/selectionBoundingBox.spec.ts (1)
66-68: Use centralized TestIds instead of inlinedata-testidliterals.Please source these IDs from
browser_tests/fixtures/selectors.tsand pass them intopage.evaluateparams, rather than hardcoding string literals in the selector.Suggested refactor
+import { TestIds } from '../fixtures/selectors' ... - const footerEls = nodeEl.querySelectorAll( - '[data-testid="subgraph-enter-button"], [data-testid="node-footer"]' - ) + const footerEls = nodeEl.querySelectorAll( + `[data-testid="${subgraphEnterButtonTestId}"], [data-testid="${nodeFooterTestId}"]` + ) ... - { ids: nodeIds, padding: SELECTION_PADDING } + { + ids: nodeIds, + padding: SELECTION_PADDING, + subgraphEnterButtonTestId: TestIds.subgraph.enterButton, + nodeFooterTestId: TestIds.nodes.footer + } ) as Promise<MeasureResult>As per coding guidelines, "Use centralized TestIds from
fixtures/selectors.tsfor DOM element selection in E2E tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/tests/selectionBoundingBox.spec.ts` around lines 66 - 68, Replace the inline data-testid literals in the selector used to build footerEls with centralized TestId values from browser_tests/fixtures/selectors.ts: import the relevant constants (e.g., SUBGRAPH_ENTER_BUTTON and NODE_FOOTER) and pass them into page.evaluate as arguments, then inside the page.evaluate callback build the selector using those params and call nodeEl.querySelectorAll with the composed selector (instead of hardcoded strings); update any references to footerEls accordingly so the test uses the selectors module values rather than inline literals.
🤖 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/selectionBoundingBox.spec.ts`:
- Around line 110-124: The helper setNodeCollapsed currently mutates
node.flags.collapsed directly which skips the graph version bump; instead,
inside the page.evaluate callback call the node's public collapse API
(LGraphNode.collapse) to toggle collapse state so the graph._version++ and
canvas dirtying behavior run; update the evaluate closure in setNodeCollapsed to
find the node by id and invoke node.collapse() or node.collapse(true/false) as
appropriate rather than setting node.flags.collapsed, ensuring graph versioning
and canvas updates happen correctly.
---
Nitpick comments:
In `@browser_tests/tests/selectionBoundingBox.spec.ts`:
- Around line 66-68: Replace the inline data-testid literals in the selector
used to build footerEls with centralized TestId values from
browser_tests/fixtures/selectors.ts: import the relevant constants (e.g.,
SUBGRAPH_ENTER_BUTTON and NODE_FOOTER) and pass them into page.evaluate as
arguments, then inside the page.evaluate callback build the selector using those
params and call nodeEl.querySelectorAll with the composed selector (instead of
hardcoded strings); update any references to footerEls accordingly so the test
uses the selectors module values rather than inline literals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c4c4b001-28e2-4246-ac18-8699ba40bcb4
⛔ Files ignored due to path filters (1)
browser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts-snapshots/vue-node-multiple-promoted-previews-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (1)
browser_tests/tests/selectionBoundingBox.spec.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
browser_tests/tests/selectionBoundingBox.spec.ts (1)
110-124:⚠️ Potential issue | 🟠 MajorUse the node collapse API instead of mutating
flags.collapseddirectly.At Line 121–123, directly assigning
node.flags.collapsedcan skip graph/node lifecycle side effects and make state/version updates less reliable. Prefer toggling through the node API only when the current state differs from the target state.Suggested fix
async function setNodeCollapsed( comfyPage: ComfyPage, nodeId: string, collapsed: boolean ) { await comfyPage.page.evaluate( ({ id, collapsed }) => { const node = window.app!.graph._nodes.find( (n: { id: number | string }) => String(n.id) === id ) if (node) { - node.flags = node.flags || {} - node.flags.collapsed = collapsed - window.app!.canvas.setDirty(true, true) + const isCollapsed = Boolean(node.flags?.collapsed) + if (isCollapsed !== collapsed) { + node.collapse() + } } }, { id: nodeId, collapsed } ) await comfyPage.vueNodes.getNodeLocator(nodeId).waitFor() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/tests/selectionBoundingBox.spec.ts` around lines 110 - 124, The helper setNodeCollapsed currently mutates node.flags.collapsed directly inside comfyPage.page.evaluate (finding nodes via window.app!.graph._nodes and then setting node.flags.collapsed and calling window.app!.canvas.setDirty), which bypasses node lifecycle side effects; instead, use the node collapse API on the node object (e.g., call the provided node.toggleCollapse / node.setCollapsed / graph API) and only invoke it if the node's current collapsed state differs from the desired collapsed parameter, then let the API handle state/version updates rather than manually setting flags or calling canvas.setDirty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@browser_tests/tests/selectionBoundingBox.spec.ts`:
- Around line 110-124: The helper setNodeCollapsed currently mutates
node.flags.collapsed directly inside comfyPage.page.evaluate (finding nodes via
window.app!.graph._nodes and then setting node.flags.collapsed and calling
window.app!.canvas.setDirty), which bypasses node lifecycle side effects;
instead, use the node collapse API on the node object (e.g., call the provided
node.toggleCollapse / node.setCollapsed / graph API) and only invoke it if the
node's current collapsed state differs from the desired collapsed parameter,
then let the API handle state/version updates rather than manually setting flags
or calling canvas.setDirty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 082c42fa-a351-4320-824b-bda647bcd47f
📒 Files selected for processing (1)
browser_tests/tests/selectionBoundingBox.spec.ts
- Add NodeOperationsHelper.loadWithPositions for reloading workflow with modified node positions - Add NodeReference.setCollapsed using node.collapse() API with current state check for deterministic set (not toggle) - Remove local helper duplicates from test file
Collaborator
Author
|
@coderabbitai can you approve it ? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. The PR description images show the corrected multi-select bounding box behavior: a dashed selection rectangle now fully encompasses both the "Test Subgraph" node (including its "Enter Subgraph" footer overlay at the bottom) and the "Empty Latent Image" node. Both images confirm the fix is working — the selection bounding box correctly extends to include the footer overlays that were previously excluded due to the regression introduced in PR
[approve] 🧠 Learnings used |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
browser_tests/tests/selectionBoundingBox.spec.ts (1)
63-65: Avoid hardcodeddata-testidliterals in spec selectors.Line 63–Line 65 uses raw test-id strings. Prefer pulling these from the shared selector source so test ids stay refactor-safe.
As per coding guidelines: "Use centralized TestIds from
fixtures/selectors.tsfor DOM element selection in E2E tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/tests/selectionBoundingBox.spec.ts` around lines 63 - 65, Replace the hardcoded test-id strings in the nodeEl.querySelectorAll call with the centralized TestIds from fixtures/selectors.ts: import the TestIds (or TEST_IDS) export and change the selector to use `[data-testid="${TestIds.SUBGRAPH_ENTER_BUTTON}"], [data-testid="${TestIds.NODE_FOOTER}"]` (keeping the variable footerEls and nodeEl.querySelectorAll as-is) so E2E selectors come from the shared source and remain refactor-safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@browser_tests/tests/selectionBoundingBox.spec.ts`:
- Around line 63-65: Replace the hardcoded test-id strings in the
nodeEl.querySelectorAll call with the centralized TestIds from
fixtures/selectors.ts: import the TestIds (or TEST_IDS) export and change the
selector to use `[data-testid="${TestIds.SUBGRAPH_ENTER_BUTTON}"],
[data-testid="${TestIds.NODE_FOOTER}"]` (keeping the variable footerEls and
nodeEl.querySelectorAll as-is) so E2E selectors come from the shared source and
remain refactor-safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 127596d5-6d81-495d-accd-00497bd40603
📒 Files selected for processing (3)
browser_tests/fixtures/helpers/NodeOperationsHelper.tsbrowser_tests/fixtures/utils/litegraphUtils.tsbrowser_tests/tests/selectionBoundingBox.spec.ts
Contributor
There was a problem hiding this comment.
Looks good to me generally. For the actual bugfix, I asked about some other possible ways to think about the solution. The good thing is, since you already made some nice working tests, you can potentially try a few different new approaches and still have the tests to easily validate they also fix the problem
Comment on lines
+121
to
+133
| async ({ positions }) => { | ||
| const data = window.app!.graph.serialize() | ||
| for (const node of data.nodes) { | ||
| const pos = positions[String(node.id)] | ||
| if (pos) node.pos = pos | ||
| } | ||
| await window.app!.loadGraphData( | ||
| data as ComfyWorkflowJSON, | ||
| true, | ||
| true, | ||
| null | ||
| ) | ||
| }, |
Contributor
There was a problem hiding this comment.
nit: I think a better method name could be explored -- this is more about changing the node positions in batch rather than loading a graph (although it does use loadGraphData). It can also possibly be decomposed into smaller pure functions.
Comment on lines
+335
to
+344
| async setCollapsed(collapsed: boolean) { | ||
| await this.comfyPage.page.evaluate( | ||
| ([id, collapsed]) => { | ||
| const node = window.app!.canvas.graph!.getNodeById(id) | ||
| if (!node) throw new Error('Node not found') | ||
| if (node.collapsed !== collapsed) node.collapse(true) | ||
| }, | ||
| [this.id, collapsed] as const | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
nit: In an ideal world, we can do this with user actions (e.g., find the collapse button and click it). Although for litegraph/canvas, it might be too much effort here.
Contributor
There was a problem hiding this comment.
I think most things outside the test.describe can actually be moved to a helper/utils file still
Comment on lines
+786
to
+810
| watchEffect((onCleanup) => { | ||
| const node = lgraphNode.value | ||
| if (!node) return | ||
|
|
||
| const needsFooterOffset = hasFooter.value | ||
| const collapsed = isCollapsed.value | ||
| const previousOnBounding = node.onBounding | ||
|
|
||
| if (needsFooterOffset || collapsed) { | ||
| node.onBounding = function (out) { | ||
| previousOnBounding?.call(this, out) | ||
|
|
||
| if (needsFooterOffset) | ||
| out[3] += collapsed | ||
| ? FOOTER_BOUNDING_OFFSET_COLLAPSED | ||
| : FOOTER_BOUNDING_OFFSET | ||
|
|
||
| // Must match CSS --min-node-width in the template. | ||
| if (collapsed) out[2] = MIN_NODE_WIDTH | ||
| } | ||
| } | ||
|
|
||
| onCleanup(() => { | ||
| node.onBounding = previousOnBounding | ||
| }) |
Contributor
There was a problem hiding this comment.
I would like to fully explore two other options, as I am not sure how maintainable this pattern is
- Make the footer part of normal document flow (not absolute-positioned overflow) so getBoundingClientRect() naturally includes it → ResizeObserver picks it up → litegraph gets the right size automatically. No onBounding hack needed.
- Adjust the ResizeObserver measurement in useVueNodeResizeTracking.ts to account for footer overflow (similar to how it already normalizes for NODE_TITLE_HEIGHT).
The current approach reaches across the boundary from the Vue layer into litegraph's internal callback system, hard-coding magic pixel offsets (32px, 34px) that must stay in sync with CSS. That's potentially quite fragile and the also sort of the kind of coupling the Vue node architecture was designed to avoid.
Collaborator
Author
There was a problem hiding this comment.
I've addressed the test code feedback and explored both approaches you mentioned. Here are two PRs for comparison:
- Option A (refactor: [Option A] inline node footer layout with core _footerHeight/_collapsedHeight #10711): Inline footer layout with core
_footerHeight/_collapsedHeightproperties onLGraphNodeand avueNodesModeguard inmeasure(). NoonBoundingusage, but adds Vue-specific fields to the core. - Option B (refactor: [Option B] inline node footer layout with onBounding + vueBoundsOverrides #10712): Same inline footer layout, but uses
onBoundingcallback reading from avueBoundsOverridesMap in the Vue layer. Zero core changes, but relies ononBounding(with DOM-measured values instead of hardcoded offsets, and proper callback chaining).
Both PRs share the same foundation: footer moved into normal document flow, border/outline unified on root element, resize handles split for footer awareness, and the same E2E test suite.
Would appreciate your thoughts on which direction is preferable, or if there's a different approach worth exploring.
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
4 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
Fix multi-select bounding box not encompassing node footer overlays (Enter Subgraph, Advanced, Error buttons).
Changes
boundingRectviaonBoundingto include footer overlay height (32px expanded, 34px collapsed) and override collapsed width toMIN_NODE_WIDTHsince Vue nodes lack canvas ctx formeasure()Review Focus
onBoundinguses an existing litegraph extension point, not adding new methodsMIN_NODE_WIDTHmust stay in sync with CSS--min-node-width(documented in comment)absolute top-full)Screenshots
Before
After
┆Issue is synchronized with this Notion page by Unito