Modal Component & Custom UI Components

[viva-jinyi]

Summary

Implementation of custom UI components for the design system, including buttons, cards, and input components.

Changes

Button Components

• ✅ Created unified button system with BaseButtonProps interface
• ✅ Implemented IconButton, TextButton, IconTextButton components
• ✅ Added MoreButton with popover functionality
• ✅ Created IconGroup for grouping icon buttons
• ✅ Support for primary, secondary, transparent styles with dark theme

Card Components

• ✅ Implemented CardContainer with flexible aspect ratios
• ✅ Created CardTop with overlay slots for corner positioning
• ✅ Added CardBottom, CardTitle, CardDescription components
• ✅ Support for different card layouts (square, portrait, tallPortrait)

Input Components

• ✅ Implemented SearchBox with icon support
• ✅ Created SingleSelect dropdown component
• ✅ Added MultiSelect for multiple selections

Other Components

• ✅ Added SquareTag component for labels/badges

Testing

Components are demonstrated in ModelSelector widget with various examples.

Next Steps

• Will set up Storybook in forked repository for component documentation
• GitHub Pages deployment for visual component library

design-system.webm

🤖 Generated with Claude Code

┆Issue is synchronized with this Notion page by Unito

[github-actions]

⚠️ Warnings

⚠️ Warning: E2E Test Coverage Missing

If this PR modifies behavior that can be covered by browser-based E2E tests, those tests are required. PRs lacking applicable test coverage may not be reviewed until added. Please add or update browser tests to ensure code quality and prevent regressions.

[christian-byrne]

Run npm run lint

> @comfyorg/[email protected][6](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/16906158538/job/47896319490?pr=4908#step:9:7).2 lint
> eslint src


> @comfyorg/[email protected] format:check
> prettier --check './**/*.{js,ts,tsx,vue,mts}'

Checking formatting...
All matched files use Prettier code style!

> @comfyorg/[email protected] knip
> knip

[log] Unused files (3)
[log] src/components/custom/button/IconGroup.vue    
src/components/custom/card/CardTitle.vue      
src/components/custom/input/SingleSelect.vue  
Error: Process completed with exit code 1.

I think we might want knip to just be a warning instead of erroring. It is complaining that we are adding files that are not used yet.

[claude]

Claude finished @viva-jinyi's task —— View job

---

Comprehensive PR Review ✅

Completed comprehensive review of PR #4908 "Modal Component & Custom UI Components"

Todo List:

• Read comprehensive PR review instructions
• Phase 1: Environment setup and PR context
• Phase 2: Load comprehensive knowledge base (partial - loaded repo guide)
• Phase 3-5: Deep analysis of changed files
• Phase 4: Post individual inline comments using gh api
• Phase 6: Contextual review based on PR type
• Phase 7: Generate comprehensive summary

Final Review Statistics:

• CRITICAL_COUNT = 1 (lodash dependency issue - will cause runtime error)
• HIGH_COUNT = 1 (CSS !important overuse)
• MEDIUM_COUNT = 5 (PrimeVue reimplementations + debug code + styling issues)
• LOW_COUNT = 1 (commented code cleanup)
• ARCHITECTURE_ISSUES = 1
• SECURITY_ISSUES = 0
• PERFORMANCE_ISSUES = 0
• QUALITY_ISSUES = 7

Review Complete ✅

8 inline comments posted covering all major issues found
1 comprehensive summary posted with detailed findings and recommendations

Key Issues Identified:

1. 🚨 CRITICAL: Missing lodash dependency will cause runtime errors
2. ⚠️ HIGH: Excessive CSS !important usage creates brittle styling
3. 📝 MEDIUM: Extensive PrimeVue reimplementation instead of using built-ins
4. 🧹 LOW: Debug code and commented sections need cleanup

Positive Findings:

• Solid TypeScript implementation
• Good component architecture and separation of concerns
• Proper accessibility considerations
• Consistent i18n usage

The critical lodash dependency issue must be resolved before merge.

[christian-byrne]

We switched from lodash to es-toolkit

[claude]

Comprehensive PR Review

This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another.

Review Summary

PR: Modal Component & Custom UI Components (#4908)
Impact: 1332 additions, 18 deletions across 52 files

Issue Distribution

• Critical: 1
• High: 1
• Medium: 5
• Low: 1

Category Breakdown

• Architecture: 1 issues
• Security: 0 issues
• Performance: 0 issues
• Code Quality: 7 issues

Key Findings

Architecture & Design

Critical Issue: The PR imports lodash.merge but does not add lodash as a dependency, which will cause runtime errors. This needs to be fixed before merge.

Design Pattern Concern: The components extensively reimplement PrimeVue functionality rather than leveraging the library's built-in capabilities. This increases maintenance burden and bundle size while reducing the benefits of using a component library.

Security Considerations

No security vulnerabilities were identified in this review.

Performance Impact

No significant performance issues were found. The components appear lightweight, though the PrimeVue reimplementation may increase bundle size unnecessarily.

Integration Points

The new design system components integrate well with the existing dialog system. The new showLayoutDialog function provides a clean API for layout-based dialogs.

Positive Observations

• Consistent TypeScript: All components use proper TypeScript interfaces and type definitions
• Responsive Design: Components use Tailwind classes appropriately for responsive layouts
• Component Architecture: Good separation of concerns with focused, single-responsibility components
• Accessibility: Proper use of semantic HTML and ARIA attributes where appropriate
• Internationalization: Proper use of i18n in user-facing text

Code Quality Issues Found

1. PrimeVue Reimplementation: Multiple components reimplement PrimeVue functionality instead of using built-in variants
2. CSS Override Patterns: Excessive use of !important and deep selectors to force styling overrides
3. Debug Code: Console.log statements left in production code
4. Code Cleanup: Commented-out code blocks should be removed

References

• ComfyUI Frontend Architecture Guide
• PrimeVue Documentation

Next Steps

1. Address critical issue: Add lodash dependency or replace merge with native alternative
2. Consider architectural feedback: Evaluate using more PrimeVue built-ins vs custom implementations
3. Code cleanup: Remove debug statements and commented code
4. CSS improvements: Reduce reliance on !important declarations

---

This is a comprehensive automated review. The critical dependency issue must be resolved before merge. For architectural decisions requiring human judgment, please request additional manual review.

[Myestery]

Basing off this as its very close to completion

[christian-byrne]

Can we make some changes to file/folder naming?

• Use camelcase for all files and folders in src
• No need to specify the components are custom as all components in src are essentially custom
• Change to reflect the updated plan for asset management:
  • Change from "widget" to "dialog" or "modal" (since this modal will not just be raised from a widget anymore)
  • Cahnge from "Model" to "Asset" (since the modal will allow viewing other assets beyond just models)

Some possible final structure:

  │       ├── components/
  │       │   ├── button/
  │       │   │   ├── IconButton.vue
  │       │   │   ├── IconGroup.vue
  │       │   │   ├── IconTextButton.vue
  │       │   │   ├── MoreButton.vue
  │       │   │   └── TextButton.vue
  │       │   ├── card/
  │       │   │   ├── CardBottom.vue
  │       │   │   ├── CardContainer.vue
  │       │   │   ├── CardDescription.vue
  │       │   │   ├── CardTitle.vue
  │       │   │   └── CardTop.vue
  │       │   ├── input/
  │       │   │   ├── MultiSelect.vue
  │       │   │   ├── SearchBox.vue
  │       │   │   └── SingleSelect.vue
  │       │   └── widget/
  │       │       ├── ModelSelector.vue
  │       │       ├── layout/
  │       │       │   └── BaseWidgetLayout.vue
  │       │       ├── nav/
  │       │       │   ├── NavItem.vue
  │       │       │   └── NavTitle.vue
  │       │       └── panel/
  │       │           ├── LeftSidePanel.vue
  │       │           ├── PanelHeader.vue
  │       │           └── RightSidePanel.vue
  │       ├── types/
  │       │   ├── buttonTypes.ts
  │       │   ├── navTypes.ts
  │       │   └── widgetTypes.ts

[Myestery]

Looks Good to Me
The ModelSelector has dummy values but since its not being used, its good for documentation

[viva-jinyi]

@Myestery I'm working on Storybook. So you can see docs via storybook later 😁

[viva-jinyi]

@christian-byrne I changed the directory following your review.

[christian-byrne]

LGTM!