Style: Grid for widgets #6891
Style: Grid for widgets #6891
Conversation
dosubot
bot
added
the
size:S
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (20)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughTwo Vue components switch from flexbox to CSS Grid: NodeWidgets.vue introduces a three-column grid (min-content, flexible, auto) and makes widget inputs span two columns; WidgetLayoutField.vue replaces a flex row with a grid (grid-cols-subgrid) and removes a fixed inner width constraint. Changes
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 11/24/2025, 07:11:05 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/24/2025, 06:57:23 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.18 MB (baseline 3.18 MB) • 🔴 +102 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 941 kB (baseline 941 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 7.97 kB (baseline 7.97 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 306 kB (baseline 306 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 141 kB (baseline 141 kB) • 🔴 +10 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 5.7 MB (baseline 5.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.87 MB (baseline 3.87 MB) • ⚪ 0 BBundles that do not match a named category
Status: 18 added / 18 removed |
| <div class="flex h-[30px] min-w-0 items-center justify-between gap-1"> | ||
| <div class="relative flex h-full min-w-0 w-20 items-center"> | ||
| <div | ||
| class="grid grid-cols-subgrid h-[30px] min-w-0 items-center justify-between gap-1" |
There was a problem hiding this comment.
nit: could we could use h-7.5 here
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/renderer/extensions/vueNodes/components/NodeWidgets.vue (2)
22-22:flex-1property is ineffective on grid items.The conditional class
has-[.widget-expands]:flex-1applies a flex property to a grid item, which has no effect since this element is a child of the grid container on line 9. Theitems-stretchclass already handles stretching in the grid context.This appears to be a leftover from the previous flexbox implementation and can be removed.
Apply this diff to remove the ineffective flex property:
- class="lg-node-widget group col-span-full grid grid-cols-subgrid items-stretch has-[.widget-expands]:flex-1" + class="lg-node-widget group col-span-full grid grid-cols-subgrid items-stretch"
55-55:flex-1property has no effect on grid items.The
flex-1class is applied to a grid item and won't affect the grid layout. Grid items are sized by the grid container's column definitions, not flex properties.Unless the widget components internally require this class for their own layout, this appears to be a leftover from the flexbox implementation and can be removed.
Apply this diff if the widget components don't rely on the
flex-1class:- class="flex-1 col-span-2" + class="col-span-2"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/renderer/extensions/vueNodes/components/NodeWidgets.vue(3 hunks)src/renderer/extensions/vueNodes/widgets/components/layout/WidgetLayoutField.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (3)
src/renderer/extensions/vueNodes/widgets/components/layout/WidgetLayoutField.vue (2)
17-17: Width constraint removal aligns with flexible layout goals.The removal of the fixed
w-20constraint in favor of grid-based sizing is consistent with the PR objective of maintaining consistent widths while allowing more flexible overall sizing. The parent grid column definition (minmax(80px,max-content)) now controls the width constraint.
14-16: The justify-between usage is correct and necessary; however, verify browser support policy for grid-cols-subgrid.The
justify-betweenproperty appropriately spreads the two layout children across the grid width—the label/icon to the start and the slot content to the end. This is intentional layout behavior, not unnecessary styling. Thegap-1property controls spacing between items, not their distribution.Regarding browser compatibility: @comfyorg/design-system browser-support requirements are undocumented. Since this codebase defines no explicit
browserslistand relies on an external design-system package for Tailwind configuration, confirm your application's minimum browser versions supportgrid-cols-subgrid(Chrome 117+, Safari 16+, Firefox 71+). If not yet confirmed, coordinate with the design-system team or add an explicitbrowserslistconfiguration to this repository.src/renderer/extensions/vueNodes/components/NodeWidgets.vue (1)
9-9: Grid column definition looks well-structured.The three-column grid layout with
min-content,minmax(80px,max-content), andminmax(125px,auto)appropriately allocates space for the input slot dot, labels, and widget inputs respectively. This aligns with the PR's goal of consistent widget widths with flexible overall sizing.
AustinMroz
left a comment
AustinMroz
left a comment
There was a problem hiding this comment.
There's a minor regression with padding on the right side of widgets getting eaten at small sizes, but the change here is beautiful and works great.
DrJKL
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
github-actions
bot
dismissed stale reviews from AustinMroz and Myestery
via
41ea467
github-actions
bot
removed
the
New Browser Test Expectations
|
Will need to fix the textarea sizing again. Not a trivial task. |
Summary
Keeps the controls and widgets a consistent width, but lets the size be more flexible
Screenshots
┆Issue is synchronized with this Notion page by Unito