[feat] Add cloud notification modal for macOS desktop users #6845
+242
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a one-time informational modal that introduces Comfy Cloud to macOS desktop users. The modal emphasizes that ComfyUI remains free and open source, with Cloud as an optional service for GPU access. Key features: - Shows once on first launch for macOS + Electron users - Persistent badge in topbar after dismissal for easy re-access - Clean design with Comfy Cloud branding - Non-intrusive messaging focused on infrastructure benefits 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
deepme987
requested review from
a team,
KarryCharon,
Yorha4D,
pythongosssss and
shinshin86
as code owners
dosubot
bot
added
the
size:L
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughIntroduces a one-time cloud notification feature for Electron users on macOS. Adds a new CloudNotificationContent component, cloud notification badge in the topbar, dialog service method, core setting to track notification display state, and localization strings. The notification displays automatically on first app launch. Changes
Sequence DiagramsequenceDiagram
participant User
participant App as App.vue
participant SettingStore as Setting Store
participant Topbar as TopbarBadges
participant Dialog as Dialog Service
participant Content as CloudNotification<br/>Component
App->>SettingStore: Check Comfy.Desktop.CloudNotificationShown
alt First Load (not shown)
SettingStore-->>App: false
App->>Dialog: showCloudNotification()
Dialog->>Content: Render notification
Content-->>User: Display cloud notification
App->>SettingStore: Set Comfy.Desktop.CloudNotificationShown = true
else Subsequent Loads
SettingStore-->>App: true
App-->>Topbar: Skip init notification
end
rect rgba(200, 220, 255, 0.3)
Note over Topbar,Dialog: Topbar Badge (always available once shown)
Topbar->>SettingStore: Read Comfy.Desktop.CloudNotificationShown
alt Electron + macOS + notification shown
SettingStore-->>Topbar: true
Topbar-->>User: Display cloud badge
User->>Topbar: Click badge
Topbar->>Dialog: handleCloudBadgeClick()
Dialog->>Content: Render notification
Content-->>User: Show dialog
end
end
User->>Content: Dismiss or Explore
Content->>Dialog: closeDialog()
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 11/23/2025, 11:00:13 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/23/2025, 10:49:36 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.19 MB (baseline 3.18 MB) • 🔴 +6.61 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 943 kB (baseline 941 kB) • 🔴 +1.84 kBGraph 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) • ⚪ 0 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: 3 added / 3 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) • 🟢 -21 BExternal libraries and shared vendor chunks
Status: 3 added / 3 removed Other — 3.87 MB (baseline 3.87 MB) • ⚪ 0 BBundles that do not match a named category
Status: 19 added / 19 removed |
![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 (4)
src/locales/en/main.json (1)
2215-2231: cloudNotification i18n block looks good, with one unused keyThe new
cloudNotification.*strings are consistent with the dialog and badge usage. Note thatcloudNotification.badgeTooltipis defined but not used anywhere; either wire it into a tooltip on the badge or remove it to avoid dead i18n keys.src/platform/settings/constants/coreSettings.ts (1)
291-301: Consider marking CloudNotificationShown as hidden/internalThe new setting id and default are correct, but this flag is effectively internal state (“has seen cloud promo”). To avoid cluttering user-facing settings, consider:
{ id: 'Comfy.Desktop.CloudNotificationShown', name: 'Cloud notification shown', type: 'hidden', defaultValue: false, // optionally: versionAdded: '1.xx.x' }If you do intend users to be able to toggle this in the UI (e.g., to re-show/hide the promo), keeping
type: 'boolean'is fine, but then it might deserve a more user-friendly name/category.src/components/topbar/TopbarBadges.vue (1)
3-18: Cloud badge logic works but has redundant state and unused i18nFunctionally, the badge will only render for Electron + macOS once
Comfy.Desktop.CloudNotificationShownistrue, and clicking it re-opens the cloud dialog. That’s consistent with tying the persistent badge to users who’ve already seen the modal, but it does introduce a few nits:
cloudBadgeandTopbarBadgeTypeare only used forv-if="cloudBadge"; the actual label/text/tooltip are hard-coded in the template. Either:
- Drop
cloudBadgeand the type import and just use a simplershouldShowCloudBadgeboolean, or- Reuse
<TopbarBadge :badge="cloudBadge" ...>so the structured badge object actually drives the UI.- You define
cloudNotification.badgeTooltipin i18n but don’t use it; consider adding a tooltip (e.g.,:title="t('cloudNotification.badgeTooltip')"or your existing tooltip directive) or removing the key.- Optional: for robustness and tests, you could guard
navigatorusage, e.g.typeof navigator !== 'undefined' && navigator.platform.toLowerCase().includes('mac').None of these are blockers, but cleaning them up would simplify the badge implementation and ensure all i18n strings are exercised.
If the intention was to show the badge even for users who have not yet seen the modal,
shouldShowCloudBadgeshould likely not depend onhasShownNotification; please confirm product requirements.Also applies to: 65-96
src/components/dialog/content/CloudNotificationContent.vue (1)
1-104: Cloud notification dialog content is solid; consider tightening window.open and closeDialog usageThe dialog layout and i18n wiring look good and align with the new
cloudNotification.*strings. Two minor polish points:
- For
onExplore, consider usingwindow.open('https://www.comfy.org/cloud', '_blank', 'noopener,noreferrer')(and optionally nullingopener) to avoid exposingwindow.openerto the new tab.useDialogStore().closeDialog()is called without a key. If the store supports multiple dialogs at once, you may want to pass{ key: 'global-cloud-notification' }(or whatever key the service uses) to ensure only this dialog is closed.Otherwise this component is straightforward and matches the intended UX.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/App.vue(3 hunks)src/components/dialog/content/CloudNotificationContent.vue(1 hunks)src/components/topbar/TopbarBadges.vue(3 hunks)src/locales/en/main.json(1 hunks)src/platform/settings/constants/coreSettings.ts(1 hunks)src/schemas/apiSchema.ts(1 hunks)src/services/dialogService.ts(3 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). (4)
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: collect
🔇 Additional comments (3)
src/schemas/apiSchema.ts (1)
368-524: CloudNotificationShown setting schema wired correctly
'Comfy.Desktop.CloudNotificationShown': z.boolean()matches the new core setting id and store usage and keeps the Settings type consistent; no issues here.src/App.vue (1)
20-21: One-time macOS/Electron notification flow is coherentUsing
Comfy.Desktop.CloudNotificationShownto gateshowCloudNotification()forisElectron() && isMacOSis consistent and should ensure the modal appears only once per settings store. The asynconMountedchange is localized to awaiting the settings write, so it won’t affect other initialization paths.Also applies to: 51-93
src/services/dialogService.ts (1)
5-5: Cloud notification dialog helper integrates cleanly
showCloudNotification()follows the existing dialogService patterns (unique key, simple config, added to the public API) and cleanly decouples callers from dialogStore details. No issues spotted.Also applies to: 545-553, 555-577
Adds Mixpanel event tracking and UTM parameters to measure: - Modal impression rate (cloud_notification_modal_shown) - Click-through rate (cloud_notification_explore_cloud_clicked) - Dismiss rate (cloud_notification_continue_locally_clicked) - Badge engagement (cloud_notification_badge_clicked) UTM parameters enable full funnel tracking: - Desktop users clicking Explore Cloud - Cloud website signups with utm_source=desktop - Subscription conversions attributed to desktop This enables measurement of the desktop→cloud conversion funnel to validate the 5.6% signup-to-paid conversion rate goal. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Fix badge not appearing by accessing settingStore.settingValues directly for proper reactivity - Increase modal delay to 2s to ensure it appears after missing models dialog - Move setting update inside setTimeout to only save when modal actually shows 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Adds a one-time modal introducing Comfy Cloud to macOS desktop users, with clear messaging that ComfyUI remains free and open source.
Changes
Review Focus
Platform targeting is intentionally macOS + Electron only to minimize backlash and test user reception before broader rollout.
Screenshots (if applicable)
┆Issue is synchronized with this Notion page by Unito