feat: accept bboxes input and add grid snapping to Create Bounding Boxes#13376
Open
jtydhr88 wants to merge 1 commit into
Open
feat: accept bboxes input and add grid snapping to Create Bounding Boxes#13376jtydhr88 wants to merge 1 commit into
jtydhr88 wants to merge 1 commit into
Conversation
dosubot
Bot
added
the
size:L
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds grid snapping and overlay rendering to the bounding boxes editor, syncs grid state with node properties, applies incoming bbox inputs from connected node outputs, and refactors palette swatches to use a slot-based color picker trigger. ChangesBounding boxes grid and incoming input handling
Palette swatch color picker flow
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ 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 |
🎭 Playwright: ✅ 1693 passed, 0 failed · 1 flaky📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 7.77 MB gzip 🔴 +1.11 kBDetailsSummary
Category Glance App Entry Points — 47.4 kB (baseline 47.4 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.26 MB (baseline 1.26 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 98 kB (baseline 98 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 3 unchanged Panels & Settings — 546 kB (baseline 546 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 16 unchanged User & Accounts — 26.9 kB (baseline 26.9 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 117 kB (baseline 117 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed / 1 unchanged UI Components — 57.5 kB (baseline 57.5 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 8 unchanged Data & Services — 270 kB (baseline 270 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed / 3 unchanged Utilities & Hooks — 3.37 MB (baseline 3.37 MB) • 🔴 +25 BHelpers, composables, and utility bundles
Status: 19 added / 19 removed / 14 unchanged Vendor & Third-Party — 15.3 MB (baseline 15.3 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 11.7 MB (baseline 11.7 MB) • 🔴 +4.17 kBBundles that do not match a named category
Status: 79 added / 79 removed / 86 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-07-07T03:13:08.467Z",
"gitSha": "98b25d0f340ab93f6971f01e76e5a2f3a4537d93",
"branch": "feat/create-bounding-boxes-input-grid",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2038.1759999999645,
"styleRecalcs": 9,
"styleRecalcDurationMs": 10.200000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 511.954,
"heapDeltaBytes": -7249432,
"heapUsedBytes": 62140224,
"domNodes": -283,
"jsHeapTotalBytes": 20074496,
"scriptDurationMs": 26.028999999999996,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2082.883000000038,
"styleRecalcs": 8,
"styleRecalcDurationMs": 10.134,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 494.1070000000001,
"heapDeltaBytes": -7907336,
"heapUsedBytes": 61581516,
"domNodes": -285,
"jsHeapTotalBytes": 19288064,
"scriptDurationMs": 21.884000000000004,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2141.9219999999655,
"styleRecalcs": 77,
"styleRecalcDurationMs": 46.029,
"layouts": 12,
"layoutDurationMs": 4.111999999999999,
"taskDurationMs": 1070.179,
"heapDeltaBytes": -12794104,
"heapUsedBytes": 56458876,
"domNodes": -246,
"jsHeapTotalBytes": 20074496,
"scriptDurationMs": 150.598,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1924.8220000000629,
"styleRecalcs": 76,
"styleRecalcDurationMs": 45.826,
"layouts": 12,
"layoutDurationMs": 3.685,
"taskDurationMs": 892.671,
"heapDeltaBytes": -12649088,
"heapUsedBytes": 56878088,
"domNodes": -238,
"jsHeapTotalBytes": 19025920,
"scriptDurationMs": 129.14100000000002,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1773.3739999999898,
"styleRecalcs": 32,
"styleRecalcDurationMs": 19.71,
"layouts": 6,
"layoutDurationMs": 0.7869999999999999,
"taskDurationMs": 396.529,
"heapDeltaBytes": 62576,
"heapUsedBytes": 52371504,
"domNodes": -242,
"jsHeapTotalBytes": 7991296,
"scriptDurationMs": 21.842999999999996,
"eventListeners": -186,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1774.505999999974,
"styleRecalcs": 32,
"styleRecalcDurationMs": 20.589,
"layouts": 6,
"layoutDurationMs": 0.9110000000000001,
"taskDurationMs": 444.346,
"heapDeltaBytes": -3560656,
"heapUsedBytes": 65868916,
"domNodes": -224,
"jsHeapTotalBytes": 13258752,
"scriptDurationMs": 29.411999999999995,
"eventListeners": -184,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 716.8619999999919,
"styleRecalcs": 8,
"styleRecalcDurationMs": 11.402999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 440.31,
"heapDeltaBytes": -1302648,
"heapUsedBytes": 50413504,
"domNodes": -279,
"jsHeapTotalBytes": -135168,
"scriptDurationMs": 64.638,
"eventListeners": -201,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 743.2199999999511,
"styleRecalcs": 12,
"styleRecalcDurationMs": 14.008,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 468.635,
"heapDeltaBytes": -23408244,
"heapUsedBytes": 46107968,
"domNodes": -279,
"jsHeapTotalBytes": 6705152,
"scriptDurationMs": 66.25200000000001,
"eventListeners": -201,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-idle",
"durationMs": 2071.228000000019,
"styleRecalcs": 9,
"styleRecalcDurationMs": 9.546,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 575.25,
"heapDeltaBytes": -25150348,
"heapUsedBytes": 58933468,
"domNodes": -272,
"jsHeapTotalBytes": 1650688,
"scriptDurationMs": 92.533,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2024.8900000000276,
"styleRecalcs": 11,
"styleRecalcDurationMs": 11.440000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 659.5439999999999,
"heapDeltaBytes": -9724940,
"heapUsedBytes": 62027604,
"domNodes": 22,
"jsHeapTotalBytes": 11153408,
"scriptDurationMs": 111.53000000000002,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-pan",
"durationMs": 2325.5060000000185,
"styleRecalcs": 68,
"styleRecalcDurationMs": 18.865000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1259.1029999999998,
"heapDeltaBytes": 3007172,
"heapUsedBytes": 56332280,
"domNodes": -274,
"jsHeapTotalBytes": 1310720,
"scriptDurationMs": 464.888,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2205.8620000000246,
"styleRecalcs": 70,
"styleRecalcDurationMs": 20.919,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1267.97,
"heapDeltaBytes": 14372620,
"heapUsedBytes": 82415180,
"domNodes": -272,
"jsHeapTotalBytes": -1114112,
"scriptDurationMs": 431.675,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-zoom",
"durationMs": 3597.0080000000166,
"styleRecalcs": 65,
"styleRecalcDurationMs": 24.099,
"layouts": 60,
"layoutDurationMs": 10.860000000000001,
"taskDurationMs": 1569.9830000000002,
"heapDeltaBytes": 20848280,
"heapUsedBytes": 75965400,
"domNodes": 12,
"jsHeapTotalBytes": 6815744,
"scriptDurationMs": 600.479,
"eventListeners": 10,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-zoom",
"durationMs": 3368.445000000065,
"styleRecalcs": 61,
"styleRecalcDurationMs": 16.151000000000003,
"layouts": 60,
"layoutDurationMs": 8.435,
"taskDurationMs": 1442.815,
"heapDeltaBytes": 15623644,
"heapUsedBytes": 70577200,
"domNodes": 2,
"jsHeapTotalBytes": 2621440,
"scriptDurationMs": 552.851,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2037.5080000000025,
"styleRecalcs": 5,
"styleRecalcDurationMs": 5.822000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 595.039,
"heapDeltaBytes": -9742996,
"heapUsedBytes": 59843020,
"domNodes": -277,
"jsHeapTotalBytes": -2105344,
"scriptDurationMs": 106.17599999999999,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "minimap-idle",
"durationMs": 2004.417999999987,
"styleRecalcs": 5,
"styleRecalcDurationMs": 4.915000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 678.814,
"heapDeltaBytes": 7572944,
"heapUsedBytes": 59959160,
"domNodes": -279,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 120.167,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 670.771000000002,
"styleRecalcs": 47,
"styleRecalcDurationMs": 12.881,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 453.32599999999996,
"heapDeltaBytes": -23167532,
"heapUsedBytes": 46330208,
"domNodes": -282,
"jsHeapTotalBytes": 6443008,
"scriptDurationMs": 129.98499999999999,
"eventListeners": -195,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 660.3550000000951,
"styleRecalcs": 48,
"styleRecalcDurationMs": 14.759000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 417.53499999999997,
"heapDeltaBytes": 8120056,
"heapUsedBytes": 67051808,
"domNodes": 22,
"jsHeapTotalBytes": 19660800,
"scriptDurationMs": 133.31199999999998,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2021.523000000002,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.417,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 419.52799999999996,
"heapDeltaBytes": -5258052,
"heapUsedBytes": 47055936,
"domNodes": -289,
"jsHeapTotalBytes": 16904192,
"scriptDurationMs": 15.110000000000001,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2000.423000000069,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.057999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 411.432,
"heapDeltaBytes": 978112,
"heapUsedBytes": 58544212,
"domNodes": 20,
"jsHeapTotalBytes": 18612224,
"scriptDurationMs": 17.352,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1748.3870000000366,
"styleRecalcs": 77,
"styleRecalcDurationMs": 47.382,
"layouts": 16,
"layoutDurationMs": 4.957000000000001,
"taskDurationMs": 761.9990000000001,
"heapDeltaBytes": -16372372,
"heapUsedBytes": 53163068,
"domNodes": -233,
"jsHeapTotalBytes": 20074496,
"scriptDurationMs": 99.91299999999998,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1754.7029999999495,
"styleRecalcs": 77,
"styleRecalcDurationMs": 44.202000000000005,
"layouts": 16,
"layoutDurationMs": 4.8260000000000005,
"taskDurationMs": 803.307,
"heapDeltaBytes": 15766952,
"heapUsedBytes": 68441468,
"domNodes": -274,
"jsHeapTotalBytes": 13758464,
"scriptDurationMs": 112.076,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-transition-enter",
"durationMs": 1413.9999999999873,
"styleRecalcs": 17,
"styleRecalcDurationMs": 32.175999999999995,
"layouts": 13,
"layoutDurationMs": 14.447999999999999,
"taskDurationMs": 929.6669999999998,
"heapDeltaBytes": 4494564,
"heapUsedBytes": 76637464,
"domNodes": 13673,
"jsHeapTotalBytes": 15990784,
"scriptDurationMs": 42.623000000000005,
"eventListeners": 2533,
"totalBlockingTimeMs": 161,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8647.199,
"styleRecalcs": 251,
"styleRecalcDurationMs": 70.809,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4409.976000000001,
"heapDeltaBytes": 13361268,
"heapUsedBytes": 73705580,
"domNodes": -264,
"jsHeapTotalBytes": 4386816,
"scriptDurationMs": 1395.411,
"eventListeners": -128,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8502.884999999991,
"styleRecalcs": 251,
"styleRecalcDurationMs": 62.92100000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4431.941,
"heapDeltaBytes": 1884056,
"heapUsedBytes": 69551620,
"domNodes": -271,
"jsHeapTotalBytes": 458752,
"scriptDurationMs": 1399.452,
"eventListeners": -181,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 13144.108000000017,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13128.617000000002,
"heapDeltaBytes": -33911312,
"heapUsedBytes": 169386504,
"domNodes": -3300,
"jsHeapTotalBytes": -3436544,
"scriptDurationMs": 564.0010000000001,
"eventListeners": -16382,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 13586.30800000003,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13554.484,
"heapDeltaBytes": -27837596,
"heapUsedBytes": 166420568,
"domNodes": -3300,
"jsHeapTotalBytes": 4194304,
"scriptDurationMs": 577.339,
"eventListeners": -16368,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 16668.127000000026,
"styleRecalcs": 92,
"styleRecalcDurationMs": 21.364999999999966,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16625.791,
"heapDeltaBytes": -38800332,
"heapUsedBytes": 170465628,
"domNodes": -3300,
"jsHeapTotalBytes": -7454720,
"scriptDurationMs": 847.484,
"eventListeners": -16372,
"totalBlockingTimeMs": 43,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 16567.886000000042,
"styleRecalcs": 104,
"styleRecalcDurationMs": 23.002000000000024,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16519.742000000002,
"heapDeltaBytes": -53316540,
"heapUsedBytes": 167920592,
"domNodes": -3300,
"jsHeapTotalBytes": 6815744,
"scriptDurationMs": 845.195,
"eventListeners": -16372,
"totalBlockingTimeMs": 9,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 466.5059999999812,
"styleRecalcs": 17,
"styleRecalcDurationMs": 36.997,
"layouts": 5,
"layoutDurationMs": 1.597,
"taskDurationMs": 238.99200000000002,
"heapDeltaBytes": -22267984,
"heapUsedBytes": 48403792,
"domNodes": -160,
"jsHeapTotalBytes": 4345856,
"scriptDurationMs": 18.073000000000004,
"eventListeners": -134,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66666666666665,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 510.00999999996566,
"styleRecalcs": 16,
"styleRecalcDurationMs": 27.398999999999997,
"layouts": 5,
"layoutDurationMs": 1.5799999999999998,
"taskDurationMs": 194.80299999999997,
"heapDeltaBytes": -6686744,
"heapUsedBytes": 46453608,
"domNodes": -280,
"jsHeapTotalBytes": 651264,
"scriptDurationMs": 19.760999999999996,
"eventListeners": -177,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
}
]
} |
![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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/boundingBoxes/WidgetBoundingBoxes.vue`:
- Line 175: The Vue UI code is importing PrimeVue’s ToggleSwitch directly
instead of using an internal wrapper. Update WidgetBoundingBoxes.vue to use an
existing toggle-switch wrapper from the local UI components if one already
exists, or create one under the UI components layer and import that instead of
primevue/toggleswitch. Keep the change aligned with the existing wrapper pattern
used by ToggleGroup and ToggleGroupItem so the component reference stays within
the app’s UI abstraction.
In `@src/composables/boundingBoxes/useBoundingBoxes.test.ts`:
- Around line 20-30: The incoming-bboxes tests only verify the initial mount
path and miss the reactive watch flow in useBoundingBoxes. Update the
`@/stores/nodeOutputStore` mock so nodeOutputs is a real ref that can change when
outputState.outputs changes, and ensure nodePreviewImages stays wired similarly
if needed. Add or adjust a test around useBoundingBoxes/applyIncomingBoxes that
mounts first, then updates outputs afterward, and asserts the incoming boxes UI
reacts to the later change instead of only the synchronous setup state.
In `@src/composables/boundingBoxes/useBoundingBoxes.ts`:
- Around line 162-173: In drawDots, the canvas state is being mutated by setting
fillStyle without restoring it, which can leak grid-specific state into later
drawing operations. Update drawDots in useBoundingBoxes.ts to bracket the
drawing logic with ctx.save() and ctx.restore(), keeping the existing grid
drawing and arc loop inside that scoped block so any canvas state changes remain
local to drawDots.
- Around line 701-704: The `fromBoundingBoxes()` update path in
`useBoundingBoxes` is skipping valid empty `input_bboxes` payloads and deduping
only on the raw boxes, so stale regions can remain and changes in
`widthValue`/`heightValue` can be ignored. Update the guard around `incoming` so
an empty array is still processed/clears state, expand the `lastIncoming` cache
key to include the current dimensions, and make the width/height watcher
retrigger the same bbox sync path when the inputs are connected.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 756fb64b-3115-4a1e-bb3c-64713036d438
📒 Files selected for processing (4)
src/components/boundingBoxes/WidgetBoundingBoxes.vuesrc/composables/boundingBoxes/useBoundingBoxes.test.tssrc/composables/boundingBoxes/useBoundingBoxes.tssrc/locales/en/main.json
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #13376 +/- ##
==========================================
+ Coverage 78.75% 78.78% +0.03%
==========================================
Files 1648 1648
Lines 111917 112000 +83
Branches 40503 41178 +675
==========================================
+ Hits 88140 88239 +99
+ Misses 22906 22891 -15
+ Partials 871 870 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes 🚀 New features to boost your workflow:
|
jtydhr88
force-pushed
the
feat/create-bounding-boxes-input-grid
branch
from
2622669 to
4225f9b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/composables/boundingBoxes/useBoundingBoxes.ts (1)
665-696: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winEmpty incoming payload is silently dropped, and cache key ignores canvas dimensions.
This is the same unresolved issue flagged in a prior review (Data Integrity & Integration, Major):
if (!incoming?.length) returnmeans an upstream[](e.g., all boxes deleted upstream) never clears stale local regions. Alsokey = JSON.stringify(incoming)dedupes only on the raw boxes, so whenwidthValue/heightValuechange (whichfromBoundingBoxesdepends on to compute normalized coordinates), the identical incoming array is skipped as a "duplicate" and regions aren't rebuilt with the new dimensions. Additionally,watch([widthValue, heightValue], () => syncState())at line 596 never re-invokesapplyIncomingBoxes(), so a dimension change whilebboxesis connected won't resync from the source of truth.🐛 Proposed fix
const incoming = outputs?.input_bboxes - if (!incoming?.length) return - const key = JSON.stringify(incoming) + if (!Array.isArray(incoming)) return + const key = JSON.stringify({ + incoming, + width: widthValue.value, + height: heightValue.value + }) if (key === lastIncoming) returnAlso consider re-running
applyIncomingBoxes()from the width/height watcher when thebboxesinput is connected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/composables/boundingBoxes/useBoundingBoxes.ts` around lines 665 - 696, `applyIncomingBoxes` is incorrectly treating an empty upstream `input_bboxes` array as “no update” and its dedupe key ignores `widthValue`/`heightValue`, so stale regions can persist and dimension changes won’t rebuild boxes. Update the logic in `applyIncomingBoxes` to clear `state.value.regions` (and sync) when the connected incoming payload is an empty array, and include the canvas dimensions in the cache key so `fromBoundingBoxes` reruns when size changes. Also make the width/height watcher in this composable re-invoke `applyIncomingBoxes` when the `bboxes` input is connected so the local state stays aligned with the source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/boundingBoxes/WidgetBoundingBoxes.vue`:
- Around line 11-41: The Grid and Clear All buttons in WidgetBoundingBoxes.vue
already have visible text content, so their aria-label attributes are redundant
and can cause duplicate announcements for assistive tech. Remove the aria-label
from both Button elements in this component and keep the existing visible <span>
text as the accessible name, using the Button markup around the grid toggle and
clearAll action as the place to update.
In `@src/composables/boundingBoxes/useBoundingBoxes.ts`:
- Around line 147-160: In drawDots, avoid calling beginPath(), arc(), and fill()
for each dot individually; this hot path runs from drawCanvas during
requestAnimationFrame while dragging. Build one combined path for all dots
inside the nested loops, then call fill() once after the loops, keeping the
existing ctx.save()/restore() and gridSpec() checks intact.
- Around line 691-693: Consolidate the duplicate deep watches on
nodeOutputStore.nodeOutputs in useBoundingBoxes so the store is traversed only
once per update. Replace the separate watches that call updateBgImage and
applyIncomingBoxes with a single watcher callback that invokes both handlers,
while keeping the existing nodePreviewImages watch unchanged. Use the existing
symbols updateBgImage, applyIncomingBoxes, and nodeOutputStore.nodeOutputs to
locate the watcher block.
---
Duplicate comments:
In `@src/composables/boundingBoxes/useBoundingBoxes.ts`:
- Around line 665-696: `applyIncomingBoxes` is incorrectly treating an empty
upstream `input_bboxes` array as “no update” and its dedupe key ignores
`widthValue`/`heightValue`, so stale regions can persist and dimension changes
won’t rebuild boxes. Update the logic in `applyIncomingBoxes` to clear
`state.value.regions` (and sync) when the connected incoming payload is an empty
array, and include the canvas dimensions in the cache key so `fromBoundingBoxes`
reruns when size changes. Also make the width/height watcher in this composable
re-invoke `applyIncomingBoxes` when the `bboxes` input is connected so the local
state stays aligned with the source of truth.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f1743d52-0583-4879-8dd7-a8ef64d077b3
📒 Files selected for processing (4)
src/components/boundingBoxes/WidgetBoundingBoxes.vuesrc/composables/boundingBoxes/useBoundingBoxes.test.tssrc/composables/boundingBoxes/useBoundingBoxes.tssrc/locales/en/main.json
jtydhr88
force-pushed
the
feat/create-bounding-boxes-input-grid
branch
from
4225f9b to
0c4b9de
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/composables/boundingBoxes/useBoundingBoxes.test.ts`:
- Around line 164-167: The shared nodeOutputs ref in the
useBoundingBoxes.test.ts setup is not being cleared between tests, so later
cases can inherit state from the “applies incoming boxes when outputs stream in
after mount” test. Update the beforeEach block in the useBoundingBoxes test
suite to also reset outputState.nodeOutputs.value alongside appState.node and
outputState.outputs, using the same outputState mock reference created by the
vi.mock factory so each test starts from a clean state.
In `@src/composables/boundingBoxes/useBoundingBoxes.ts`:
- Around line 124-145: Resize snapping in snapRegion() is re-snapping both
corners for every non-move HitMode, which can move the anchored edge during
single-edge resizes. Update snapRegion() so resize-t/b/l/r preserves the
opposite edge fixed as applyDrag() intends, only snapping the actively dragged
edge and recomputing size from that fixed side. Use the existing HitMode checks
in useBoundingBoxes.ts to keep the resize handle behavior consistent with the
drag logic.
In `@src/locales/en/main.json`:
- Around line 2169-2170: The toolbar label for the grid action is lowercased in
the locale entry, which makes it inconsistent with adjacent labels like “Clear
all.” Update the translation value for the grid key in the en locale JSON so the
rendered label uses the same capitalization style as the surrounding toolbar
items, and verify WidgetBoundingBoxes uses that key consistently.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 3fd40cdd-9b45-4ac6-9722-218bba931505
📒 Files selected for processing (4)
src/components/boundingBoxes/WidgetBoundingBoxes.vuesrc/composables/boundingBoxes/useBoundingBoxes.test.tssrc/composables/boundingBoxes/useBoundingBoxes.tssrc/locales/en/main.json
jtydhr88
force-pushed
the
feat/create-bounding-boxes-input-grid
branch
from
0c4b9de to
0c807d2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/composables/boundingBoxes/useBoundingBoxes.test.ts`:
- Around line 316-346: Add test coverage for the `grid` and
`node.properties.bboxGrid` synchronization in `useBoundingBoxes`: the current
suite only checks snapping behavior, but it does not verify that `grid` is
initialized from `node.properties.bboxGrid` or that toggling `c.grid.value`
writes back to `properties.bboxGrid`. Extend `useBoundingBoxes.test.ts` with
cases using `setup()` (or a node fixture) that start with `bboxGrid: false` and
assert `grid.value` reflects it, then mutate `c.grid.value` and confirm the node
property updates accordingly.
In `@src/composables/boundingBoxes/useBoundingBoxes.ts`:
- Around line 704-706: The dedupe logic in useBoundingBoxes currently relies on
JSON.stringify(incoming), which is order-sensitive and unnecessarily
re-serializes the full payload on each change. Update the comparison around the
lastIncoming check to use es-toolkit’s isEqual against the previously stored
incoming value instead, and store the prior object/value rather than a string
key. Keep the behavior in useBoundingBoxes consistent while removing the
stringify-based dedupe path.
- Around line 623-639: The bboxGrid sync currently writes directly to
litegraphNode.value.properties inside the grid watcher, which bypasses
LiteGraph’s property lifecycle. Update the watch(grid, ...) logic in
useBoundingBoxes to use the node’s property API by calling
litegraphNode.value?.setProperty('bboxGrid', enabled) instead of assigning
props.bboxGrid directly, and if this change must mark the graph as changed, also
trigger litegraphNode.value?.graph?.incrementVersion() after the property
update.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 46aa848a-27ed-4571-b0dd-0ee8e9b608b1
📒 Files selected for processing (4)
src/components/boundingBoxes/WidgetBoundingBoxes.vuesrc/composables/boundingBoxes/useBoundingBoxes.test.tssrc/composables/boundingBoxes/useBoundingBoxes.tssrc/locales/en/main.json
Comment on lines
+316
to
+346
| describe('useBoundingBoxes grid snapping', () => { | ||
| it('snaps a drawn box to the grid when grid is enabled (default)', async () => { | ||
| const c = setup() | ||
| c.onPointerDown(pe(10, 10)) | ||
| c.onCanvasPointerMove(pe(60, 60)) | ||
| c.onDocPointerUp(pe(60, 60)) | ||
| await flush() | ||
| expect(c.modelValue.value).toHaveLength(1) | ||
| expect(c.modelValue.value[0].x).toBe(64) | ||
| expect(c.modelValue.value[0].width).toBe(256) | ||
| }) | ||
|
|
||
| it('does not snap when grid is disabled', async () => { | ||
| const c = setup() | ||
| c.grid.value = false | ||
| c.onPointerDown(pe(10, 10)) | ||
| c.onCanvasPointerMove(pe(55, 55)) | ||
| c.onDocPointerUp(pe(55, 55)) | ||
| await flush() | ||
| expect(c.modelValue.value[0].width).toBe(230) | ||
| }) | ||
|
|
||
| it('keeps the anchored edge fixed when resizing a single edge', async () => { | ||
| const c = setup([box({ x: 51, y: 51, width: 256, height: 256 })]) | ||
| c.onPointerDown(pe(60, 30)) | ||
| c.onCanvasPointerMove(pe(80, 30)) | ||
| c.onDocPointerUp(pe(80, 30)) | ||
| await flush() | ||
| expect(c.modelValue.value[0].x).toBe(51) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
No coverage for the grid ↔ node.properties.bboxGrid sync watchers.
useBoundingBoxes.ts (lines 623-639) initializes grid from node.properties.bboxGrid and writes back on toggle, but no test in this suite exercises that path (init from a node with bboxGrid: false, or asserting properties.bboxGrid updates after c.grid.value = ...). Since this is new, user-facing persistence behavior, consider adding coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/composables/boundingBoxes/useBoundingBoxes.test.ts` around lines 316 -
346, Add test coverage for the `grid` and `node.properties.bboxGrid`
synchronization in `useBoundingBoxes`: the current suite only checks snapping
behavior, but it does not verify that `grid` is initialized from
`node.properties.bboxGrid` or that toggling `c.grid.value` writes back to
`properties.bboxGrid`. Extend `useBoundingBoxes.test.ts` with cases using
`setup()` (or a node fixture) that start with `bboxGrid: false` and assert
`grid.value` reflects it, then mutate `c.grid.value` and confirm the node
property updates accordingly.
Source: Coding guidelines
Comment on lines
+623
to
+639
| watch( | ||
| litegraphNode, | ||
| (node) => { | ||
| const props = node?.properties as { bboxGrid?: unknown } | undefined | ||
| if (props && typeof props.bboxGrid === 'boolean') | ||
| grid.value = props.bboxGrid | ||
| }, | ||
| { immediate: true } | ||
| ) | ||
| watch(grid, (enabled) => { | ||
| const props = litegraphNode.value?.properties as | ||
| | Record<string, unknown> | ||
| | undefined | ||
| if (props) props.bboxGrid = enabled | ||
| requestDraw() | ||
| }) | ||
|
|
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "setProperty\s*\(" src/lib/litegraph -S | head -30
echo "---"
rg -n "\.properties\.[a-zA-Z_]+\s*=" src -S -g '!**/*.test.ts' | head -30Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2298
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== useBoundingBoxes context =="
sed -n '600,660p' src/composables/boundingBoxes/useBoundingBoxes.ts
echo
echo "== LGraphNode.setProperty implementation =="
sed -n '1040,1105p' src/lib/litegraph/src/LGraphNode.ts
echo
echo "== any graph/version or onPropertyChanged references near setProperty =="
rg -n "onPropertyChanged|_version|setProperty" src/lib/litegraph/src/LGraphNode.ts src/lib/litegraph/src/LGraphCanvas.ts src/lib/litegraph/src/widgets/BaseWidget.ts -n -SRepository: Comfy-Org/ComfyUI_frontend
Length of output: 4704
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "properties\.[A-Za-z_][A-Za-z0-9_]*\s*=" src/composables src/extensions -SRepository: Comfy-Org/ComfyUI_frontend
Length of output: 2218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '60,100p' src/lib/litegraph/README.mdRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '180,230p' src/lib/litegraph/src/LGraphNode.ts
sed -n '820,860p' src/lib/litegraph/src/LGraphNode.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 3866
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "bboxGrid|grid" src/composables/boundingBoxes/useBoundingBoxes.ts src -SRepository: Comfy-Org/ComfyUI_frontend
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '7958,7990p' src/lib/litegraph/src/LGraphCanvas.ts
echo
rg -n "incrementVersion\(|_version\s*\+\+|setProperty\(" src/lib/litegraph/src -SRepository: Comfy-Org/ComfyUI_frontend
Length of output: 3116
Use the LiteGraph property API for bboxGrid
node.properties.bboxGrid = enabled bypasses onPropertyChanged; if this setting should behave like other node properties, call node.setProperty('bboxGrid', enabled) and, if dirty tracking matters here, also bump node.graph?.incrementVersion().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/composables/boundingBoxes/useBoundingBoxes.ts` around lines 623 - 639,
The bboxGrid sync currently writes directly to litegraphNode.value.properties
inside the grid watcher, which bypasses LiteGraph’s property lifecycle. Update
the watch(grid, ...) logic in useBoundingBoxes to use the node’s property API by
calling litegraphNode.value?.setProperty('bboxGrid', enabled) instead of
assigning props.bboxGrid directly, and if this change must mark the graph as
changed, also trigger litegraphNode.value?.graph?.incrementVersion() after the
property update.
Source: Coding guidelines
Comment on lines
+704
to
+706
| const key = JSON.stringify(incoming) | ||
| if (key === lastIncoming) return | ||
| lastIncoming = key |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider isEqual from es-toolkit instead of JSON.stringify for the dedupe key.
JSON.stringify(incoming) is order-sensitive and re-serializes the whole payload on every output change. es-toolkit's isEqual (deep equality) against the previously-stored value avoids both the property-order fragility and the stringify cost.
♻️ Proposed refactor
- let lastIncoming = ''
+ let lastIncoming: BoundingBox[] | null = null
function applyIncomingBoxes() {
...
const incoming = outputs?.input_bboxes
if (!incoming?.length) return
- const key = JSON.stringify(incoming)
- if (key === lastIncoming) return
- lastIncoming = key
+ if (lastIncoming && isEqual(incoming, lastIncoming)) return
+ lastIncoming = incomingAs per coding guidelines: "Use es-toolkit for utility functions."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/composables/boundingBoxes/useBoundingBoxes.ts` around lines 704 - 706,
The dedupe logic in useBoundingBoxes currently relies on
JSON.stringify(incoming), which is order-sensitive and unnecessarily
re-serializes the full payload on each change. Update the comparison around the
lastIncoming check to use es-toolkit’s isEqual against the previously stored
incoming value instead, and store the prior object/value rather than a string
key. Keep the behavior in useBoundingBoxes consistent while removing the
stringify-based dedupe path.
Source: Coding guidelines
jtydhr88
force-pushed
the
feat/create-bounding-boxes-input-grid
branch
from
0c807d2 to
2d90adf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/composables/boundingBoxes/useBoundingBoxes.test.ts (1)
316-346: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStill missing coverage for
grid↔node.properties.bboxGridpersistence.The new suite covers snap-enabled/disabled/anchored-resize behavior, but no test initializes
gridfrom a node withproperties.bboxGridset, nor asserts that togglinggrid.valuewrites back toproperties.bboxGrid. This is new, user-facing persistence behavior introduced in this PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/composables/boundingBoxes/useBoundingBoxes.test.ts` around lines 316 - 346, Add coverage for the persistence bridge between useBoundingBoxes grid and node.properties.bboxGrid. In useBoundingBoxes.test.ts, extend the existing grid snapping suite to initialize the composable from a node whose properties.bboxGrid is set and assert grid.value starts in sync, then toggle grid.value and verify the underlying node.properties.bboxGrid is updated. Use the existing setup helper and the grid/modelValue refs in useBoundingBoxes to keep the test aligned with the new user-facing behavior.src/composables/boundingBoxes/useBoundingBoxes.ts (1)
623-639: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBypasses LiteGraph's property API for
bboxGrid.
node.properties.bboxGrid = enabledmutates the property object directly instead of callingnode.setProperty('bboxGrid', enabled), which skipsonPropertyChangedand any dirty/version tracking that extensions may rely on.🔧 Proposed fix
watch(grid, (enabled) => { - const props = litegraphNode.value?.properties as - | Record<string, unknown> - | undefined - if (props) props.bboxGrid = enabled + litegraphNode.value?.setProperty('bboxGrid', enabled) requestDraw() })As per coding guidelines: "Follow ADR constraints for entity architecture: use commands for mutations, keep entity data in dedicated stores... Changes to entity callbacks,
node.widgets,node.serialize, orgraph._version++require migration guidance."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/composables/boundingBoxes/useBoundingBoxes.ts` around lines 623 - 639, The `watch(grid, ...)` handler in `useBoundingBoxes` is mutating `litegraphNode.value.properties.bboxGrid` directly, which bypasses LiteGraph’s property lifecycle. Update the `bboxGrid` change flow to call the node’s property API via `litegraphNode.value.setProperty('bboxGrid', enabled)` instead of writing into `properties`, while keeping the existing `watch(litegraphNode, ...)` sync and `requestDraw()` behavior intact.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/palette/PaletteSwatchRow.vue`:
- Around line 10-18: The swatch trigger in PaletteSwatchRow.vue is currently a
non-focusable div, so it cannot be reached or activated via keyboard when
replacing ColorPicker’s default button. Update the swatch element in the
PaletteSwatchRow template to use a button (preferred) or add equivalent
focusability plus Enter/Space keyboard handling, while keeping the existing
remove(i), onPointerDown(i, $event), and data attributes behavior intact.
In `@src/components/ui/color-picker/ColorPicker.vue`:
- Line 68: The trigger slot in ColorPicker.vue is using an explicit slot prop
binding for previewColor even though the local variable name matches the prop
name, so update the <slot name="trigger"> binding to use same-name shorthand for
that prop. Keep the existing isOpen binding as-is, and change the previewColor
slot prop in the ColorPicker component to the shorthand form to match the
project’s slot binding convention.
---
Duplicate comments:
In `@src/composables/boundingBoxes/useBoundingBoxes.test.ts`:
- Around line 316-346: Add coverage for the persistence bridge between
useBoundingBoxes grid and node.properties.bboxGrid. In useBoundingBoxes.test.ts,
extend the existing grid snapping suite to initialize the composable from a node
whose properties.bboxGrid is set and assert grid.value starts in sync, then
toggle grid.value and verify the underlying node.properties.bboxGrid is updated.
Use the existing setup helper and the grid/modelValue refs in useBoundingBoxes
to keep the test aligned with the new user-facing behavior.
In `@src/composables/boundingBoxes/useBoundingBoxes.ts`:
- Around line 623-639: The `watch(grid, ...)` handler in `useBoundingBoxes` is
mutating `litegraphNode.value.properties.bboxGrid` directly, which bypasses
LiteGraph’s property lifecycle. Update the `bboxGrid` change flow to call the
node’s property API via `litegraphNode.value.setProperty('bboxGrid', enabled)`
instead of writing into `properties`, while keeping the existing
`watch(litegraphNode, ...)` sync and `requestDraw()` behavior intact.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 4bb68ee8-250a-43f7-8171-ef33e33d1c51
📒 Files selected for processing (9)
src/components/boundingBoxes/WidgetBoundingBoxes.vuesrc/components/palette/PaletteSwatchRow.test.tssrc/components/palette/PaletteSwatchRow.vuesrc/components/ui/color-picker/ColorPicker.vuesrc/composables/boundingBoxes/useBoundingBoxes.test.tssrc/composables/boundingBoxes/useBoundingBoxes.tssrc/composables/palette/usePaletteSwatchRow.test.tssrc/composables/palette/usePaletteSwatchRow.tssrc/locales/en/main.json
| </div> | ||
| <div | ||
| class="flex flex-1 items-center justify-between pl-1 text-xs text-component-node-foreground" | ||
| <slot name="trigger" :open="isOpen" :preview-color="previewColor"> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use shorthand for same-name slot prop binding.
previewColor (the local variable) matches the intended prop name, so this should use the shorthand form rather than the kebab-case explicit binding.
♻️ Proposed fix
- <slot name="trigger" :open="isOpen" :preview-color="previewColor">
+ <slot name="trigger" :open="isOpen" :previewColor>As per coding guidelines, "Use same-name shorthand for slot prop bindings, such as :isExpanded instead of :is-expanded="isExpanded"."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <slot name="trigger" :open="isOpen" :preview-color="previewColor"> | |
| <slot name="trigger" :open="isOpen" :previewColor> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/color-picker/ColorPicker.vue` at line 68, The trigger slot
in ColorPicker.vue is using an explicit slot prop binding for previewColor even
though the local variable name matches the prop name, so update the <slot
name="trigger"> binding to use same-name shorthand for that prop. Keep the
existing isOpen binding as-is, and change the previewColor slot prop in the
ColorPicker component to the shorthand form to match the project’s slot binding
convention.
Source: Coding guidelines
jtydhr88
force-pushed
the
feat/create-bounding-boxes-input-grid
branch
from
2d90adf to
ad9f0fa
Compare
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
2 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
BE is Comfy-Org/ComfyUI#14724
Screenshots (if applicable)
2026-07-01.15-40-17.mp4