Skip to content

fix: Add dropdown size control to Select components and improve UI#5290

Merged
christian-byrne merged 12 commits intomainfrom
feature/design-size-adjust
Sep 11, 2025
Merged

fix: Add dropdown size control to Select components and improve UI#5290
christian-byrne merged 12 commits intomainfrom
feature/design-size-adjust

Conversation

@viva-jinyi
Copy link
Member

@viva-jinyi viva-jinyi commented Sep 1, 2025 •
edited by sync-by-unito bot
Loading

Summary

  • Added dropdown size control props to MultiSelect and SingleSelect components
  • Added size variant support to SearchBox component
  • Created new CardGridList component for responsive grid layouts

Changes

Select Components Enhancement

Added three new props to both MultiSelect and SingleSelect components:

  • listMaxHeight: Controls the maximum height of the dropdown list container
  • popoverMinWidth: Sets minimum width for the dropdown popover
  • popoverMaxWidth: Sets maximum width for the dropdown popover

These props enable better control over dropdown dimensions to handle various content sizes and improve UI consistency.

SearchBox Component

  • Added size prop with 'md' (default) and 'lg' variants
  • Provides consistent sizing options across input components

CardGridList Component

  • New component for creating responsive grid layouts
  • Configurable grid columns, spacing, and card dimensions
  • Automatically adjusts layout based on container width

Test Plan

  • Updated Storybook stories for all modified components
  • Added stories demonstrating new size control props
  • Verified dropdown behavior with different content sizes
  • Tested responsive behavior of CardGridList component

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

┆Issue is synchronized with this Notion page by Unito

@viva-jinyi viva-jinyi requested a review from a team as a code owner September 1, 2025 08:55
@github-actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

🎭 Playwright Test Results

✅ Tests completed successfully!

⏰ Completed at: 09/11/2025, 08:02:27 AM UTC

📊 Test Reports by Browser

🎉 Click on the links above to view detailed test results for each browser configuration.

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Sep 1, 2025
@viva-jinyi viva-jinyi self-assigned this Sep 1, 2025
@viva-jinyi viva-jinyi force-pushed the feature/design-size-adjust branch from 5efe1d4 to a44580c Compare September 1, 2025 08:56
DrJKL
DrJKL requested changes Sep 1, 2025
View reviewed changes
christian-byrne
christian-byrne reviewed Sep 2, 2025
View reviewed changes
christian-byrne
christian-byrne reviewed Sep 2, 2025
View reviewed changes
christian-byrne
christian-byrne reviewed Sep 2, 2025
View reviewed changes
christian-byrne
christian-byrne reviewed Sep 2, 2025
View reviewed changes
christian-byrne
christian-byrne reviewed Sep 2, 2025
View reviewed changes
Copy link
Contributor

@christian-byrne christian-byrne left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary (Automated Claude Review)

I've reviewed the dropdown size control enhancements and UI improvements. The implementation follows ComfyUI patterns well with proper Vue 3 Composition API usage and comprehensive Storybook documentation.

Key Strengths:

  • Clean component architecture with appropriate separation of concerns
  • Excellent TypeScript integration and prop definitions
  • Comprehensive Storybook stories demonstrating all variants
  • Proper i18n usage and accessibility considerations
  • Maintains backward compatibility

Areas for improvement:

  • Consider using Tailwind classes over inline styles for better maintainability
  • Add runtime validation for CSS grid parameters
  • Ensure consistent documentation across all props
  • Reference existing design tokens for sizing consistency

The changes provide valuable flexibility for dropdown sizing while maintaining the established ComfyUI design patterns. Overall good implementation with room for minor refinements.

@viva-jinyi viva-jinyi force-pushed the feature/design-size-adjust branch 2 times, most recently from 1acd20f to 423ddaa Compare September 9, 2025 02:20
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Sep 9, 2025
@viva-jinyi viva-jinyi force-pushed the feature/design-size-adjust branch from 4293ee7 to b5783c2 Compare September 9, 2025 14:15
@viva-jinyi viva-jinyi added the New Browser Test Expectations New browser test screenshot should be set by github action label Sep 9, 2025
@viva-jinyi
Copy link
Member Author

viva-jinyi commented Sep 9, 2025

Based on Alex (Designer)’s feedback, I updated the design and renamed some components. In addition, from the code reviews here, I refactored the long styling code to use the cn util throughout.

@github-actions github-actions bot requested a review from a team as a code owner September 9, 2025 15:04
@viva-jinyi viva-jinyi force-pushed the feature/design-size-adjust branch from af95050 to a4883e7 Compare September 10, 2025 01:12
@viva-jinyi viva-jinyi enabled auto-merge (squash) September 10, 2025 01:12
@viva-jinyi viva-jinyi force-pushed the feature/design-size-adjust branch 2 times, most recently from a39ccbc to a346018 Compare September 10, 2025 01:48
DrJKL
DrJKL previously approved these changes Sep 10, 2025
View reviewed changes
Copy link
Contributor

@DrJKL DrJKL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits, but I like moving more towards this pattern a lot.

@viva-jinyi viva-jinyi force-pushed the feature/design-size-adjust branch from a346018 to a82ab5c Compare September 10, 2025 12:21
@viva-jinyi viva-jinyi force-pushed the feature/design-size-adjust branch from d86aad8 to d24fe3c Compare September 11, 2025 07:47
viva-jinyi and others added 12 commits September 11, 2025 16:50
@viva-jinyi
feature: size adjust
4c2ecb2
@viva-jinyi
feature: design adjust
0682e82
@viva-jinyi
fix: popover width, height added
701b74d
@viva-jinyi
fix: li style override
0f817bf
@viva-jinyi
refactor: improve component readability and
a9d4cf2 
  maintainability per PR feedback

  - Replace CardGridList component with
  createGridStyle utility function
  - Add runtime validation for grid column values
  - Remove !important usage in MultiSelect, use cn()
  function instead
  - Extract popover sizing logic into
  usePopoverSizing composable
  - Improve class string readability by splitting
  into logical groups
  - Use Tailwind size utilities (size-8, size-10)
  instead of separate width/height
  - Remove magic numbers in SearchBox, align with
  button sizes
  - Rename BaseWidgetLayout to BaseModalLayout for
  clarity
  - Enhance SearchBox click area to cover entire
  component
  - Refactor long class strings using cn() utility
  across components
@viva-jinyi
fix: BaseWidgetLayout => BaseModalLayout
e2bd9b4
@viva-jinyi
fix: CardGrid deleted
9f4ccf4
@viva-jinyi
fix: unused exported types
8428af8
@viva-jinyi
Update test expectations [skip ci]
047ed23
@viva-jinyi
chore: code review
c8b5b50
@viva-jinyi
Update test expectations [skip ci]
f0feb13
@viva-jinyi
chore: restore screenshot
86f5798
@viva-jinyi viva-jinyi force-pushed the feature/design-size-adjust branch from fa5b955 to 86f5798 Compare September 11, 2025 07:51
@viva-jinyi viva-jinyi added New Browser Test Expectations New browser test screenshot should be set by github action and removed New Browser Test Expectations New browser test screenshot should be set by github action labels Sep 11, 2025
@christian-byrne christian-byrne merged commit e70ddea into main Sep 11, 2025
23 of 25 checks passed
@christian-byrne christian-byrne deleted the feature/design-size-adjust branch September 11, 2025 10:01
snomiao pushed a commit that referenced this pull request Sep 12, 2025
@viva-jinyi @snomiao
fix: Add dropdown size control to Select components and improve UI (#…
ae14085 
…5290)

* feature: size adjust

* feature: design adjust

* fix: popover width, height added

* fix: li style override

* refactor: improve component readability and
  maintainability per PR feedback

  - Replace CardGridList component with
  createGridStyle utility function
  - Add runtime validation for grid column values
  - Remove !important usage in MultiSelect, use cn()
  function instead
  - Extract popover sizing logic into
  usePopoverSizing composable
  - Improve class string readability by splitting
  into logical groups
  - Use Tailwind size utilities (size-8, size-10)
  instead of separate width/height
  - Remove magic numbers in SearchBox, align with
  button sizes
  - Rename BaseWidgetLayout to BaseModalLayout for
  clarity
  - Enhance SearchBox click area to cover entire
  component
  - Refactor long class strings using cn() utility
  across components

* fix: BaseWidgetLayout => BaseModalLayout

* fix: CardGrid deleted

* fix: unused exported types

* Update test expectations [skip ci]

* chore: code review

* Update test expectations [skip ci]

* chore: restore screenshot

---------

Co-authored-by: github-actions <github-actions@github.com>
@jtydhr88 jtydhr88 mentioned this pull request Dec 8, 2025
Merged
jtydhr88 added a commit that referenced this pull request Dec 8, 2025
@jtydhr88
feat: Don't hide scrollbar in BaseModalLayout (#7226)
6081404 
## Summary
- Add showScrollbar prop to BaseModalLayout component to control
scrollbar visibility
- Enable scrollbar in Templates dialog for better navigation when
content overflows

The original implementation used scrollbar-hide class by default. Since
the intent behind hiding the scrollbar wasn't documented in the original
PR (#5290), this change preserves the default behavior (showScrollbar:
false) while allowing individual dialogs to opt-in to visible
scrollbars.

The Templates dialog specifically benefits from a visible scrollbar as
it contains a large grid of template cards that requires scrolling.

fix #6915

## Screenshots (if applicable)
<img width="2490" height="1097" alt="image"
src="https://github.com/user-attachments/assets/711b060c-9752-4cee-84c0-d90210969f5a"
/>

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7226-feat-add-showScrollbar-prop-to-BaseModalLayout-2c36d73d365081b1b55bdcd50ca89030)
by [Unito](https://www.unito.io)
Yourz pushed a commit that referenced this pull request Dec 24, 2025
@jtydhr88 @Yourz
feat: Don't hide scrollbar in BaseModalLayout (#7226)
cff85e7 
## Summary
- Add showScrollbar prop to BaseModalLayout component to control
scrollbar visibility
- Enable scrollbar in Templates dialog for better navigation when
content overflows

The original implementation used scrollbar-hide class by default. Since
the intent behind hiding the scrollbar wasn't documented in the original
PR (#5290), this change preserves the default behavior (showScrollbar:
false) while allowing individual dialogs to opt-in to visible
scrollbars.

The Templates dialog specifically benefits from a visible scrollbar as
it contains a large grid of template cards that requires scrolling.

fix #6915

## Screenshots (if applicable)
<img width="2490" height="1097" alt="image"
src="https://github.com/user-attachments/assets/711b060c-9752-4cee-84c0-d90210969f5a"
/>

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7226-feat-add-showScrollbar-prop-to-BaseModalLayout-2c36d73d365081b1b55bdcd50ca89030)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@christian-byrne christian-byrne christian-byrne approved these changes

@DrJKL DrJKL DrJKL left review comments

@arjansingh arjansingh Awaiting requested review from arjansingh

Assignees

@viva-jinyi viva-jinyi

@christian-byrne christian-byrne

Labels

New Browser Test Expectations New browser test screenshot should be set by github action size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@viva-jinyi @DrJKL @christian-byrne