mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
fix: wait for settings before cloud desktop promo (#10526)
this fixes two issues, setting store race did not await load, and it only cleared shown on clear not on show ## Summary Wait for settings to load before deciding whether to show the one-time macOS desktop cloud promo so the persisted dismissal state is respected on launch. ## Changes - **What**: Await `settingStore.load()` before checking `Comfy.Desktop.CloudNotificationShown`, keep the promo gated to macOS desktop, and persist the shown flag before awaiting dialog close. - **Dependencies**: None ## Review Focus - Launch-time settings race for `Comfy.Desktop.CloudNotificationShown` - One-time modal behavior if the app closes before the dialog is dismissed - Regression coverage in `src/App.test.ts` ## Screenshots (if applicable) - N/A ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10526-fix-wait-for-settings-before-cloud-desktop-promo-32e6d73d365081939fc3ca5b4346b873) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org>
This commit is contained in:
25
src/App.vue
25
src/App.vue
@@ -7,17 +7,15 @@
|
||||
<script setup lang="ts">
|
||||
import { captureException } from '@sentry/vue'
|
||||
import BlockUI from 'primevue/blockui'
|
||||
import { computed, onMounted, onUnmounted, watch } from 'vue'
|
||||
import { computed, onMounted, watch } from 'vue'
|
||||
|
||||
import GlobalDialog from '@/components/dialog/GlobalDialog.vue'
|
||||
import config from '@/config'
|
||||
import { isDesktop } from '@/platform/distribution/types'
|
||||
import { useSettingStore } from '@/platform/settings/settingStore'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useWorkspaceStore } from '@/stores/workspaceStore'
|
||||
import { electronAPI } from '@/utils/envUtil'
|
||||
import { parsePreloadError } from '@/utils/preloadErrorUtil'
|
||||
import { useDialogService } from '@/services/dialogService'
|
||||
import { useConflictDetection } from '@/workbench/extensions/manager/composables/useConflictDetection'
|
||||
|
||||
const workspaceStore = useWorkspaceStore()
|
||||
@@ -129,26 +127,5 @@ onMounted(() => {
|
||||
// Initialize conflict detection in background
|
||||
// This runs async and doesn't block UI setup
|
||||
void conflictDetection.initializeConflictDetection()
|
||||
|
||||
// Show cloud notification for macOS desktop users (one-time)
|
||||
if (isDesktop && electronAPI()?.getPlatform() === 'darwin') {
|
||||
const settingStore = useSettingStore()
|
||||
if (!settingStore.get('Comfy.Desktop.CloudNotificationShown')) {
|
||||
const dialogService = useDialogService()
|
||||
cloudNotificationTimer = setTimeout(async () => {
|
||||
try {
|
||||
await dialogService.showCloudNotification()
|
||||
} catch (e) {
|
||||
console.warn('[CloudNotification] Failed to show', e)
|
||||
}
|
||||
await settingStore.set('Comfy.Desktop.CloudNotificationShown', true)
|
||||
}, 2000)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
let cloudNotificationTimer: ReturnType<typeof setTimeout> | undefined
|
||||
onUnmounted(() => {
|
||||
if (cloudNotificationTimer) clearTimeout(cloudNotificationTimer)
|
||||
})
|
||||
</script>
|
||||
|
||||
@@ -0,0 +1,135 @@
|
||||
import { flushPromises, mount } from '@vue/test-utils'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { nextTick } from 'vue'
|
||||
|
||||
import DesktopCloudNotificationController from './DesktopCloudNotificationController.vue'
|
||||
|
||||
const settingState = {
|
||||
shown: false
|
||||
}
|
||||
|
||||
const settingStore = {
|
||||
load: vi.fn<() => Promise<void>>(),
|
||||
get: vi.fn((key: string) =>
|
||||
key === 'Comfy.Desktop.CloudNotificationShown'
|
||||
? settingState.shown
|
||||
: undefined
|
||||
),
|
||||
set: vi.fn(async (_key: string, value: boolean) => {
|
||||
settingState.shown = value
|
||||
})
|
||||
}
|
||||
|
||||
const dialogService = {
|
||||
showCloudNotification: vi.fn<() => Promise<void>>()
|
||||
}
|
||||
|
||||
const electron = {
|
||||
getPlatform: vi.fn(() => 'darwin')
|
||||
}
|
||||
|
||||
vi.mock('@/platform/distribution/types', () => ({
|
||||
isDesktop: true
|
||||
}))
|
||||
|
||||
vi.mock('@/platform/settings/settingStore', () => ({
|
||||
useSettingStore: () => settingStore
|
||||
}))
|
||||
|
||||
vi.mock('@/services/dialogService', () => ({
|
||||
useDialogService: () => dialogService
|
||||
}))
|
||||
|
||||
vi.mock('@/utils/envUtil', () => ({
|
||||
electronAPI: () => electron
|
||||
}))
|
||||
|
||||
function createDeferred() {
|
||||
let resolve!: () => void
|
||||
const promise = new Promise<void>((res) => {
|
||||
resolve = res
|
||||
})
|
||||
|
||||
return { promise, resolve }
|
||||
}
|
||||
|
||||
describe('DesktopCloudNotificationController', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
vi.clearAllMocks()
|
||||
|
||||
settingState.shown = false
|
||||
electron.getPlatform.mockReturnValue('darwin')
|
||||
settingStore.load.mockResolvedValue(undefined)
|
||||
settingStore.set.mockImplementation(
|
||||
async (_key: string, value: boolean) => {
|
||||
settingState.shown = value
|
||||
}
|
||||
)
|
||||
dialogService.showCloudNotification.mockResolvedValue(undefined)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('waits for settings to load before deciding whether to show the notification', async () => {
|
||||
const loadSettings = createDeferred()
|
||||
settingStore.load.mockImplementation(() => loadSettings.promise)
|
||||
|
||||
const wrapper = mount(DesktopCloudNotificationController)
|
||||
await nextTick()
|
||||
|
||||
settingState.shown = true
|
||||
loadSettings.resolve()
|
||||
|
||||
await flushPromises()
|
||||
await vi.advanceTimersByTimeAsync(2000)
|
||||
|
||||
expect(dialogService.showCloudNotification).not.toHaveBeenCalled()
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
|
||||
it('does not schedule or show the notification after unmounting before settings load resolves', async () => {
|
||||
const loadSettings = createDeferred()
|
||||
settingStore.load.mockImplementation(() => loadSettings.promise)
|
||||
|
||||
const wrapper = mount(DesktopCloudNotificationController)
|
||||
await nextTick()
|
||||
|
||||
wrapper.unmount()
|
||||
loadSettings.resolve()
|
||||
|
||||
await flushPromises()
|
||||
await vi.advanceTimersByTimeAsync(2000)
|
||||
|
||||
expect(settingStore.set).not.toHaveBeenCalled()
|
||||
expect(dialogService.showCloudNotification).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('marks the notification as shown before awaiting dialog close', async () => {
|
||||
const dialogOpen = createDeferred()
|
||||
dialogService.showCloudNotification.mockImplementation(
|
||||
() => dialogOpen.promise
|
||||
)
|
||||
|
||||
const wrapper = mount(DesktopCloudNotificationController)
|
||||
|
||||
await flushPromises()
|
||||
await vi.advanceTimersByTimeAsync(2000)
|
||||
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Desktop.CloudNotificationShown',
|
||||
true
|
||||
)
|
||||
expect(settingStore.set.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
dialogService.showCloudNotification.mock.invocationCallOrder[0]
|
||||
)
|
||||
|
||||
dialogOpen.resolve()
|
||||
await flushPromises()
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,57 @@
|
||||
<script setup lang="ts">
|
||||
import { onMounted, onUnmounted } from 'vue'
|
||||
|
||||
import { isDesktop } from '@/platform/distribution/types'
|
||||
import { useSettingStore } from '@/platform/settings/settingStore'
|
||||
import { useDialogService } from '@/services/dialogService'
|
||||
import { electronAPI } from '@/utils/envUtil'
|
||||
|
||||
const settingStore = useSettingStore()
|
||||
const dialogService = useDialogService()
|
||||
|
||||
let isDisposed = false
|
||||
let cloudNotificationTimer: ReturnType<typeof setTimeout> | undefined
|
||||
|
||||
async function scheduleCloudNotification() {
|
||||
if (!isDesktop || electronAPI()?.getPlatform() !== 'darwin') return
|
||||
|
||||
try {
|
||||
await settingStore.load()
|
||||
} catch (error) {
|
||||
console.warn('[CloudNotification] Failed to load settings', error)
|
||||
return
|
||||
}
|
||||
|
||||
if (isDisposed) return
|
||||
if (settingStore.get('Comfy.Desktop.CloudNotificationShown')) return
|
||||
|
||||
cloudNotificationTimer = setTimeout(async () => {
|
||||
if (isDisposed) return
|
||||
|
||||
try {
|
||||
await settingStore.set('Comfy.Desktop.CloudNotificationShown', true)
|
||||
if (isDisposed) return
|
||||
await dialogService.showCloudNotification()
|
||||
} catch (error) {
|
||||
console.warn('[CloudNotification] Failed to show', error)
|
||||
await settingStore
|
||||
.set('Comfy.Desktop.CloudNotificationShown', false)
|
||||
.catch((resetError) => {
|
||||
console.warn(
|
||||
'[CloudNotification] Failed to reset shown state',
|
||||
resetError
|
||||
)
|
||||
})
|
||||
}
|
||||
}, 2000)
|
||||
}
|
||||
|
||||
onMounted(() => {
|
||||
void scheduleCloudNotification()
|
||||
})
|
||||
|
||||
onUnmounted(() => {
|
||||
isDisposed = true
|
||||
if (cloudNotificationTimer) clearTimeout(cloudNotificationTimer)
|
||||
})
|
||||
</script>
|
||||
@@ -26,6 +26,7 @@
|
||||
<ModelImportProgressDialog />
|
||||
<AssetExportProgressDialog />
|
||||
<ManagerProgressToast />
|
||||
<DesktopCloudNotificationController />
|
||||
<UnloadWindowConfirmDialog v-if="!isDesktop" />
|
||||
<MenuHamburger />
|
||||
</template>
|
||||
@@ -63,6 +64,7 @@ import type { ServerConfig, ServerConfigValue } from '@/constants/serverConfig'
|
||||
import { i18n, loadLocale } from '@/i18n'
|
||||
import AssetExportProgressDialog from '@/platform/assets/components/AssetExportProgressDialog.vue'
|
||||
import ModelImportProgressDialog from '@/platform/assets/components/ModelImportProgressDialog.vue'
|
||||
import DesktopCloudNotificationController from '@/platform/cloud/notification/components/DesktopCloudNotificationController.vue'
|
||||
import { isCloud, isDesktop } from '@/platform/distribution/types'
|
||||
import { useSettingStore } from '@/platform/settings/settingStore'
|
||||
import { useTelemetry } from '@/platform/telemetry'
|
||||
|
||||
Reference in New Issue
Block a user