-
Notifications
You must be signed in to change notification settings - Fork 402
Cloud feedback Extension #6626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cloud feedback Extension #6626
Conversation
Add new ActionBarButton interface with support for: - Lucide icon integration - Optional label text - Custom CSS classes - Tooltip support - Click handlers
Create Pinia store to aggregate action bar buttons from all extensions. Uses computed property to dynamically collect buttons from extension store.
Create component to render action bar buttons with: - Lucide icon support via template slot - Optional label display - Custom styling via class prop - Secondary severity for neutral colors - Tooltip support
Add ActionBarButtons component at the start of the action bar container. Buttons will appear before legacy elements, ComfyActionbar, and user controls.
Add cloud-only extension that registers a feedback button in the action bar. - Uses message-circle-question-mark Lucide icon - Opens Zendesk feedback URL in new tab - Displays 'Feedback' label
Import cloudFeedbackTopbarButton extension. Extension loads unconditionally to support action bar buttons API.
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/07/2025, 06:29:46 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 11/07/2025, 06:46:51 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Replace hardcoded 'Feedback' string with t('g.feedback') translation.
Ensures proper localization support for the feedback button.
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.25 MB (baseline 3.25 MB) • 🔴 +55 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 795 kB (baseline 793 kB) • 🔴 +1.66 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 8.18 kB (baseline 8.18 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 293 kB (baseline 293 kB) • 🟢 -2 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 12.6 kB (baseline 12.6 kB) • ⚪ 0 BReusable component library chunks
Status: 1 added / 1 removed Data & Services — 10.4 kB (baseline 10.4 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 1 added / 1 removed Utilities & Hooks — 1.07 kB (baseline 1.07 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 5.32 MB (baseline 5.32 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 2.55 MB (baseline 2.55 MB) • ⚪ 0 BBundles that do not match a named category
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: Cloud feedback Extension (#6626)
Impact: 101 additions, 0 deletions across 6 files
Issue Distribution
- Critical: 0
- High: 0
- Medium: 3
- Low: 3
Category Breakdown
- Architecture: 1 issue
- Security: 1 issue
- Performance: 1 issue
- Code Quality: 3 issues
Key Findings
Architecture & Design
The PR introduces a clean extensible system for action bar buttons following established patterns in the ComfyUI frontend. The implementation properly leverages the existing extension system and Pinia stores. However, there's a minor inconsistency in using getter patterns instead of direct property assignment which deviates from other similar extensions.
Security Considerations
The implementation uses window.open() with proper security attributes (noopener, noreferrer), which is good. However, as this pattern may be reused in other extensions, consider establishing a centralized URL handling service to ensure consistent security practices across all external link handling.
Performance Impact
The action bar button store uses Vue's computed property with flatMap, which will recreate the buttons array on any extension store change. For the current scale this is acceptable, but consider memoization if the number of extensions grows significantly.
Integration Points
The implementation integrates well with the existing TopMenuSection component and follows the established pattern for extension-contributed UI elements. The cloud-only loading mechanism ensures proper tree-shaking for OSS builds.
Positive Observations
- Excellent use of TypeScript interfaces for type safety
- Proper integration with the i18n system
- Clean separation of concerns with dedicated store
- Consistent with existing extension architecture patterns
- Good use of PrimeVue components and styling conventions
- Proper conditional loading for cloud environments
References
Next Steps
- Address medium priority security and accessibility concerns
- Consider architectural consistency improvements
- Add comprehensive JSDoc documentation for the new interface
- Consider adding unit tests for the new store and component
This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.
|
Looks nice ! But can you add an user option to hide the text and show only the icon ? I mean, not only for this button, but also other future custom buttons, added this way. If the custom node developer does not provide an icon, maybe just show the first letter in the text. Otherwise, the top floating panel will get very large, and we lose the benefits ( extra space ) of having a floating panel, instead of a full length Top bar. |
at the moment, you can pass empty string to either icon or label |
christian-byrne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
| <div | ||
| class="actionbar-container pointer-events-auto flex h-12 items-center rounded-lg border border-[var(--interface-stroke)] px-2 shadow-interface" | ||
| > | ||
| <ActionBarButtons /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit (non-blocking): Is it worth to do v-if on condition !!button.length?
This pull request introduces a new extensible system for adding custom action bar buttons to the top menu, and demonstrates its use by adding a cloud feedback button. The changes include defining a new
ActionBarButtontype, updating the extension system to support action bar buttons, creating a Pinia store to aggregate buttons from extensions, and updating the UI to render these buttons in the top menu. The cloud feedback button is now conditionally loaded for cloud environments.Extensible Action Bar Button System
ActionBarButtoninterface incomfy.tsfor describing buttons (icon, label, tooltip, class, click handler) and added anactionBarButtonsproperty to theComfyExtensioninterface to allow extensions to contribute buttons. [[1]](diffhunk://#diff-c29886a1b0c982c6fff3545af0caUI Integration
ActionBarButtons.vuecomponent that renders all action bar buttons using the store, and integrated it into the top menu section (TopMenuSection.vue). [1] [2] [3]Cloud Feedback Button Extension
cloudFeedbackTopbarButton.ts) that registers a "Feedback" button opening the Zendesk feedback page, and ensured it loads only in cloud environments. [1] [2]┆Issue is synchronized with this Notion page by Unito