style: update ui and design of system notification components (what's new, new release notification, help center) #6300
Conversation
christian-byrne
requested review from
a team,
KarryCharon,
Yorha4D and
shinshin86
as code owners
dosubot
bot
added
the
size:L
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 10/26/2025, 09:15:15 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 10/26/2025, 09:31:45 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.29 MB (baseline 3.29 MB) • 🔴 +20 B_Main entry bundles and manifests_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ---------------------------------------- | ------- | ------- | ----------------------- | ---------------------- | ----------------------- | | **assets/index-oyoi7kkj.js** _(new)_ | — | 2.68 MB | 🔴 +2.68 MB | 🔴 +556 kB | 🔴 +421 kB | | ~~assets/index-DWvOzJH0.js~~ _(removed)_ | 2.68 MB | — | 🟢 -2.68 MB | 🟢 -556 kB | 🟢 -421 kB | | ~~assets/index-AKNvU-45.js~~ _(removed)_ | 614 kB | — | 🟢 -614 kB | 🟢 -114 kB | 🟢 -90.1 kB | | **assets/index-D6L-uhbp.js** _(new)_ | — | 614 kB | 🔴 +614 kB | 🔴 +114 kB | 🔴 +90.2 kB |Status: 2 added / 2 removed Graph Workspace — 720 kB (baseline 720 kB) • 🔴 +637 B_Graph editor runtime, canvas, workflow orchestration_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | -------------------------------------------- | ------ | ------ | ---------------------- | ---------------------- | ---------------------- | | **assets/GraphView-Bow481nD.js** _(new)_ | — | 720 kB | 🔴 +720 kB | 🔴 +141 kB | 🔴 +109 kB | | ~~assets/GraphView-Dg4Aai_0.js~~ _(removed)_ | 720 kB | — | 🟢 -720 kB | 🟢 -141 kB | 🟢 -109 kB |Status: 1 added / 1 removed Views & Navigation — 8.15 kB (baseline 8.15 kB) • ⚪ 0 B_Top-level views, pages, and routed surfaces_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/UserSelectView-D3DFxCF3.js** _(new)_ | — | 8.15 kB | 🔴 +8.15 kB | 🔴 +2.47 kB | 🔴 +2.15 kB | | ~~assets/UserSelectView-DatsetKh.js~~ _(removed)_ | 8.15 kB | — | 🟢 -8.15 kB | 🟢 -2.47 kB | 🟢 -2.15 kB |Status: 1 added / 1 removed Panels & Settings — 294 kB (baseline 294 kB) • ⚪ 0 B_Configuration panels, inspectors, and settings screens_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ---------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/CreditsPanel-CKVTjdFW.js** _(new)_ | — | 22.1 kB | 🔴 +22.1 kB | 🔴 +5.28 kB | 🔴 +4.6 kB | | ~~assets/CreditsPanel-CQ9CnGx9.js~~ _(removed)_ | 22.1 kB | — | 🟢 -22.1 kB | 🟢 -5.28 kB | 🟢 -4.6 kB | | **assets/KeybindingPanel-4oHRl0X5.js** _(new)_ | — | 15.2 kB | 🔴 +15.2 kB | 🔴 +3.76 kB | 🔴 +3.3 kB | | ~~assets/KeybindingPanel-BwN_MAa7.js~~ _(removed)_ | 15.2 kB | — | 🟢 -15.2 kB | 🟢 -3.76 kB | 🟢 -3.31 kB | | ~~assets/ExtensionPanel-BiHnURfY.js~~ _(removed)_ | 12.1 kB | — | 🟢 -12.1 kB | 🟢 -2.83 kB | 🟢 -2.48 kB | | **assets/ExtensionPanel-BXJ6-BPZ.js** _(new)_ | — | 12.1 kB | 🔴 +12.1 kB | 🔴 +2.83 kB | 🔴 +2.48 kB | | ~~assets/AboutPanel-BRiEpjA2.js~~ _(removed)_ | 10.3 kB | — | 🟢 -10.3 kB | 🟢 -2.66 kB | 🟢 -2.34 kB | | **assets/AboutPanel-CLupg0jN.js** _(new)_ | — | 10.3 kB | 🔴 +10.3 kB | 🔴 +2.66 kB | 🔴 +2.35 kB | | ~~assets/ServerConfigPanel-D5shDQkx.js~~ _(removed)_ | 8.2 kB | — | 🟢 -8.2 kB | 🟢 -2.17 kB | 🟢 -1.9 kB | | **assets/ServerConfigPanel-gO5YIM9t.js** _(new)_ | — | 8.2 kB | 🔴 +8.2 kB | 🔴 +2.17 kB | 🔴 +1.9 kB | | ~~assets/UserPanel-D1oKpou8.js~~ _(removed)_ | 7.91 kB | — | 🟢 -7.91 kB | 🟢 -2.06 kB | 🟢 -1.79 kB | | **assets/UserPanel-EMxAX9tB.js** _(new)_ | — | 7.91 kB | 🔴 +7.91 kB | 🔴 +2.06 kB | 🔴 +1.79 kB | | assets/settings-B-df0dZe.js | 20.7 kB | 20.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-CI6OKvJn.js | 22.9 kB | 22.9 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-CXGVj_nD.js | 24.5 kB | 24.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DfQ6dSJj.js | 31.6 kB | 31.6 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DJ2QgDzm.js | 25.2 kB | 25.2 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DRNLPMG6.js | 23.7 kB | 23.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DVVycxDc.js | 19.9 kB | 19.9 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-G6Dybj1b.js | 24.1 kB | 24.1 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-M6_GZccG.js | 26 kB | 26 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |Status: 6 added / 6 removed UI Components — 12.3 kB (baseline 12.3 kB) • ⚪ 0 B_Reusable component library chunks_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ----------------------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/ComfyQueueButton-Cmy-oyTr.js** _(new)_ | — | 11.1 kB | 🔴 +11.1 kB | 🔴 +2.75 kB | 🔴 +2.44 kB | | ~~assets/ComfyQueueButton-DlaBIspA.js~~ _(removed)_ | 11.1 kB | — | 🟢 -11.1 kB | 🟢 -2.76 kB | 🟢 -2.44 kB | | assets/UserAvatar.vue_vue_type_script_setup_true_lang-C9bSkTC5.js | 1.12 kB | 1.12 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |Status: 1 added / 1 removed Data & Services — 10 kB (baseline 10 kB) • ⚪ 0 B_Stores, services, APIs, and repositories_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ---------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ---------------------- | | **assets/keybindingService-DwKr-ope.js** _(new)_ | — | 7.21 kB | 🔴 +7.21 kB | 🔴 +1.74 kB | 🔴 +1.5 kB | | ~~assets/keybindingService-NE6iybSq.js~~ _(removed)_ | 7.21 kB | — | 🟢 -7.21 kB | 🟢 -1.75 kB | 🟢 -1.5 kB | | assets/serverConfigStore-CvX_HMTJ.js | 2.79 kB | 2.79 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |Status: 1 added / 1 removed Utilities & Hooks — 1.07 kB (baseline 1.07 kB) • ⚪ 0 B_Helpers, composables, and utility bundles_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | --------------------------- | ------- | ------- | ------------------ | ------------------ | ------------------ | | assets/mathUtil-CTARWQ-l.js | 1.07 kB | 1.07 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |Vendor & Third-Party — 5.36 MB (baseline 5.36 MB) • ⚪ 0 B_External libraries and shared vendor chunks_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | --------------------------------------- | ------- | ------- | ------------------ | ------------------ | ------------------ | | assets/vendor-other-EuL9bHKm.js | 3.22 MB | 3.22 MB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/vendor-primevue-PESgPnbc.js | 517 B | 517 B | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/vendor-tiptap-DY0_3CMM.js | 232 kB | 232 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/vendor-visualization-BEfdbjRw.js | 1.82 MB | 1.82 MB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/vendor-vue-Di9L6lvm.js | 92.4 kB | 92.4 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |Other — 2.55 MB (baseline 2.55 MB) • ⚪ 0 B_Bundles that do not match a named category_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | --------------------------- | ------- | ------- | ------------------ | ------------------ | ------------------ | | assets/commands-B2KZRBmX.js | 15.1 kB | 15.1 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-Bw-ckyga.js | 13.9 kB | 13.9 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-C_NmM85I.js | 13.8 kB | 13.8 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-CuozCW4W.js | 14 kB | 14 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-DGfVUJCR.js | 16.2 kB | 16.2 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-dOJNDogK.js | 14.5 kB | 14.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-DwiE551e.js | 14.7 kB | 14.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-Fw7mvqSy.js | 13.1 kB | 13.1 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-FXnO1W4Q.js | 13.2 kB | 13.2 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-Bgu6_Hvd.js | 59.5 kB | 59.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-Bv0L0qvp.js | 93 kB | 93 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-C3Doz3n_.js | 67.6 kB | 67.6 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-C7eBl607.js | 70.7 kB | 70.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-CHiV9ds2.js | 76.4 kB | 76.4 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-CIc79Nts.js | 68.5 kB | 68.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-DK5LmuBm.js | 58.8 kB | 58.8 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-J1nit7cj.js | 66.3 kB | 66.3 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-W97XgvAQ.js | 80.4 kB | 80.4 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-8Ef8lY1m.js | 196 kB | 196 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-BdF8EiZl.js | 200 kB | 200 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-Bv9Y8Cvp.js | 229 kB | 229 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-cMdB_wHv.js | 179 kB | 179 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-CvNWbbtX.js | 194 kB | 194 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-CwDWxzVz.js | 215 kB | 215 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-CyPAVHpA.js | 191 kB | 191 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-D6QTD6bJ.js | 181 kB | 181 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-DKn6VmRJ.js | 192 kB | 192 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | |
christian-byrne
added
the
claude-review
christian-byrne
added
claude-review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| --interface-menu-component-surface-hovered: var(--color-gray-200); | ||
| --interface-menu-component-surface-selected: var(--color-gray-400); | ||
| --interface-menu-keybind-surface-default: var(--color-gray-500); | ||
| --interface-menu-surface: var(--color-pure-white); |
There was a problem hiding this comment.
[quality] medium Priority
Issue: New CSS design tokens lack documentation
Context: The new tokens --interface-menu-surface and --interface-menu-stroke are added without any inline documentation explaining their purpose or usage guidelines
Suggestion: Add inline comments above the token definitions explaining their intended use cases and relationship to existing tokens
| style="margin-left: auto" | ||
| /> | ||
| </button> | ||
| </nav> |
There was a problem hiding this comment.
[security] high Priority
Issue: Potential XSS vulnerability with v-html usage
Context: Line 45 uses v-html with formattedContent which could allow arbitrary HTML injection if content is not properly sanitized
Suggestion: Use DOMPurify for sanitization before rendering or use safe markdown rendering approach. Check if renderMarkdownToHtml already includes sanitization
| width: 12px; | ||
| height: 2px; | ||
| background: white; | ||
| background: var(--text-primary); |
There was a problem hiding this comment.
[quality] low Priority
Issue: Inconsistent semantic token usage for text colors
Context: Line uses var(--text-primary) for both background and color properties which seems incorrect
Suggestion: Verify that using text tokens for background colors is intentional and semantically correct
| icon: 'icon-[lucide--clipboard-pen]', | ||
| label: t('helpCenter.feedback'), | ||
| action: () => { | ||
| // TODO: Implement feedback dialog action |
There was a problem hiding this comment.
[quality] medium Priority
Issue: Unimplemented functionality with TODO comment
Context: The feedback button action contains a TODO comment indicating incomplete implementation
Suggestion: Either implement the feedback dialog action or remove the feedback button if it's not ready for this release. Avoid shipping incomplete features with TODO markers
| <i | ||
| v-if="menuItem.showExternalIcon" | ||
| class="icon-[lucide--external-link] text-text-primary" | ||
| style="width: 16px; height: 16px; margin-left: auto" |
There was a problem hiding this comment.
[quality] low Priority
Issue: Inline styles instead of CSS classes
Context: Lines 37 and 42 use inline styles for icon sizing and positioning that could be moved to CSS classes
Suggestion: Create CSS classes like .external-icon and .chevron-icon to replace inline styles for better maintainability and consistency
| <span class="menu-label">{{ menuItem.label }}</span> | ||
| <i | ||
| v-if="menuItem.showExternalIcon" | ||
| class="icon-[lucide--external-link] text-text-primary" |
There was a problem hiding this comment.
[quality] medium Priority
Issue: Redundant CSS class naming - text-text-primary
Context: Line uses text-text-primary which appears to have redundant text- prefix
Suggestion: Use just text-primary or verify if this is the correct class name. The current naming suggests it should be just text-primary
|
|
||
| // Verify help center menu appears | ||
| const helpMenu = comfyPage.page.locator('.help-center-menu') | ||
| await expect(helpMenu).toBeVisible() |
There was a problem hiding this comment.
[architecture] low Priority
Issue: Test cases do not verify accessibility compliance
Context: New UI components and updated menu structure should include accessibility testing
Suggestion: Add test cases to verify ARIA attributes, keyboard navigation, and screen reader compatibility for the updated help center menu and notification components
There was a problem hiding this comment.
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: style: update ui and design of system notification components (what's new, new release notification, help center) (#6300)
Impact: 190 additions, 159 deletions across 6 files
Issue Distribution
- Critical: 0
- High: 0
- Medium: 4
- Low: 2
Category Breakdown
- Architecture: 1 issues
- Security: 0 issues
- Performance: 0 issues
- Code Quality: 5 issues
Key Findings
Architecture & Design
The PR correctly implements semantic design tokens following the established pattern with --interface-menu-surface and --interface-menu-stroke. The token naming follows existing conventions and properly supports light/dark theme switching. However, new design tokens lack documentation which could make them difficult for other developers to understand and use correctly.
Security Considerations
No new security vulnerabilities introduced. The changes are primarily cosmetic, replacing hardcoded colors with semantic tokens. The existing v-html usage in WhatsNewPopup was not modified and appears to use proper sanitization through renderMarkdownToHtml.
Performance Impact
No significant performance regressions identified. The changes replace static color values with CSS custom properties which have negligible performance impact. The computed property structure in HelpCenterMenuContent appears well-organized without circular dependencies.
Integration Points
The changes maintain backward compatibility and follow the existing component architecture. The help center UI improvements (new "Give Feedback" button, icon updates) align with the design system evolution. Tests appropriately cover the functionality changes.
Positive Observations
- Consistent use of semantic design tokens throughout all components
- Proper maintenance of existing accessibility attributes (role, aria-label)
- Comprehensive test coverage for the release notification functionality
- Clean separation of concerns between styling and logic changes
- Follows Vue 3 Composition API best practices
Issues to Address
Medium Priority
- Missing documentation for new CSS design tokens - add inline comments explaining purpose
- Unimplemented functionality - feedback button contains TODO comment, should be implemented or removed
- CSS class naming - text-text-primary appears redundant, should be text-primary
- Semantic token usage - verify using text tokens for background colors is semantically correct
Low Priority
- Inline styles - replace with CSS classes for better maintainability
- Accessibility testing - add test cases for ARIA compliance and keyboard navigation
References
- Repository Architecture Guide
- ComfyUI Design System Guidelines
- CLAUDE.md project guidelines in repository
Next Steps
- Address medium priority issues before merge, especially the unimplemented feedback functionality
- Consider adding documentation for design tokens to improve maintainability
- Review CSS class naming consistency across components
- Consider adding accessibility test coverage for enhanced UI components
This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.
github-actions
bot
removed
the
claude-review
1 participant
Summary
Migrated help center and release notification components from hardcoded colors to semantic design tokens for automatic light/dark theme support.
Changes
--interface-menu-surfaceand--interface-menu-stroketokens to style.css for consistent menu theming┆Issue is synchronized with this Notion page by Unito