From 68bed804db6939009f87ff3142b5f30ddb9060c3 Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Sat, 13 Jun 2026 00:14:04 -0700 Subject: [PATCH 001/184] feat(updater): default to startup install + installer UI on Windows (#1097) * feat(updater): default startup install + installer UI on Windows Flip installUpdatesOnStartup and showInstallerUI from opt-in (default off) to default-on on Windows, mirroring the autoInstallUpdates opt-out pattern (!== false). Windows now applies staged updates at startup (disabling the crash-prone electron-updater install-on-quit) and shows the NSIS progress window during the install. Set either setting to false in settings.json to opt back out. No-op on macOS/Linux. Amp-Thread-ID: https://ampcode.com/threads/T-019eb80f-cba2-715c-8d1c-b5d98540cbec Co-authored-by: Amp * docs(updater): fix comments stale after default-on flip Amp-Thread-ID: https://ampcode.com/threads/T-019eb80f-cba2-715c-8d1c-b5d98540cbec Co-authored-by: Amp * refactor(updater): share Windows opt-out gate; drop Option B/C wording Addresses CodeRabbit review on #1097: - Extract isWindowsOptOutGate() so the startup-install and installer-UI gates share one win32 + setting !== false check (no drift). - Replace plan-reference 'Option B/C' wording in tests/comments with behavior-based descriptions. Amp-Thread-ID: https://ampcode.com/threads/T-019eb80f-cba2-715c-8d1c-b5d98540cbec Co-authored-by: Amp --------- Co-authored-by: Amp --- src/main/lib/updater.test.ts | 44 +++++++++++------------ src/main/lib/updater.ts | 67 ++++++++++++++++++++---------------- src/main/settings.ts | 29 +++++++--------- 3 files changed, 72 insertions(+), 68 deletions(-) diff --git a/src/main/lib/updater.test.ts b/src/main/lib/updater.test.ts index 86d6ce177..8e946c3c2 100644 --- a/src/main/lib/updater.test.ts +++ b/src/main/lib/updater.test.ts @@ -289,7 +289,7 @@ describe('app-update telemetry dedup (volume regression)', () => { }) /** - * Issue #1065 — install staged Desktop updates at startup (Option C) instead of + * Issue #1065 — install staged Desktop updates at startup instead of * silently on quit, and never spawn the installer while the OS session is * ending. Installing on quit is what a Windows shutdown interrupts mid-write, * corrupting the install and forcing endless reinstalls. @@ -312,9 +312,8 @@ describe('startup update install + session-end guard (issue #1065)', () => { beforeEach(() => { vi.resetModules() settingsStore = {} - // These tests exercise the gated "Option C" startup-install path, so enable - // the local flag. Individual default-mode tests delete it. - settingsStore['installUpdatesOnStartup'] = true + // Startup install and the NSIS installer UI default on (enabled) on Windows. + // Tests that exercise the opt-out path set these to false explicitly. listeners = {} sessionEnding = false readyVersion = null @@ -369,24 +368,26 @@ describe('startup update install + session-end guard (issue #1065)', () => { const findEmitCalls = (event: string): unknown[][] => emitMock.mock.calls.filter((c) => c[0] === event) - it('register() leaves install-on-quit enabled by default (Option B)', async () => { - delete settingsStore['installUpdatesOnStartup'] + it('register() disables install-on-quit by default on Windows when startup install is enabled', async () => { const updater = await import('./updater') updater.register() - // Default mode keeps electron-updater's install-on-quit; it's only suppressed - // when the OS session ends (see suppressInstallOnQuit). - expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(true) + // Startup install is the default on Windows, so install-on-quit is disabled + // entirely and the staged update applies at the next launch. + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false) }) - it('register() disables install-on-quit when the startup-install flag is on (Option C)', async () => { + it('register() keeps install-on-quit when startup install is explicitly opted out', async () => { + settingsStore['installUpdatesOnStartup'] = false const updater = await import('./updater') updater.register() - expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false) + // Opted out: electron-updater's install-on-quit stays armed; it's only + // suppressed when the OS session ends (see suppressInstallOnQuit). + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(true) }) it('startup install is inert on non-Windows even when the flag is on', async () => { // macOS (Squirrel.Mac / ShipIt) and Linux don't have the NSIS shutdown - // corruption, so Option C must stay off there regardless of the setting. + // corruption, so startup install must stay off there regardless of the setting. Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }) settingsStore['installUpdatesOnStartup'] = true settingsStore['pendingDownloadedUpdateVersion'] = '1.0.1' @@ -400,8 +401,8 @@ describe('startup update install + session-end guard (issue #1065)', () => { expect(fakeUpdater.restartAndInstall).not.toHaveBeenCalled() }) - it('suppressInstallOnQuit() disables install-on-quit (Option B session-end guard)', async () => { - delete settingsStore['installUpdatesOnStartup'] + it('suppressInstallOnQuit() disables install-on-quit for the session-end guard', async () => { + settingsStore['installUpdatesOnStartup'] = false const updater = await import('./updater') updater.register() expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(true) @@ -409,8 +410,8 @@ describe('startup update install + session-end guard (issue #1065)', () => { expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false) }) - it('startup install is inert when the flag is off, even with a staged update', async () => { - delete settingsStore['installUpdatesOnStartup'] + it('startup install is inert when opted out, even with a staged update', async () => { + settingsStore['installUpdatesOnStartup'] = false settingsStore['pendingDownloadedUpdateVersion'] = '1.0.1' readyVersion = '1.0.1' const updater = await import('./updater') @@ -428,20 +429,19 @@ describe('startup update install + session-end guard (issue #1065)', () => { expect(fakeUpdater.restartAndInstall).not.toHaveBeenCalled() }) - it('installUpdate() installs silently by default (showInstallerUI off)', async () => { - delete settingsStore['showInstallerUI'] + it('installUpdate() shows the NSIS installer UI by default on Windows', async () => { const updater = await import('./updater') updater.register() updater.installUpdate() - expect(fakeUpdater.restartAndInstall).toHaveBeenCalledWith({ isSilent: true }) + expect(fakeUpdater.restartAndInstall).toHaveBeenCalledWith({ isSilent: false }) }) - it('installUpdate() shows the NSIS installer UI when showInstallerUI is on', async () => { - settingsStore['showInstallerUI'] = true + it('installUpdate() installs silently when showInstallerUI is opted out', async () => { + settingsStore['showInstallerUI'] = false const updater = await import('./updater') updater.register() updater.installUpdate() - expect(fakeUpdater.restartAndInstall).toHaveBeenCalledWith({ isSilent: false }) + expect(fakeUpdater.restartAndInstall).toHaveBeenCalledWith({ isSilent: true }) }) it('installUpdate() ignores showInstallerUI off Windows (isSilent stays true)', async () => { diff --git a/src/main/lib/updater.ts b/src/main/lib/updater.ts index 00d61a6e9..f0ca3188c 100644 --- a/src/main/lib/updater.ts +++ b/src/main/lib/updater.ts @@ -125,6 +125,16 @@ function isSystemPackageInstall(): boolean { return appPath.startsWith('/opt/') || appPath.startsWith('/usr/') } +/** + * Windows-only feature gate that defaults on: enabled unless the setting is + * explicitly `false`. The startup-install and installer-UI gates share this so + * their platform check and opt-out semantics can't drift apart. + */ +function isWindowsOptOutGate(key: 'installUpdatesOnStartup' | 'showInstallerUI'): boolean { + if (process.platform !== 'win32') return false + return settings.get(key) !== false +} + /** * Local, static feature gate for applying a staged update at the next launch * (the "startup install" path) instead of letting electron-updater install it @@ -137,19 +147,15 @@ function isSystemPackageInstall(): boolean { * so applying updates at startup there would only add risk to a working update * channel. On those platforms this always returns false. * - * Default OFF even on Windows. With it off, the app keeps the normal - * install-on-quit behavior; the `session-end` guard (`suppressInstallOnQuit`) - * only suppresses that install while the OS is shutting down. With it on, - * install-on-quit is disabled entirely and the staged update applies on the next - * boot. - * - * Not a remote flag yet — flip it via the hidden `installUpdatesOnStartup` - * setting (edited by hand in settings.json), so the startup-install path can be - * canaried before any wider rollout. + * Default ON on Windows. The staged update applies at startup and + * electron-updater's install-on-quit is disabled entirely. Set the + * `installUpdatesOnStartup` setting to `false` to opt back out to the old + * install-on-quit behavior (where the `session-end` guard, + * `suppressInstallOnQuit`, only suppresses the install while the OS is shutting + * down). */ function isStartupInstallEnabled(): boolean { - if (process.platform !== 'win32') return false - return settings.get('installUpdatesOnStartup') === true + return isWindowsOptOutGate('installUpdatesOnStartup') } /** @@ -164,12 +170,11 @@ function isStartupInstallEnabled(): boolean { * continuous visual feedback during the actual file copy — which our Electron * "Updating…" splash can't, since the copy runs after the app has quit. * - * Default OFF. Not remote yet — flip the hidden `showInstallerUI` setting by - * hand in settings.json to canary it. + * Default ON on Windows. Set the `showInstallerUI` setting to `false` to opt + * back out to a fully silent install. */ function isInstallerUIEnabled(): boolean { - if (process.platform !== 'win32') return false - return settings.get('showInstallerUI') === true + return isWindowsOptOutGate('showInstallerUI') } /** @@ -264,8 +269,8 @@ function bindUpdaterEvents(): void { emitTelemetry('comfy.desktop.app_update.download_complete', { version }) } // Persist that an installer is staged on disk. electron-updater caches the - // download across restarts; this marker lets the startup-install path (when - // enabled) apply it on the next boot. Harmless in the default on-quit mode — + // download across restarts; this marker lets the startup-install path apply + // it on the next boot. Harmless when installing on quit instead — // it's just a record that a download finished and is cleared once the staged // version is the one running. try { @@ -530,8 +535,8 @@ export function installUpdate(): void { app.releaseSingleInstanceLock() } // `isSilent: false` shows the NSIS progress window during the install (see - // `isInstallerUIEnabled` — Windows-only, gated, default off). Off everywhere - // else, so the macOS/Linux paths and the default Windows path stay silent. + // `isInstallerUIEnabled` — Windows-only, default on). Forced silent on + // macOS/Linux, where `isSilent` has no effect anyway. updater.restartAndInstall({ isSilent: !isInstallerUIEnabled() }) } catch (err) { clearQuitReason() @@ -586,8 +591,9 @@ type StartupInstallDecision = * Decide whether to install a staged Desktop update on this launch. Cheap and * synchronous (reads only persisted markers + environment). * - * Returns a skip for: the startup-install gate being off (the default — installs - * still happen on quit), E2E runs, system-package-managed installs (apt/dnf own + * Returns a skip for: the startup-install gate being off (non-Windows, or the + * `installUpdatesOnStartup` opt-out — installs still happen on quit), E2E runs, + * system-package-managed installs (apt/dnf own * the update), an OS session that's already ending, no staged download (or one * that's already the running version), and the loop-breaker case (we already * auto-attempted this exact version and are still on the old one). @@ -730,16 +736,17 @@ export async function applyPendingUpdateOnStartup(splashShownAt?: number): Promi export function register(): void { bindUpdaterEvents() - // Default ("Option B"): keep electron-updater's install-on-quit. A normal - // quit still installs a staged update; the `session-end` guard - // (`suppressInstallOnQuit`) flips `autoInstallOnAppQuit` off only when the OS - // is shutting down, so a Windows shutdown/restart/logoff can't kill the - // installer mid-write (the "reinstall on every shutdown" corruption loop). - // - // Gated ("Option C"): when the startup-install path is enabled, disable + // Startup install (the Windows default): disable electron-updater's // install-on-quit entirely up front — the staged update applies on the next - // launch (`applyPendingUpdateOnStartup`) instead. `electronAutoUpdater` is the - // same singleton the ToDesktop runtime drives, so this affects the real updater. + // launch (`applyPendingUpdateOnStartup`) instead of on quit, which is what a + // Windows shutdown can kill mid-write (the "reinstall on every shutdown" + // corruption loop). `electronAutoUpdater` is the same singleton the ToDesktop + // runtime drives, so this affects the real updater. + // + // Opted out (non-Windows, or `installUpdatesOnStartup` set to false): keep + // install-on-quit armed. A normal quit still installs a staged update; the + // `session-end` guard (`suppressInstallOnQuit`) flips `autoInstallOnAppQuit` + // off only when the OS is shutting down. if (isStartupInstallEnabled()) { suppressInstallOnQuit() } diff --git a/src/main/settings.ts b/src/main/settings.ts index 397fa0002..c9c285643 100644 --- a/src/main/settings.ts +++ b/src/main/settings.ts @@ -61,23 +61,20 @@ export interface KnownSettings { * the same version on the next boot — the user can still install it manually * via the update pill. Cleared once that version is actually running. */ lastStartupUpdateAttemptVersion?: string - /** Hidden, local-only gate (default false / off) for applying a staged Desktop - * update on the next launch instead of letting electron-updater install it on - * quit. Windows-only — ignored on macOS/Linux, whose updaters don't have the - * shutdown install-corruption this addresses. Off: install-on-quit stays armed - * and is only suppressed while the OS is shutting down. On: install-on-quit is - * disabled and the update applies at startup. Not remote yet — flipped by hand - * in settings.json to canary the startup-install path. */ + /** Windows-only gate (default on) for applying a staged Desktop update on the + * next launch instead of letting electron-updater install it on quit. Ignored + * on macOS/Linux, whose updaters don't have the shutdown install-corruption + * this addresses. On (default): install-on-quit is disabled and the update + * applies at startup. Set to `false` to opt back out — install-on-quit stays + * armed and is only suppressed while the OS is shutting down. */ installUpdatesOnStartup?: boolean - /** Hidden, local-only gate (default false / off) for showing the NSIS - * installer's own progress window while an update installs, instead of - * installing fully silently. Windows-only — `isSilent` is an NSIS concept and - * is ignored on macOS/Linux. On update the assisted installer skips the - * welcome/license/directory pages and our `customFinishPage` auto-launches + - * skips the finish page, so the user only sees a progress window (no clicks). - * Gives continuous visual feedback during the real file copy, which our - * Electron "Updating…" splash can't cover (the copy happens after we quit). - * Not remote yet — flipped by hand in settings.json to canary it. */ + /** Windows-only gate (default on) for showing the NSIS installer's own + * progress window while an update installs, instead of installing fully + * silently. Ignored on macOS/Linux — `isSilent` is an NSIS concept. On update + * the assisted installer skips the welcome/license/directory pages and our + * `customFinishPage` auto-launches + skips the finish page, so the user only + * sees a progress window (no clicks). Set to `false` for a fully silent + * install. */ showInstallerUI?: boolean } From badf68e517f61271f3e88d7c14624397667ef390 Mon Sep 17 00:00:00 2001 From: "cloud-code-bot[bot]" <234529496+cloud-code-bot[bot]@users.noreply.github.com> Date: Sat, 13 Jun 2026 00:16:10 -0700 Subject: [PATCH 002/184] chore: bump version to 1.0.19 (#1098) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 44d778cdc..7e40c50b7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "comfyui-desktop-2", - "version": "1.0.19-rc.9", + "version": "1.0.19", "description": "Comfy Desktop", "author": { "name": "Comfy Org", From e56f94b2715c16a8419ad039c463c95f90116d24 Mon Sep 17 00:00:00 2001 From: "cloud-code-bot[bot]" <234529496+cloud-code-bot[bot]@users.noreply.github.com> Date: Sat, 13 Jun 2026 00:34:57 -0700 Subject: [PATCH 003/184] chore: bump version to 1.0.20 (#1100) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7e40c50b7..ea3371d6b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "comfyui-desktop-2", - "version": "1.0.19", + "version": "1.0.20", "description": "Comfy Desktop", "author": { "name": "Comfy Org", From eb05abed948828c16ce6792e2aada33f30083f18 Mon Sep 17 00:00:00 2001 From: Maanil Verma Date: Mon, 15 Jun 2026 15:01:18 +0530 Subject: [PATCH 004/184] Feat/titlebar theme match and version pill (#1102) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(titlebar): match ComfyUI theme inside instances - Drive the title-bar header + OS window-controls overlay from ComfyUI's reported bg while inside an instance, instead of locking to brand purple; symbol color is luminance-derived so the window controls stay legible on any theme - Un-stub `isLight` so the existing `.is-light` chrome activates, and extend it to the resting/hover/open states it never covered (install pill, downloads tray, icon buttons) so nothing washes out or lifts to invisible yellow on a light bar - Scoped to attached instances only — the dashboard/chooser keeps its purple * feat(chooser): show install version alongside the update pill - Render the current version pill independently so an available update no longer hides it (was a v-if/v-else-if chain) - Right-align the update/migrate action so it reads as the affordance, apart from the source + version metadata; version becomes the secondary shrink target after source - Carry the target version in the update label ("Update v0.25.0") via the new `chooser.updatePillVersion` key, sourced from `statusTag.version` * refactor: dedupe luminance + action-dispatch logic (CodeRabbit) - Extract the renderer's canvas-normalize + lightness test into a single `isColorLight` helper (`lib/colorScheme.ts`), reused by `useTitleBarIdentity` and `TitlePopupApp` instead of two copies; it reuses the shared `perceivedLuminance` math - Collapse the guarded `emit('trigger-action', …)` repeated across the update + migrate pills into one `triggerInstallAction` method in `ChooserInstallTile` --- locales/en.json | 1 + locales/zh.json | 1 + src/main/host/attach.ts | 25 +++++---- src/main/lib/theme.ts | 26 +++++++++ .../src/comfyTitleBar/TitleBarApp.vue | 44 +++++++++++++-- .../src/comfyTitleBar/useTitleBarIdentity.ts | 21 ++------ .../src/comfyTitlePopup/TitlePopupApp.vue | 13 +---- src/renderer/src/lib/colorScheme.ts | 22 ++++++++ .../src/views/chooser/ChooserInstallTile.vue | 54 +++++++++++++------ .../src/views/chooser/chooser-tiles.css | 15 +++++- src/shared/colorLuminance.ts | 13 +++++ src/types/ipc.ts | 2 +- 12 files changed, 172 insertions(+), 65 deletions(-) create mode 100644 src/renderer/src/lib/colorScheme.ts create mode 100644 src/shared/colorLuminance.ts diff --git a/locales/en.json b/locales/en.json index aa767527d..486b4e36a 100644 --- a/locales/en.json +++ b/locales/en.json @@ -37,6 +37,7 @@ "filterCloud": "Cloud", "filterRemote": "Remote", "updatePill": "Update", + "updatePillVersion": "Update {version}", "migratePill": "Migrate", "openInstall": "Open", "manageInstall": "Manage", diff --git a/locales/zh.json b/locales/zh.json index a4104ec8f..39e38b596 100644 --- a/locales/zh.json +++ b/locales/zh.json @@ -37,6 +37,7 @@ "filterCloud": "云端", "filterRemote": "远程", "updatePill": "更新", + "updatePillVersion": "更新到 {version}", "migratePill": "迁移", "openInstall": "打开", "manageInstall": "管理", diff --git a/src/main/host/attach.ts b/src/main/host/attach.ts index 19b7650a6..7762178b7 100644 --- a/src/main/host/attach.ts +++ b/src/main/host/attach.ts @@ -5,7 +5,7 @@ import { getModelDownloadContentScript } from '../lib/comfyContentScript' import { getComfyTerminalContentScript } from '../lib/comfyTerminalContentScript' import { closeInstallPopouts } from '../lib/popoutWindows' import { _operationAborts, sourceMap } from '../lib/ipc/shared' -import { TITLEBAR_BG } from '../lib/theme' +import { readableSymbolColor } from '../lib/theme' import * as mainTelemetry from '../lib/telemetry' import { refreshCloudUserTier } from '../lib/userTier' import { noteCloudEntered } from '../lib/cloudEntry' @@ -213,17 +213,16 @@ export function attachInstall(entry: ComfyWindowEntry, opts: AttachInstallOpts): } installationEvents.on('updated', onInstallationUpdated) - // Sync the title bar and overlay colors with the ComfyUI frontend's theme. - // Currently locked to the dark title-bar palette regardless of the - // reported bg/text — the app's title-bar surfaces (Vue pills, - // dropdown popups, tooltips, OS overlay) are dark-only today, and - // pushing a light bg into the OS overlay paints the min/max/close - // symbols light over the still-dark Vue header. The arguments are - // kept so the observer + ipc-message wiring stays intact for a - // future re-introduction of theme tracking. - const applyComfyTheme = (_bg: string, _text: string): void => { + /** + * Paint the Vue header and the OS window-controls overlay from ComfyUI's + * reported `bg` in one call, so the strip behind the min/max/close controls + * stays seamless with the bar (the #647 divergence). `symbolColor` is + * luminance-derived to keep the glyphs legible on any theme. Instance-only — + * the install-less chooser keeps `--titlebar-bg`. + */ + const applyComfyTheme = (bg: string): void => { if (comfyWindow.isDestroyed()) return - const theme = { bg: TITLEBAR_BG, text: '#dddddd' } + const theme = { bg, text: readableSymbolColor(bg) } entry.lastTheme = theme if (!titleBarView.webContents.isDestroyed()) { titleBarView.webContents.send('comfy-titlebar:theme-changed', theme) @@ -240,8 +239,8 @@ export function attachInstall(entry: ComfyWindowEntry, opts: AttachInstallOpts): ...args: unknown[] ): void => { if (channel === 'desktop2-theme-report') { - const { bg, text } = (args[0] || {}) as { bg?: string; text?: string } - if (bg) applyComfyTheme(bg, text || '#ddd') + const { bg } = (args[0] || {}) as { bg?: string; text?: string } + if (bg) applyComfyTheme(bg) } } comfyContents.on('ipc-message', onIpcMessage) diff --git a/src/main/lib/theme.ts b/src/main/lib/theme.ts index cc7a11ab7..3f78f9707 100644 --- a/src/main/lib/theme.ts +++ b/src/main/lib/theme.ts @@ -1,5 +1,7 @@ // Shared main-process color constants. Brand colors match the frontend design system. +import { perceivedLuminance, LUMINANCE_LIGHT_THRESHOLD } from '../../shared/colorLuminance' + /** ComfyUI "Electric Yellow" — `--color-brand-yellow` / `--color-electric-400` in the frontend. */ export const BRAND_YELLOW = '#F0FF41' @@ -16,6 +18,30 @@ export const COMFY_BG = '#171717' * so the OS window-controls overlay matches the Vue `.title-bar` on every window. */ export const TITLEBAR_BG = '#211927' +/** Window-control symbol color (`#dddddd` on dark backgrounds, `#333333` on light) chosen by the + * perceived luminance of `bg` so the min/max/close glyphs stay legible against any reported + * ComfyUI theme. Accepts `#rgb` / `#rrggbb` / `rgb()` / `rgba()`; falls back to the dark-safe + * light glyph on any parse failure. */ +export function readableSymbolColor(bg: string): string { + const rgb = parseColor(bg) + if (!rgb) return '#dddddd' + const [r, g, b] = rgb + return perceivedLuminance(r, g, b) >= LUMINANCE_LIGHT_THRESHOLD ? '#333333' : '#dddddd' +} + +function parseColor(input: string): [number, number, number] | null { + const s = input.trim().toLowerCase() + const hex = s.match(/^#([0-9a-f]{3}|[0-9a-f]{6})$/) + if (hex?.[1]) { + const h = hex[1] + const full = h.length === 3 ? h.replace(/./g, (c) => c + c) : h + return [parseInt(full.slice(0, 2), 16), parseInt(full.slice(2, 4), 16), parseInt(full.slice(4, 6), 16)] + } + const rgb = s.match(/^rgba?\(\s*([\d.]+)\s*,\s*([\d.]+)\s*,\s*([\d.]+)/) + if (rgb) return [Number(rgb[1]), Number(rgb[2]), Number(rgb[3])] + return null +} + export interface SplashTheme { bg: string fg: string diff --git a/src/renderer/src/comfyTitleBar/TitleBarApp.vue b/src/renderer/src/comfyTitleBar/TitleBarApp.vue index dc1f457c5..4fc5175e6 100644 --- a/src/renderer/src/comfyTitleBar/TitleBarApp.vue +++ b/src/renderer/src/comfyTitleBar/TitleBarApp.vue @@ -214,6 +214,7 @@ const isInstallLess = ref((bridge?.getInstallationId() ?? '') === '') const { installLabel, sourceCategory, + themeBg, themeText, isFullscreen, firstUseMode, @@ -645,6 +646,8 @@ onUnmounted(() => { :data-collapse-mode="collapseMode" :style="{ color: themeText ?? undefined, + '--titlebar-bg-active': themeBg ?? undefined, + '--titlebar-icon': themeText ?? undefined, '--title-trailing-width': `${trailingWidthPx}px` }" > @@ -906,7 +909,10 @@ onUnmounted(() => { reserves 140px on the right for native min/max/close. */ padding: 0 12px; box-sizing: border-box; - background: var(--titlebar-bg); + /* `--titlebar-bg-active` is set inline from the reported ComfyUI bg while + inside an instance; install-less / pre-report hosts fall back to the + static brand `--titlebar-bg`. */ + background: var(--titlebar-bg-active, var(--titlebar-bg)); color: var(--text-muted); border-bottom: 1px solid var(--border); font: 12px/1 var(--font-sans, 'Inter', system-ui, sans-serif); @@ -1084,6 +1090,27 @@ onUnmounted(() => { border-color: var(--neutral-50); color: var(--neutral-50); } +/* Light comfy theme: the resting surface (`rgba(255,255,255,.04)`) and text + * (`--neutral-100`, light grey) vanish on a white bar. Swap to a faint dark + * film + dark text so the pill — and its `currentColor` brand mark / caret — + * stay legible. Mirrors the `.is-light` treatment on the update chip. */ +.title-bar.is-light .title-install-pill { + background: rgba(0, 0, 0, 0.04); + color: var(--neutral-700); +} +.title-bar.is-light.is-hover-active .title-install-pill.is-interactive:hover:not(.is-open), +.title-bar.is-light .title-install-pill.is-interactive:focus-visible:not(.is-open) { + background: rgba(0, 0, 0, 0.07); + border-color: color-mix(in srgb, var(--neutral-700) 35%, transparent); + color: var(--neutral-700); +} +/* Open state: the dark-theme yellow lift (`--neutral-50`) is near-invisible on + * a white bar, so commit border + text (and the `currentColor` mark / caret) + * to full-strength dark instead. */ +.title-bar.is-light .title-install-pill.is-interactive.is-open { + border-color: color-mix(in srgb, var(--neutral-700) 55%, transparent); + color: var(--neutral-700); +} /* Inline instance-update CTA, sitting just after the install name inside * the identity pill. Brand-yellow chip so it reads as the actionable @@ -1325,8 +1352,11 @@ onUnmounted(() => { .title-bar.is-hover-active .title-downloads-tray:hover:not(:disabled) { color: var(--comfy-yellow); } -.title-bar.is-light .title-downloads-tray { - color: var(--comfy-yellow); +/* Light comfy theme: yellow hover/open lift is near-invisible on a white bar, + * so deepen the muted resting icon to full-strength dark text instead — the + * same active cue, kept in the light theme's color family. */ +.title-bar.is-light.is-hover-active .title-downloads-tray:hover:not(:disabled) { + color: var(--neutral-700); } .title-downloads-badge { @@ -1347,7 +1377,7 @@ onUnmounted(() => { border-radius: 999px; /* Subtle ring against the title-bar background so the badge reads * as a separate token from the icon underneath at any zoom. */ - box-shadow: 0 0 0 2px var(--titlebar-bg, var(--neutral-900)); + box-shadow: 0 0 0 2px var(--titlebar-bg-active, var(--titlebar-bg, var(--neutral-900))); background: var(--accent, #60a5fa); color: #fff; font-size: 9px; @@ -1373,6 +1403,10 @@ onUnmounted(() => { .title-bar.is-hover-active .title-downloads-tray.is-open:hover { color: var(--neutral-50); } +.title-bar.is-light .title-downloads-tray.is-open, +.title-bar.is-light.is-hover-active .title-downloads-tray.is-open:hover { + color: var(--neutral-700); +} .title-downloads-tray.is-flashing .title-downloads-badge { animation: @@ -1422,7 +1456,7 @@ onUnmounted(() => { height: 8px; border-radius: 999px; background: var(--danger); - box-shadow: 0 0 0 2px var(--titlebar-bg, var(--neutral-900)); + box-shadow: 0 0 0 2px var(--titlebar-bg-active, var(--titlebar-bg, var(--neutral-900))); pointer-events: none; } diff --git a/src/renderer/src/comfyTitleBar/useTitleBarIdentity.ts b/src/renderer/src/comfyTitleBar/useTitleBarIdentity.ts index 1100541a0..545580225 100644 --- a/src/renderer/src/comfyTitleBar/useTitleBarIdentity.ts +++ b/src/renderer/src/comfyTitleBar/useTitleBarIdentity.ts @@ -7,6 +7,7 @@ import { isLoadingLockdownMode, type FirstUseMode, } from '../../../shared/firstUseMode' +import { isColorLight } from '../lib/colorScheme' interface InstallTypeMeta { icon: ReturnType['icon'] @@ -88,23 +89,9 @@ export function useTitleBarIdentity(opts: UseTitleBarIdentityOpts): TitleBarIden const showBrandMark = computed(() => opts.isInstallLess.value && !isPreviewMode.value) - /** Locked to `false`: the title-bar surface is the dark token in both themes, so - * light hover variants would produce light chrome on a dark bar. */ - const isLight = computed(() => false) - // Original luminance test, kept inline for the restoration. - // const isLight = computed(() => { - // const bg = themeBg.value - // if (!bg) return false - // const ctx = document.createElement('canvas').getContext('2d') - // if (!ctx) return false - // ctx.fillStyle = bg - // const hex = ctx.fillStyle as string - // if (!hex.startsWith('#') || hex.length < 7) return false - // const r = parseInt(hex.slice(1, 3), 16) - // const g = parseInt(hex.slice(3, 5), 16) - // const b = parseInt(hex.slice(5, 7), 16) - // return (r * 299 + g * 587 + b * 114) / 1000 >= 128 - // }) + /** True when the reported ComfyUI bg is light, so the title bar's `.is-light` chrome + * variants (lighter hover/pills/chips) kick in to stay legible on the matching surface. */ + const isLight = computed(() => isColorLight(themeBg.value)) let unsubTitle: (() => void) | undefined let unsubSourceCategory: (() => void) | undefined diff --git a/src/renderer/src/comfyTitlePopup/TitlePopupApp.vue b/src/renderer/src/comfyTitlePopup/TitlePopupApp.vue index 2da9ea398..334efb209 100644 --- a/src/renderer/src/comfyTitlePopup/TitlePopupApp.vue +++ b/src/renderer/src/comfyTitlePopup/TitlePopupApp.vue @@ -11,6 +11,7 @@ import { dismissPickerModals } from './dismissPickerModals' import { popupLocaleSource } from './pickerSettingsApiShim' import { useAppLocale } from '../lib/useAppLocale' import type { DetailSection, SnapshotListData } from '../types/ipc' +import { isColorLight } from '../lib/colorScheme' // Title-bar dropdown popup shell. Hosts every title-bar dropdown in one // reused transparent WebContentsView attached to the host window. Each open @@ -221,17 +222,7 @@ const globalSettingsSnapshot = ref({ const downloadsState = ref({ active: [], recent: [] }) /** Body-luminance test driving is-light styling; matches TitleBarApp.vue. */ -const isLight = computed(() => { - const ctx = document.createElement('canvas').getContext('2d') - if (!ctx) return false - ctx.fillStyle = themeBg.value - const hex = ctx.fillStyle as string - if (!hex.startsWith('#') || hex.length < 7) return false - const r = parseInt(hex.slice(1, 3), 16) - const g = parseInt(hex.slice(3, 5), 16) - const b = parseInt(hex.slice(5, 7), 16) - return (r * 299 + g * 587 + b * 114) / 1000 >= 128 -}) +const isLight = computed(() => isColorLight(themeBg.value)) function handleActivate(id: string): void { bridge?.activate(id) diff --git a/src/renderer/src/lib/colorScheme.ts b/src/renderer/src/lib/colorScheme.ts new file mode 100644 index 000000000..9b71866d7 --- /dev/null +++ b/src/renderer/src/lib/colorScheme.ts @@ -0,0 +1,22 @@ +import { perceivedLuminance, LUMINANCE_LIGHT_THRESHOLD } from '../../../shared/colorLuminance' + +/** + * Whether a CSS color reads as "light" (so chrome should switch to its dark/`.is-light` + * variant). Normalises any CSS color to `#rrggbb` via a throwaway canvas — the only reliable + * way to resolve named/`rgb()`/`hsl()` inputs in the renderer — then runs the shared + * perceived-luminance test. Returns `false` for empty / unresolvable colors so the default + * stays dark. Renderer-only (depends on `document`); main uses `readableSymbolColor` in + * `src/main/lib/theme.ts`, which shares the same luminance math. + */ +export function isColorLight(color: string | null | undefined): boolean { + if (!color) return false + const ctx = document.createElement('canvas').getContext('2d') + if (!ctx) return false + ctx.fillStyle = color + const hex = ctx.fillStyle as string + if (!hex.startsWith('#') || hex.length < 7) return false + const r = parseInt(hex.slice(1, 3), 16) + const g = parseInt(hex.slice(3, 5), 16) + const b = parseInt(hex.slice(5, 7), 16) + return perceivedLuminance(r, g, b) >= LUMINANCE_LIGHT_THRESHOLD +} diff --git a/src/renderer/src/views/chooser/ChooserInstallTile.vue b/src/renderer/src/views/chooser/ChooserInstallTile.vue index b0405ea5f..662fbafcf 100644 --- a/src/renderer/src/views/chooser/ChooserInstallTile.vue +++ b/src/renderer/src/views/chooser/ChooserInstallTile.vue @@ -54,6 +54,15 @@ const statusPill = computed<{ label: string; dotClass: string } | null>(() => { }) const hasUpdate = computed(() => inst.value.statusTag?.style === 'update') +/** "Update v0.25.0" when the backend tags the target version, else the bare + * "Update" — the action stays self-describing without hiding the current + * version pill beside it. */ +const updatePillLabel = computed(() => { + const version = inst.value.statusTag?.version + return version + ? t('chooser.updatePillVersion', { version }) + : t('chooser.updatePill') +}) // The backend tags every migratable install (Legacy Desktop, portable, git) // with a `migrate` status tag — mirror `hasUpdate` rather than special-casing // a single source. @@ -74,6 +83,13 @@ function handleClick(): void { if (isStopping.value) return emit('pick', inst.value) } + +/** Fire an action pill's emit, no-op while REQUIRES_STOPPED actions are gated. + * Shared by the update + migrate pills' click / enter / space handlers. */ +function triggerInstallAction(action: 'update' | 'migrate'): void { + if (props.isStoppedActionGated) return + emit('trigger-action', action, inst.value) +} diff --git a/src/renderer/src/views/chooser/chooser-tiles.css b/src/renderer/src/views/chooser/chooser-tiles.css index 51b2a3287..19a875305 100644 --- a/src/renderer/src/views/chooser/chooser-tiles.css +++ b/src/renderer/src/views/chooser/chooser-tiles.css @@ -46,6 +46,16 @@ .chooser-tile:focus-visible .chooser-tile-meta { color: var(--text); } +.chooser-tile:hover .chooser-tile-meta-line, +.chooser-tile:focus-visible .chooser-tile-meta-line, +.chooser-tile:hover .chooser-tile-meta-source, +.chooser-tile:focus-visible .chooser-tile-meta-source, +.chooser-tile:hover .chooser-tile-meta-sep, +.chooser-tile:focus-visible .chooser-tile-meta-sep, +.chooser-tile:hover .chooser-tile-recency-text, +.chooser-tile:focus-visible .chooser-tile-recency-text { + color: var(--text-muted); +} .chooser-tile:hover .chooser-tile-icon, .chooser-tile:focus-visible .chooser-tile-icon { /* Drop the resting opacity so the icon matches the brightened text. */ @@ -64,35 +74,108 @@ color: var(--neutral-100); } +.chooser-tile--install .chooser-tile-body { + margin-top: auto; + display: flex; + flex-direction: column; + gap: 4px; + width: 100%; + min-width: 0; +} + .chooser-tile-name { font-size: 16px; font-weight: 600; display: block; color: var(--neutral-100); - /* Reserve space for the type icon at top-left so name doesn't collide. */ + /* Reserve space for the type icon at top-left so name doesn't collide + * (New-Install / Cloud tiles, where the icon is absolute). */ margin-top: auto; /* Single-line ellipsis: long install names (cloud workspace names, * user-renamed installs) must not blow out the tile's fixed width - * or wrap into the pill row below. */ + * or wrap into the rows below. Highest truncation priority — only the + * name's own length ellipsizes it, never a long source/meta line. */ max-width: 100%; min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } +.chooser-tile--install .chooser-tile-name { + margin-top: 0; +} +.chooser-tile-name-tip { + display: flex; + max-width: 100%; + min-width: 0; +} +.chooser-tile-name-tip .chooser-tile-name { + min-width: 0; +} +.chooser-tile-meta-tip { + display: flex; + max-width: 100%; + min-width: 0; +} +.chooser-tile-meta-tip .chooser-tile-meta-line { + display: block; + min-width: 0; +} -.chooser-tile-meta { - /* Single-row pill layout: no wrap, so a long source pill shrinks - * (via the pill's own max-width / min-width: 0) and triggers - * ellipsis. Wrapping pills would otherwise size each one to its - * content and defeat truncation. */ +.chooser-tile-meta-line { + font-size: 12px; + color: var(--text-faint); + max-width: 100%; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} +.chooser-tile-meta-source { + color: var(--text-faint); +} +.chooser-tile-meta-sep { + margin: 0 5px; + color: var(--text-faint); +} +.chooser-tile-meta-version { + color: var(--text-muted); + font-weight: 500; +} +.chooser-tile:hover .chooser-tile-meta-version, +.chooser-tile:focus-visible .chooser-tile-meta-version { + color: var(--text); +} + +.chooser-tile-footer { display: flex; - flex-wrap: nowrap; - gap: 6px; + align-items: center; + gap: 8px; + width: 100%; + min-width: 0; +} +.chooser-tile-recency { + min-width: 0; +} +.chooser-tile-recency-text { + display: block; + font-size: 11px; + color: var(--text-faint); + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +/* New-Install / Cloud tiles: simple single-line text meta (no pills). */ +.chooser-tile-meta { width: 100%; min-width: 0; font-size: 12px; color: var(--neutral-100); + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; } .chooser-tile-pill { @@ -113,31 +196,14 @@ text-overflow: ellipsis; white-space: nowrap; } -.chooser-tile-pill-version { - font-family: ui-monospace, SFMono-Regular, Menlo, Consolas, monospace; - /* Secondary shrink target: with source + version + a (possibly - * versioned) action pill in one no-wrap row, the source pill - * ellipsizes first, then the version. The action pill holds its - * width (see `.chooser-tile-pill-action`). */ - flex-shrink: 1; - min-width: 0; -} - -/* Action pill (update / migrate) — right-aligned so it sits apart from the - * source + version metadata cluster (App-Store-style action), and holds its +/* Action pill (update / migrate) — pinned right in the footer row so it + * reads as the affordance, apart from the quiet recency text, and holds its * width so the interactive label never truncates. */ .chooser-tile-pill-action { margin-left: auto; flex-shrink: 0; } -/* Launched-time pill — secondary info, lower contrast so it doesn't - * compete with source/action pills. Holds its width like the others. */ -.chooser-tile-pill-launched { - flex-shrink: 0; - opacity: 0.7; -} - /* Update-available pill — accent-blue tinted, click-target. */ .chooser-tile-pill-update { gap: 4px; From 77ffdb61582c1bc639d61846a47e698932847251 Mon Sep 17 00:00:00 2001 From: Maanil Verma Date: Tue, 16 Jun 2026 10:46:50 +0530 Subject: [PATCH 009/184] feat(downloads): serve "View All Downloads" as a title-bar popup (#1123) * refactor(title-popup): hoist popup kind tags into a shared constant Add `POPUP_KIND` and `PopupTheme` to types/ipc.ts so the popup config discriminant has one source across main, preload, and the renderer instead of literal strings re-typed in each. * feat(title-popup): add the centred downloads-full popup kind - Open "View All Downloads" as a large centred backdrop popup on the reused title-bar view, fed by the existing tray download broadcast - Generalise global-settings sizing into `computeCenteredCardBounds` + `kindIsCentered`, shared by the open path and the resize refit * fix(title-popup): stop the preload validators dropping new popup kinds The config and will-show validators gated on a hardcoded kind allowlist, silently rejecting any kind they didn't enumerate. Key them off `POPUP_KIND` so the renderer actually receives `downloads-full`. * feat(downloads): render the full view inside the title-bar popup - Move the DownloadsModal design into `DownloadsFullView`, rewired from the panel store / window.api to the popup's prop + `__comfyTitlePopup` bridge, and replace BaseModal chrome with the popup card - Drop the per-row entrance animation: its lingering transform layer blocked pointer hit-testing in the transparent popup view - Wire the `downloads-full` branch + card chrome into TitlePopupApp * refactor(panel): drop the downloads-v2 overlay panel path The full Downloads view no longer mounts in the panel renderer, so remove the `downloads-v2` body mode and its DownloadsModal mount across the panel, registry, and host-window layout. `feedback` stays the lone overlay mode. * test(downloads): cover the full-popup hand-off; drop downloads-v2 fixtures - Add e2e for "View All Downloads" opening the full popup with seeded rows and for its empty state - Point the panel-key test fixtures at `feedback` and remove the dead DownloadsModal mock now that the component is gone * fix(downloads): restore hover/cursor in the full popup Chromium skips mousemove delivery to the transparent popup WebContentsView when nothing subscribes to pointer-move, freezing :hover and the cursor until a click forces a hit-test. Add a no-op @mousemove subscriber, and move the close button in-flow into the header (dropping the panel's position:relative stacking context) to match the working global-settings view. * test(downloads): de-flake the full-popup e2e hand-off Wait for the tray footer link and stable bounds before clicking "View All", so the kind-switch isn't raced on the empty (fast-rendering) path. * refactor(downloads): address review nits in the full popup - Import the download entry / state / action types from the preload bridge instead of re-declaring them locally - Memoise the status badge into a lookup so it isn't recomputed three times per row - Extract the tray -> "View All" e2e flow into a shared helper * refactor(title-popup): key remaining kind checks off POPUP_KIND, dedup download types Amp-Thread-ID: https://ampcode.com/threads/T-019ececa-4fd3-76fe-ac8b-1f37facb91a6 Co-authored-by: Amp --------- Co-authored-by: Jedrzej Kosinski Co-authored-by: Amp --- e2e/downloads-shelf.test.ts | 52 ++ src/main/host/createHostWindow.ts | 11 +- src/main/host/panelView.test.ts | 10 +- src/main/host/registry.test.ts | 4 +- src/main/host/registry.ts | 3 - src/main/index.ts | 8 +- src/main/popups/titlePopup.ts | 265 +++--- src/preload/comfyTitlePopupPreload.ts | 52 +- .../src/comfyTitlePopup/DownloadsFullView.vue | 749 +++++++++++++++++ .../src/comfyTitlePopup/DownloadsView.test.ts | 4 +- .../src/comfyTitlePopup/TitlePopupApp.vue | 42 +- .../src/components/DownloadsModal.vue | 767 ------------------ src/renderer/src/panel/PanelApp.test.ts | 8 - src/renderer/src/panel/PanelApp.vue | 25 +- src/renderer/src/panel/usePanelOverlays.ts | 2 - src/types/ipc.ts | 18 + 16 files changed, 1003 insertions(+), 1017 deletions(-) create mode 100644 src/renderer/src/comfyTitlePopup/DownloadsFullView.vue delete mode 100644 src/renderer/src/components/DownloadsModal.vue diff --git a/e2e/downloads-shelf.test.ts b/e2e/downloads-shelf.test.ts index 6c523e9d4..0993f18d4 100644 --- a/e2e/downloads-shelf.test.ts +++ b/e2e/downloads-shelf.test.ts @@ -314,6 +314,46 @@ test('the open popup repaints live when tray state changes @windows @macos @linu }).toBe(2) }) +// --------------------------------------------------------------------------- +// "View All Downloads" — the tray footer link reopens the SAME popup view as +// the large centred `downloads-full` kind. Regression guard for the preload +// config-validator allowlist that silently dropped the new kind, leaving the +// renderer stuck on the tray `downloads` view. +// --------------------------------------------------------------------------- + +test('View All Downloads opens the full downloads popup with the seeded rows @windows @macos @linux', async () => { + await seedDownloads(ctx.app, { + active: [makeEntry({ url: 'u-dl', filename: 'dl.safetensors', status: 'downloading', progress: 0.5 })], + recent: [ + makeEntry({ url: 'u-co', filename: 'co.safetensors', status: 'completed', progress: 1, savePath: '/tmp/co.safetensors' }), + makeEntry({ url: 'u-er', filename: 'er.safetensors', status: 'error', progress: 0, error: 'boom' }), + ], + }) + await openFullDownloadsPopup() + + // The reused view flips to the `downloads-full` kind: the tray markup is + // gone and the full `.dlm-panel` is rendered with every seeded row. + expect(await popup.count('.downloads')).toBe(0) + await expect.poll(() => popup.count('.dlm-row'), { + timeout: 5_000, + intervals: [100, 200, 400], + }).toBe(3) + + // Filter chips appear once more than one status bucket is present + // (active + completed + error here), and the footer summarises counts. + expect(await popup.count('.dlm-filter-chip')).toBeGreaterThan(0) + expect(await popup.count('.dlm-footer')).toBe(1) +}) + +test('the full downloads popup shows the empty state when nothing is seeded @windows @macos @linux', async () => { + await openFullDownloadsPopup() + + expect(await popup.count('.dlm-empty')).toBe(1) + expect(await popup.count('.dlm-row')).toBe(0) + // No footer summary bar when there are no downloads. + expect(await popup.count('.dlm-footer')).toBe(0) +}) + // --------------------------------------------------------------------------- // Helpers — kept inline so the test file stays self-contained; promote // to `support/` if a second test file ends up needing the same logic. @@ -343,6 +383,18 @@ async function waitForPopupVisible(app: ElectronApplication): Promise { }).toBe(true) } +/** Open the tray and click its "View All" footer to flip the reused popup view + * to the `downloads-full` kind. Waits for the footer link and stable bounds + * first so the kind-switch isn't raced on the empty (fast-rendering) path. */ +async function openFullDownloadsPopup(): Promise { + await openDownloadsTray(ctx.titleBar) + await waitForPopupVisible(ctx.app) + await popup.waitForSelector('.downloads-link', { timeout: 5_000 }) + await waitForStableBounds(ctx.app) + expect(await popup.clickByText('.downloads-link', 'View All')).toBe(true) + await popup.waitForSelector('.dlm-panel', { timeout: 5_000 }) +} + /** Wait until the popup's bounds height stops changing for `settleMs`, * which is how we know the renderer's `request-size` round-trip has * landed and main has applied the natural-content height. */ diff --git a/src/main/host/createHostWindow.ts b/src/main/host/createHostWindow.ts index 1a29b84f8..039d0ab8c 100644 --- a/src/main/host/createHostWindow.ts +++ b/src/main/host/createHostWindow.ts @@ -680,13 +680,10 @@ export function createHostWindow(opts: CreateHostWindowOpts): CreateHostWindowRe // hosts, so the install-backed visibility branch handles both. const mode = entry ? computeBodyMode(entry) : 'comfy' const showPanel = mode !== 'comfy' - // `'downloads-v2'` and `'feedback'` are overlay modes — their modal - // mounts over the live ComfyUI canvas, so unlike other panel modes - // we keep `comfyView` visible underneath at full bodyRect. The - // panel renderer paints itself transparent (see `PanelApp.vue`'s - // `panel-overlay-mode` body class) except for the modal + dim - // backdrop, so the canvas composites through on macOS CALayers. - const isOverlayMode = mode === 'downloads-v2' || mode === 'feedback' + /** Overlay mode mounts a modal over the live canvas, kept visible underneath at full + * bodyRect; the panel paints transparent (PanelApp's `panel-overlay-mode`) so it + * composites through on macOS CALayers. */ + const isOverlayMode = mode === 'feedback' if (showPanel && entry?.panelView) { entry.panelView.setBounds(bodyRect) entry.panelView.setVisible(true) diff --git a/src/main/host/panelView.test.ts b/src/main/host/panelView.test.ts index 6708e5761..827a4e8fd 100644 --- a/src/main/host/panelView.test.ts +++ b/src/main/host/panelView.test.ts @@ -113,21 +113,21 @@ afterEach(() => { describe('setActivePanel', () => { it('no-ops when the requested panel is already active', () => { - const fixture = makeEntry({ activePanel: 'downloads-v2' }) + const fixture = makeEntry({ activePanel: 'feedback' }) comfyWindows.set(fixture.entry.windowKey, fixture.entry) - setActivePanel(fixture.entry.windowKey, 'downloads-v2') + setActivePanel(fixture.entry.windowKey, 'feedback') expect(fixture.layoutCalls).toBe(0) expect(fixture.titleBarWc.sent).toHaveLength(0) }) it('no-ops when the windowKey does not resolve to an entry', () => { - expect(() => setActivePanel(999_999, 'downloads-v2')).not.toThrow() + expect(() => setActivePanel(999_999, 'feedback')).not.toThrow() }) it('no-ops when the host window has been destroyed', () => { const fixture = makeEntry({ activePanel: 'comfy', destroyed: true }) comfyWindows.set(fixture.entry.windowKey, fixture.entry) - setActivePanel(fixture.entry.windowKey, 'downloads-v2') + setActivePanel(fixture.entry.windowKey, 'feedback') expect(fixture.layoutCalls).toBe(0) expect(fixture.entry.activePanel).toBe('comfy') }) @@ -147,7 +147,7 @@ describe('refreshComfyTabBody', () => { }) it('no-ops when the entry is currently parked on a non-comfy panel', () => { - const fixture = makeEntry({ installationId: 'inst-1', activePanel: 'downloads-v2' }) + const fixture = makeEntry({ installationId: 'inst-1', activePanel: 'feedback' }) comfyWindows.set(fixture.entry.windowKey, fixture.entry) indexInstallationId('inst-1', fixture.entry.windowKey) refreshComfyTabBody('inst-1') diff --git a/src/main/host/registry.test.ts b/src/main/host/registry.test.ts index f0a03d174..5767b2852 100644 --- a/src/main/host/registry.test.ts +++ b/src/main/host/registry.test.ts @@ -146,8 +146,8 @@ describe('computeBodyMode', () => { }) it('passes non-comfy panels through for install-less hosts', () => { - const entry = makeEntry({ installationId: null, activePanel: 'downloads-v2' }) - expect(computeBodyMode(entry)).toBe('downloads-v2') + const entry = makeEntry({ installationId: null, activePanel: 'feedback' }) + expect(computeBodyMode(entry)).toBe('feedback') }) it('routes the comfy pill to comfy when the install session is running', () => { diff --git a/src/main/host/registry.ts b/src/main/host/registry.ts index 8cb1e500a..356091b34 100644 --- a/src/main/host/registry.ts +++ b/src/main/host/registry.ts @@ -17,7 +17,6 @@ export const hostInstallEvents = new EventEmitter() */ export type ComfyPanelKey = | 'comfy' - | 'downloads-v2' | 'feedback' | 'new-install' | 'track' @@ -30,7 +29,6 @@ export type ComfyPanelKey = export const VALID_PANELS: ReadonlySet = new Set([ 'comfy', - 'downloads-v2', 'feedback', 'new-install', 'track', @@ -47,7 +45,6 @@ export const VALID_PANELS: ReadonlySet = new Set([ export type BodyMode = | 'comfy' | 'comfy-lifecycle' - | 'downloads-v2' | 'feedback' | 'chooser' /** Mirror of the `'progress'` ComfyPanelKey; forces the panel to fully cover diff --git a/src/main/index.ts b/src/main/index.ts index 0b5a7c60f..c949eea5f 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -996,11 +996,9 @@ function _broadcastDownloadsToTitleBars(): void { function triggerOpenFeedback(entryId: number, source: 'titlebar' | 'menu'): void { const parentEntry = comfyWindows.get(entryId) if (!parentEntry || parentEntry.window.isDestroyed()) return - // Flip into the 'feedback' overlay panel — same pattern as - // 'downloads-v2'. setActivePanel lazily ensures the panel view, - // makes it visible over comfyView, and broadcasts `panel-switch` to - // the renderer. The IPC below carries the click `source` so the - // renderer's telemetry payload can distinguish titlebar vs. menu. + // Flip into the 'feedback' overlay panel. setActivePanel lazily ensures the + // panel view, makes it visible over comfyView, and broadcasts `panel-switch`. + // The IPC below carries the click `source` for telemetry (titlebar vs. menu). const panelView = parentEntry.panelView ?? ensurePanelView(entryId, parentEntry, 'feedback') setActivePanel(entryId, 'feedback') sendToPanelDeferred(panelView, 'comfy-panel:open-feedback', { source }) diff --git a/src/main/popups/titlePopup.ts b/src/main/popups/titlePopup.ts index 38f287905..0be6e91d1 100644 --- a/src/main/popups/titlePopup.ts +++ b/src/main/popups/titlePopup.ts @@ -1,4 +1,6 @@ import { ipcMain, shell, dialog, WebContentsView, BrowserWindow } from 'electron' +import { POPUP_KIND } from '../../types/ipc' +import type { PopupTheme, TitlePopupKind } from '../../types/ipc' import { TITLEBAR_HEIGHT } from '../lib/titleBarOverlay' import { cancelModelDownload, @@ -73,13 +75,14 @@ interface TitlePopupMenuItem { kind?: 'separator' } -type TitlePopupKind = 'menu' | 'downloads' | 'instance-picker' | 'global-settings' - -/** Kinds that dim the host body with the shared backdrop view. Single source - * of truth — geometry, blur-dismiss opt-out, and backdrop show/hide all key - * off this so a new kind only has to opt in here. */ +/** Kinds that dim the host body with the shared backdrop view — geometry, + * blur-dismiss opt-out, and backdrop show/hide all key off this. */ function kindUsesBackdrop(kind: TitlePopupKind): boolean { - return kind === 'instance-picker' || kind === 'global-settings' + return ( + kind === POPUP_KIND.instancePicker || + kind === POPUP_KIND.globalSettings || + kind === POPUP_KIND.downloadsFull + ) } /** Single install row pushed to the instance-picker popup. Mirrors the @@ -354,26 +357,15 @@ function buildPickerStorageSlice(): PickerStorageSlice { } } +/** Process-local mirror of the popup config. The `kind` tags are shared via + * `POPUP_KIND`; the snapshot shapes stay main-specific (rich types here vs the + * loosened serialized mirrors in `comfyTitlePopupPreload.ts`). */ type TitlePopupConfig = - | { - kind: 'menu' - items: TitlePopupMenuItem[] - theme: { bg: string; text: string } - } - | { - kind: 'downloads' - theme: { bg: string; text: string } - } - | { - kind: 'instance-picker' - snapshot: InstancePickerSnapshot - theme: { bg: string; text: string } - } - | { - kind: 'global-settings' - snapshot: GlobalSettingsSnapshot - theme: { bg: string; text: string } - } + | { kind: typeof POPUP_KIND.menu; items: TitlePopupMenuItem[]; theme: PopupTheme } + | { kind: typeof POPUP_KIND.downloads; theme: PopupTheme } + | { kind: typeof POPUP_KIND.downloadsFull; theme: PopupTheme } + | { kind: typeof POPUP_KIND.instancePicker; snapshot: InstancePickerSnapshot; theme: PopupTheme } + | { kind: typeof POPUP_KIND.globalSettings; snapshot: GlobalSettingsSnapshot; theme: PopupTheme } /** * One reusable popup `WebContentsView` per parent BrowserWindow. @@ -1194,30 +1186,58 @@ function computePickerBounds(parent: BrowserWindow): PickerBounds { return { x, y, width, height } } -/** Global-settings popup sizing — two-pane tabbed card. Width and - * height are computed once per open from host content bounds (no - * renderer-driven `requestSize` loop) so the popup stays a fixed - * size regardless of which tab is selected and only re-fits when the - * host window itself resizes. Clamps keep the card usable on both - * narrow and ultra-wide windows. */ -const GLOBAL_SETTINGS_POPUP_MIN_WIDTH = 640 -const GLOBAL_SETTINGS_POPUP_MAX_WIDTH = 880 -const GLOBAL_SETTINGS_POPUP_WIDTH_RATIO = 0.65 -const GLOBAL_SETTINGS_POPUP_MIN_HEIGHT = 420 -const GLOBAL_SETTINGS_POPUP_MAX_HEIGHT = 560 -const GLOBAL_SETTINGS_POPUP_HEIGHT_RATIO = 0.7 - -function computeGlobalSettingsBounds(parent: BrowserWindow): { width: number; height: number } { +/** Fluid clamp for a centred-card popup, per axis: min/max px + target ratio of host content. */ +interface CenteredCardClamp { + minWidth: number + maxWidth: number + widthRatio: number + minHeight: number + maxHeight: number + heightRatio: number +} + +type CenteredCardKind = typeof POPUP_KIND.globalSettings | typeof POPUP_KIND.downloadsFull + +/** Per-kind clamps for centred-card popups — fixed-size, only re-fit on host resize. */ +const CENTERED_CARD_CLAMPS: Record = { + [POPUP_KIND.globalSettings]: { + minWidth: 640, + maxWidth: 880, + widthRatio: 0.65, + minHeight: 420, + maxHeight: 560, + heightRatio: 0.7 + }, + [POPUP_KIND.downloadsFull]: { + minWidth: 640, + maxWidth: 900, + widthRatio: 0.7, + minHeight: 440, + maxHeight: 680, + heightRatio: 0.75 + } +} + +/** Renders as a large card centred in the host content area, vs anchored under its trigger. */ +function kindIsCentered(kind: TitlePopupKind): kind is CenteredCardKind { + return kind === POPUP_KIND.globalSettings || kind === POPUP_KIND.downloadsFull +} + +/** Size + centred x/y for a centred-card popup, centred in the band below the title bar. */ +function computeCenteredCardBounds( + kind: CenteredCardKind, + parent: BrowserWindow +): { x: number; y: number; width: number; height: number } { + const clamp = CENTERED_CARD_CLAMPS[kind] const { width: cw, height: ch } = parent.getContentBounds() - const width = Math.min( - GLOBAL_SETTINGS_POPUP_MAX_WIDTH, - Math.max(GLOBAL_SETTINGS_POPUP_MIN_WIDTH, Math.round(cw * GLOBAL_SETTINGS_POPUP_WIDTH_RATIO)) - ) + const width = Math.min(clamp.maxWidth, Math.max(clamp.minWidth, Math.round(cw * clamp.widthRatio))) const height = Math.min( - GLOBAL_SETTINGS_POPUP_MAX_HEIGHT, - Math.max(GLOBAL_SETTINGS_POPUP_MIN_HEIGHT, Math.round(ch * GLOBAL_SETTINGS_POPUP_HEIGHT_RATIO)) + clamp.maxHeight, + Math.max(clamp.minHeight, Math.round(ch * clamp.heightRatio)) ) - return { width, height } + const x = Math.max(0, Math.round((cw - width) / 2)) + const y = Math.max(TITLEBAR_HEIGHT, Math.round(TITLEBAR_HEIGHT + (ch - TITLEBAR_HEIGHT - height) / 2)) + return { x, y, width, height } } /** Right-edge gutter when the popup gets shifted away from its @@ -1282,44 +1302,31 @@ function refitPopupForParent(entry: TitlePopupEntry): void { return } - const contentHeight = parent.getContentBounds().height + if (kindIsCentered(entry.kind)) { + const target = computeCenteredCardBounds(entry.kind, parent) + if ( + target.x === cur.x && + target.y === cur.y && + target.width === cur.width && + target.height === cur.height + ) { + return + } + entry.view.popup.setBounds(target) + return + } let height = cur.height - let width = cur.width if (entry.kind === 'downloads') { const ceiling = Math.min( DOWNLOADS_POPUP_MAX_HEIGHT_PX, - Math.round(contentHeight * DOWNLOADS_POPUP_MAX_HEIGHT_RATIO) + Math.round(parent.getContentBounds().height * DOWNLOADS_POPUP_MAX_HEIGHT_RATIO) ) height = Math.max(1, Math.min(cur.height, ceiling)) - } else if (entry.kind === 'global-settings') { - // Recompute both dimensions from the same clamp the open path uses - // — the popup tracks the host window proportionally as it resizes, - // not the renderer-reported content height (the right pane scrolls, - // not the popup). - ;({ width, height } = computeGlobalSettingsBounds(parent)) - } - - // Re-anchor the centred-card kinds (global-settings) on the new - // window centre so they don't drift off-axis after a resize. Other - // kinds anchor at their trigger button — that's already in title- - // bar-local coords (which don't move on resize) so a fresh - // `clampPopupX` of the existing X is sufficient. - let x: number - let y = cur.y - if (entry.kind === 'global-settings') { - const { width: contentWidth, height: contentHeightForY } = parent.getContentBounds() - x = Math.max(0, Math.round((contentWidth - width) / 2)) - y = Math.max( - TITLEBAR_HEIGHT, - Math.round(TITLEBAR_HEIGHT + (contentHeightForY - TITLEBAR_HEIGHT - height) / 2) - ) - } else { - x = clampPopupX(cur.x, cur.width, parent) } - - if (x === cur.x && y === cur.y && width === cur.width && height === cur.height) return - entry.view.popup.setBounds({ x, y, width, height }) + const x = clampPopupX(cur.x, cur.width, parent) + if (x === cur.x && height === cur.height) return + entry.view.popup.setBounds({ x, y: cur.y, width: cur.width, height }) } type OpenTitlePopupOpts = { @@ -1329,10 +1336,11 @@ type OpenTitlePopupOpts = { theme: { bg: string; text: string } titleBarSender: Electron.WebContents } & ( - | { kind: 'menu'; items: TitlePopupMenuItem[] } - | { kind: 'downloads' } - | { kind: 'instance-picker'; snapshot: InstancePickerSnapshot } - | { kind: 'global-settings'; snapshot: GlobalSettingsSnapshot } + | { kind: typeof POPUP_KIND.menu; items: TitlePopupMenuItem[] } + | { kind: typeof POPUP_KIND.downloads } + | { kind: typeof POPUP_KIND.downloadsFull } + | { kind: typeof POPUP_KIND.instancePicker; snapshot: InstancePickerSnapshot } + | { kind: typeof POPUP_KIND.globalSettings; snapshot: GlobalSettingsSnapshot } ) function openTitlePopup(opts: OpenTitlePopupOpts): void { @@ -1389,43 +1397,20 @@ function openTitlePopup(opts: OpenTitlePopupOpts): void { x = clampPopupX(rawX, width, opts.parent) } else if (opts.kind === 'downloads') { width = DOWNLOADS_POPUP_WIDTH - const contentHeight = opts.parent.getContentBounds().height - // Open at the ceiling (smaller of the fixed pixel cap or 60% of the - // host window's content height, so the popup never overflows tiny - // windows). The renderer immediately measures its natural content - // height and asks for it via `requestSize`, which clamps back into - // this band. The popup stays hidden until the renderer's - // `notifyRendered` ack arrives, so the user never sees this - // provisional size. + /** Provisional ceiling; the renderer measures and `requestSize`s its real height. */ height = Math.min( DOWNLOADS_POPUP_MAX_HEIGHT_PX, - Math.round(contentHeight * DOWNLOADS_POPUP_MAX_HEIGHT_RATIO) + Math.round(opts.parent.getContentBounds().height * DOWNLOADS_POPUP_MAX_HEIGHT_RATIO) ) x = clampPopupX(rawX, width, opts.parent) } else if (opts.kind === 'instance-picker') { - // instance-picker geometry is delegated to `computePickerBounds` - // — single source of truth shared with the parent-resize refit so - // both paths produce consistent bounds. Geometry function owns the - // title-bar inset, so the popup never paints over the title chrome. const bounds = computePickerBounds(opts.parent) width = bounds.width height = bounds.height x = bounds.x y = bounds.y } else { - // global-settings — fluid-clamped centred card. Width + height are - // pinned once from host content bounds; tab switches inside the - // popup never trigger a resize. Both axes centre on the area - // below the title bar. Anchor coords are title-bar-local so `y=0` - // sits at the title-bar top; the centred-y formula recentres - // inside the `contentHeight - TITLEBAR_HEIGHT` band beneath it. - ;({ width, height } = computeGlobalSettingsBounds(opts.parent)) - const { width: contentWidth, height: contentHeight } = opts.parent.getContentBounds() - x = Math.max(0, Math.round((contentWidth - width) / 2)) - y = Math.max( - TITLEBAR_HEIGHT, - Math.round(TITLEBAR_HEIGHT + (contentHeight - TITLEBAR_HEIGHT - height) / 2) - ) + ;({ x, y, width, height } = computeCenteredCardBounds(opts.kind, opts.parent)) } // Update bounds while still hidden — the popup is flipped visible @@ -1439,11 +1424,11 @@ function openTitlePopup(opts: OpenTitlePopupOpts): void { // leaving the downloads popup stuck at the ceiling height. entry.view.popup.setBounds({ x, y, width, height }) - // Downloads popup feeds on a separate channel — push the latest - // snapshot now so the first paint shows current state instead of - // the empty-state placeholder. Subsequent updates arrive via the - // tray-state-changed broadcast. - if (opts.kind === 'downloads' && entry.view.rendererReady) { + /** Seed first paint with current state; live updates arrive via tray-state-changed. */ + if ( + (opts.kind === POPUP_KIND.downloads || opts.kind === POPUP_KIND.downloadsFull) && + entry.view.rendererReady + ) { notifyTitlePopupDownloads(entry.view.popup) } @@ -1453,21 +1438,18 @@ function openTitlePopup(opts: OpenTitlePopupOpts): void { // Vue is still processing the config update. entry.view.cancelPendingShow() let config: TitlePopupConfig - if (opts.kind === 'menu') { - config = { kind: 'menu', items: opts.items, theme: opts.theme } - } else if (opts.kind === 'downloads') { - config = { kind: 'downloads', theme: opts.theme } - } else if (opts.kind === 'instance-picker') { - config = { kind: 'instance-picker', snapshot: opts.snapshot, theme: opts.theme } - // Seed the broadcast-dedupe cache with the snapshot we're about to - // ship as the initial config. Without this, a subsequent live - // broadcast that happens to equal a *previous* session's last - // broadcast (but differs from the snapshot the renderer is currently - // displaying) would be silently skipped by the dedupe check in - // `broadcastInstancePickerUpdate` / `broadcastGlobalSettingsUpdate`. + if (opts.kind === POPUP_KIND.menu) { + config = { kind: POPUP_KIND.menu, items: opts.items, theme: opts.theme } + } else if (opts.kind === POPUP_KIND.downloads) { + config = { kind: POPUP_KIND.downloads, theme: opts.theme } + } else if (opts.kind === POPUP_KIND.downloadsFull) { + config = { kind: POPUP_KIND.downloadsFull, theme: opts.theme } + } else if (opts.kind === POPUP_KIND.instancePicker) { + config = { kind: POPUP_KIND.instancePicker, snapshot: opts.snapshot, theme: opts.theme } + /** Seed the dedupe cache so a later broadcast equal to a prior session's isn't skipped. */ entry.lastPickerBroadcastJson = JSON.stringify(opts.snapshot) } else { - config = { kind: 'global-settings', snapshot: opts.snapshot, theme: opts.theme } + config = { kind: POPUP_KIND.globalSettings, snapshot: opts.snapshot, theme: opts.theme } entry.lastGlobalSettingsBroadcastJson = JSON.stringify(opts.snapshot) } const configJson = JSON.stringify(config) @@ -1662,6 +1644,24 @@ function openGlobalSettingsForHost( })() } +/** Open the large centred "View All Downloads" popup. Reuses the tray popup's + * live download feed, so no snapshot is built here. */ +function openDownloadsFullForHost( + parentEntry: ComfyWindowEntry, + parentEntryId: number, + titleBarSender: Electron.WebContents +): void { + if (parentEntry.window.isDestroyed()) return + openTitlePopup({ + parent: parentEntry.window, + parentEntryId, + kind: POPUP_KIND.downloadsFull, + anchor: { x: 0, y: TITLEBAR_HEIGHT }, + theme: parentEntry.lastTheme, + titleBarSender + }) +} + /** * Open the instance-picker popup parented to the given host window. * Shared by the title-bar centre-pill click and the panel-side @@ -2134,7 +2134,7 @@ export function registerTitlePopupIpc(bindings: TitlePopupHostBindings): void { entry.lastConfigJson = JSON.stringify(flushed) entry.view.popup.webContents.send('comfy-titlepopup:set-config', flushed) entry.pendingConfig = null - if (flushed.kind === 'downloads') { + if (flushed.kind === POPUP_KIND.downloads || flushed.kind === POPUP_KIND.downloadsFull) { notifyTitlePopupDownloads(entry.view.popup) } } @@ -2283,19 +2283,18 @@ export function registerTitlePopupIpc(bindings: TitlePopupHostBindings): void { }) }) - // Popup → host deep-link to the standalone "View All Downloads" modal. - // Flips the host into the `'downloads-v2'` overlay panel mode, which - // `layoutViews` recognises as a transparent panel-forward state. The - // renderer mounts `DownloadsModal` by watching `activePanel === 'downloads-v2'`. - // No deep-link IPC needed — the mode swap IS the open signal — and dismiss - // routes through `closeCurrentPanel()` which returns the body to `'comfy'`. + /** Tray popup's "View All Downloads" → reopen the same WebContentsView as the + * large centred `downloads-full` popup (replaces the outgoing tray kind). */ ipcMain.on('comfy-titlepopup:open-downloads-modal', (event) => { const popupEntry = titlePopupsByWebContents.get(event.sender.id) if (!popupEntry) return const parentEntry = comfyWindows.get(popupEntry.parentEntryId) if (!parentEntry) return - hideTitlePopup(popupEntry, { releaseFocusToParent: false }) - bindings.setActivePanel(popupEntry.parentEntryId, 'downloads-v2') + openDownloadsFullForHost( + parentEntry, + popupEntry.parentEntryId, + parentEntry.titleBarView.webContents + ) }) // Title-bar downloads-tray click. Opens the title-bar dropdown popup diff --git a/src/preload/comfyTitlePopupPreload.ts b/src/preload/comfyTitlePopupPreload.ts index 4d2328617..331eb5f59 100644 --- a/src/preload/comfyTitlePopupPreload.ts +++ b/src/preload/comfyTitlePopupPreload.ts @@ -1,7 +1,7 @@ import { contextBridge, ipcRenderer } from 'electron' import type { IpcRendererEvent } from 'electron' -import { PICKER_SETTINGS_CHANNELS as CH } from '../types/ipc' -import type { TerminalRestore } from '../types/ipc' +import { PICKER_SETTINGS_CHANNELS as CH, POPUP_KIND } from '../types/ipc' +import type { PopupTheme, TerminalRestore } from '../types/ipc' /** Bridge for the title-bar dropdown popup (waffle menu, downloads tray, * instance-picker, global-settings), which share one reused child @@ -113,25 +113,11 @@ export interface PopupGlobalSettingsSnapshot { } export type TitlePopupConfig = - | { - kind: 'menu' - items: TitlePopupMenuItem[] - theme: { bg: string; text: string } - } - | { - kind: 'downloads' - theme: { bg: string; text: string } - } - | { - kind: 'instance-picker' - snapshot: PopupInstancePickerSnapshot - theme: { bg: string; text: string } - } - | { - kind: 'global-settings' - snapshot: PopupGlobalSettingsSnapshot - theme: { bg: string; text: string } - } + | { kind: typeof POPUP_KIND.menu; items: TitlePopupMenuItem[]; theme: PopupTheme } + | { kind: typeof POPUP_KIND.downloads; theme: PopupTheme } + | { kind: typeof POPUP_KIND.downloadsFull; theme: PopupTheme } + | { kind: typeof POPUP_KIND.instancePicker; snapshot: PopupInstancePickerSnapshot; theme: PopupTheme } + | { kind: typeof POPUP_KIND.globalSettings; snapshot: PopupGlobalSettingsSnapshot; theme: PopupTheme } /** Mirrors `DownloadProgress` in `src/main/lib/comfyDownloadManager.ts`. */ export interface PopupDownloadEntry { @@ -402,17 +388,18 @@ function isPopupConfig(value: unknown): value is TitlePopupConfig { snapshot?: unknown } if ( - v.kind !== 'menu' - && v.kind !== 'downloads' - && v.kind !== 'instance-picker' - && v.kind !== 'global-settings' + v.kind !== POPUP_KIND.menu + && v.kind !== POPUP_KIND.downloads + && v.kind !== POPUP_KIND.downloadsFull + && v.kind !== POPUP_KIND.instancePicker + && v.kind !== POPUP_KIND.globalSettings ) return false if (!v.theme || typeof v.theme !== 'object') return false const theme = v.theme as { bg?: unknown; text?: unknown } if (typeof theme.bg !== 'string' || typeof theme.text !== 'string') return false - if (v.kind === 'menu' && !Array.isArray(v.items)) return false - if (v.kind === 'instance-picker' && !isInstancePickerSnapshot(v.snapshot)) return false - if (v.kind === 'global-settings' && !isGlobalSettingsSnapshot(v.snapshot)) return false + if (v.kind === POPUP_KIND.menu && !Array.isArray(v.items)) return false + if (v.kind === POPUP_KIND.instancePicker && !isInstancePickerSnapshot(v.snapshot)) return false + if (v.kind === POPUP_KIND.globalSettings && !isGlobalSettingsSnapshot(v.snapshot)) return false return true } @@ -592,10 +579,11 @@ const bridge: ComfyTitlePopupBridge = { if (!data || typeof data !== 'object') return const kind = (data as { kind?: unknown }).kind if ( - kind !== 'menu' - && kind !== 'downloads' - && kind !== 'instance-picker' - && kind !== 'global-settings' + kind !== POPUP_KIND.menu + && kind !== POPUP_KIND.downloads + && kind !== POPUP_KIND.downloadsFull + && kind !== POPUP_KIND.instancePicker + && kind !== POPUP_KIND.globalSettings ) return cb({ kind }) } diff --git a/src/renderer/src/comfyTitlePopup/DownloadsFullView.vue b/src/renderer/src/comfyTitlePopup/DownloadsFullView.vue new file mode 100644 index 000000000..836e84df8 --- /dev/null +++ b/src/renderer/src/comfyTitlePopup/DownloadsFullView.vue @@ -0,0 +1,749 @@ + + + + + diff --git a/src/renderer/src/comfyTitlePopup/DownloadsView.test.ts b/src/renderer/src/comfyTitlePopup/DownloadsView.test.ts index 467bf54b7..ef07c3576 100644 --- a/src/renderer/src/comfyTitlePopup/DownloadsView.test.ts +++ b/src/renderer/src/comfyTitlePopup/DownloadsView.test.ts @@ -262,14 +262,14 @@ describe('comfyTitlePopup/DownloadsView', () => { ]) }) - it('routes the footer link to the standalone DownloadsModal (not the Settings tab)', async () => { + it('routes the footer link to the full downloads popup (not the Settings tab)', async () => { const { default: DownloadsView } = await import('./DownloadsView.vue') const wrapper = mount(DownloadsView, { props: { state: EMPTY_STATE } }) await flushPromises() expect(wrapper.find('.downloads-link').text()).toBe('View All Downloads') await wrapper.find('.downloads-link').trigger('click') expect(bridgeState.openDownloadsModalCalls).toBe(1) - // The modal, not `openSettingsTab('downloads')`, is now the destination. + // The full popup, not `openSettingsTab('downloads')`, is now the destination. expect(bridgeState.openSettingsTabCalls).toEqual([]) }) }) diff --git a/src/renderer/src/comfyTitlePopup/TitlePopupApp.vue b/src/renderer/src/comfyTitlePopup/TitlePopupApp.vue index 0ba6281d5..4d459cf51 100644 --- a/src/renderer/src/comfyTitlePopup/TitlePopupApp.vue +++ b/src/renderer/src/comfyTitlePopup/TitlePopupApp.vue @@ -2,6 +2,7 @@ import { computed, nextTick, onMounted, onUnmounted, ref, watch } from 'vue' import MenuView from './MenuView.vue' import DownloadsView from './DownloadsView.vue' +import DownloadsFullView from './DownloadsFullView.vue' import InstancePickerView from './InstancePickerView.vue' import GlobalSettingsView from './GlobalSettingsView.vue' import ModalDialog from '../components/ModalDialog.vue' @@ -11,6 +12,7 @@ import { dismissPickerModals } from './dismissPickerModals' import { popupLocaleSource } from './pickerSettingsApiShim' import { useAppLocale } from '../lib/useAppLocale' import type { DetailSection, SnapshotListData } from '../types/ipc' +import type { PopupDownloadsState as DownloadsState } from '../../../preload/comfyTitlePopupPreload' import { isColorLight } from '../lib/colorScheme' // Title-bar dropdown popup shell. Hosts every title-bar dropdown in one @@ -25,27 +27,6 @@ interface MenuItem { kind?: 'separator' } -interface DownloadEntry { - url: string - filename: string - directory?: string - savePath?: string - progress: number - receivedBytes?: number - totalBytes?: number - speedBytesPerSec?: number - etaSeconds?: number - status: 'pending' | 'downloading' | 'paused' | 'completed' | 'error' | 'cancelled' - error?: string - createdAt?: number - isImage?: boolean -} - -interface DownloadsState { - active: DownloadEntry[] - recent: DownloadEntry[] -} - interface PickerInstall { id: string name: string @@ -135,6 +116,10 @@ type PopupConfig = kind: 'downloads' theme: { bg: string; text: string } } + | { + kind: 'downloads-full' + theme: { bg: string; text: string } + } | { kind: 'instance-picker' snapshot: PickerSnapshot @@ -158,9 +143,7 @@ interface Bridge { /** Resize the popup view to fit the given natural content height (CSS px). * Menu kind is sized deterministically main-side instead. */ requestSize(height: number): void - onWillShow( - cb: (info: { kind: 'menu' | 'downloads' | 'instance-picker' | 'global-settings' }) => void - ): () => void + onWillShow(cb: (info: { kind: PopupConfig['kind'] }) => void): () => void /** Cancel any open useModal / useDialogs entry so a half-open confirm * doesn't survive a kind-switch as orphaned Vue state. */ onDismissModals(cb: () => void): () => void @@ -168,7 +151,7 @@ interface Bridge { const bridge = (window as unknown as { __comfyTitlePopup?: Bridge }).__comfyTitlePopup -const kind = ref<'menu' | 'downloads' | 'instance-picker' | 'global-settings'>('menu') +const kind = ref('menu') const items = ref([]) const themeBg = ref('#262729') const themeText = ref('#dddddd') @@ -392,12 +375,14 @@ onUnmounted(() => { 'is-light': isLight, 'is-menu': kind === 'menu', 'is-picker': kind === 'instance-picker', - 'is-global-settings': kind === 'global-settings' + 'is-global-settings': kind === 'global-settings', + 'is-downloads-full': kind === 'downloads-full' }" :style="{ background: themeBg, color: themeText }" > + @@ -437,9 +422,10 @@ onUnmounted(() => { font-size: 13px; } -/* Instance picker + Global Settings share the in-app modal-card chrome. */ +/* Centred-card popups share the in-app modal-card chrome. */ .popup.is-picker, -.popup.is-global-settings { +.popup.is-global-settings, +.popup.is-downloads-full { background: var(--modal-surface-bg) !important; border: 1px solid var(--modal-surface-border); border-radius: 14px; diff --git a/src/renderer/src/components/DownloadsModal.vue b/src/renderer/src/components/DownloadsModal.vue deleted file mode 100644 index 5cdb88310..000000000 --- a/src/renderer/src/components/DownloadsModal.vue +++ /dev/null @@ -1,767 +0,0 @@ - - - - - - - - diff --git a/src/renderer/src/panel/PanelApp.test.ts b/src/renderer/src/panel/PanelApp.test.ts index 378e6a70b..3eeefeffd 100644 --- a/src/renderer/src/panel/PanelApp.test.ts +++ b/src/renderer/src/panel/PanelApp.test.ts @@ -113,14 +113,6 @@ vi.mock('../views/FirstUseTakeover.vue', () => ({ methods: { open: vi.fn() } } })) -vi.mock('../components/DownloadsModal.vue', () => ({ - default: { - name: 'DownloadsModal', - props: ['open'], - emits: ['close'], - template: '
' - } -})) vi.mock('../views/MigrateConfirmTakeover.vue', () => ({ default: { name: 'MigrateConfirmTakeover', diff --git a/src/renderer/src/panel/PanelApp.vue b/src/renderer/src/panel/PanelApp.vue index ebf7ea5fc..a5e394030 100644 --- a/src/renderer/src/panel/PanelApp.vue +++ b/src/renderer/src/panel/PanelApp.vue @@ -4,7 +4,6 @@ import { useI18n } from 'vue-i18n' import ProgressModal from '../views/ProgressModal.vue' import ModalDialog from '../components/ModalDialog.vue' import DialogHost from '../components/DialogHost.vue' -import DownloadsModal from '../components/DownloadsModal.vue' import FeedbackModal from '../components/FeedbackModal.vue' import ComfyLifecycleView from './ComfyLifecycleView.vue' import ChooserView from '../views/ChooserView.vue' @@ -311,23 +310,11 @@ function handleProgressSuccessChoice(actionId: string, targetInstallationId: str } } -// `'downloads-v2'` brings the panel forward in an overlay mode; the renderer -// mounts `DownloadsModal` and dismiss routes back through `closeCurrentPanel` -// so the body returns to comfy/lifecycle without leaving stale state. -function closeDownloadsV2(): void { - window.api.closeCurrentPanel() -} - -// Toggles transparency rules in the non-scoped diff --git a/src/renderer/src/lib/formatting.ts b/src/renderer/src/lib/formatting.ts index 3a48e517b..60aef3747 100644 --- a/src/renderer/src/lib/formatting.ts +++ b/src/renderer/src/lib/formatting.ts @@ -4,3 +4,11 @@ export function formatBytes(bytes: number): string { if (bytes < 1073741824) return `${(bytes / 1048576).toFixed(1)} MB` return `${(bytes / 1073741824).toFixed(2)} GB` } + +/** Rounded sizes for picker meta lines (~16 GB, not ~16.05 GB). */ +export function formatBytesCoarse(bytes: number): string { + if (!bytes || bytes <= 0) return '' + if (bytes < 1048576) return `${Math.round(bytes / 1024)} KB` + if (bytes < 1073741824) return `${Math.round(bytes / 1048576)} MB` + return `${Math.round(bytes / 1073741824)} GB` +} diff --git a/src/renderer/src/lib/installHelpers.test.ts b/src/renderer/src/lib/installHelpers.test.ts new file mode 100644 index 000000000..b08be1671 --- /dev/null +++ b/src/renderer/src/lib/installHelpers.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect } from 'vitest' +import { + templateDiskRequiredBytes, + isTemplateDiskBlocked, + minTemplateModelBytes +} from './installHelpers' + +const GB = 1024 * 1024 * 1024 + +describe('templateDiskRequiredBytes', () => { + it('is zero for a model-free template (nothing to block on)', () => { + expect(templateDiskRequiredBytes(0)).toBe(0) + expect(templateDiskRequiredBytes(-1)).toBe(0) + }) + + it('adds headroom over the raw model size', () => { + const required = templateDiskRequiredBytes(2 * GB) + expect(required).toBeGreaterThan(2 * GB) + expect(required).toBe(Math.ceil(2 * GB * 1.1)) + }) +}) + +describe('isTemplateDiskBlocked', () => { + it('does not block when disk space is unknown yet', () => { + expect(isTemplateDiskBlocked(null, 2 * GB)).toBe(false) + }) + + it('does not block a model-free template', () => { + expect(isTemplateDiskBlocked({ free: 0, total: 100 * GB }, 0)).toBe(false) + }) + + it('blocks when free space is below model size + headroom', () => { + // 2 GB models → ~2.2 GB required; 2.1 GB free is short. + expect(isTemplateDiskBlocked({ free: 2.1 * GB, total: 100 * GB }, 2 * GB)).toBe(true) + }) + + it('allows when free space covers model size + headroom', () => { + expect(isTemplateDiskBlocked({ free: 3 * GB, total: 100 * GB }, 2 * GB)).toBe(false) + }) +}) + +describe('minTemplateModelBytes', () => { + it('is zero when no template carries models', () => { + expect(minTemplateModelBytes([])).toBe(0) + expect(minTemplateModelBytes([0, 0])).toBe(0) + }) + + it('returns the smallest model-bearing footprint, ignoring zero-model ones', () => { + expect(minTemplateModelBytes([0, 5 * GB, 2 * GB, 8 * GB])).toBe(2 * GB) + }) + + it('drives the skip-the-picker gate: cheapest still does not fit', () => { + const cheapest = minTemplateModelBytes([3 * GB, 6 * GB]) + expect(isTemplateDiskBlocked({ free: 1 * GB, total: 100 * GB }, cheapest)).toBe(true) + }) + + it('drives the skip-the-picker gate: cheapest fits → picker stays', () => { + const cheapest = minTemplateModelBytes([3 * GB, 6 * GB]) + expect(isTemplateDiskBlocked({ free: 10 * GB, total: 100 * GB }, cheapest)).toBe(false) + }) +}) diff --git a/src/renderer/src/lib/installHelpers.ts b/src/renderer/src/lib/installHelpers.ts index 33558078c..ec2a47104 100644 --- a/src/renderer/src/lib/installHelpers.ts +++ b/src/renderer/src/lib/installHelpers.ts @@ -159,6 +159,83 @@ export async function checkDiskSpaceOrWarn(opts: { return true } +/** + * Hard-block (not a warn) when the volume can't hold the template's required + * models. Unlike `checkDiskSpaceOrWarn`, there's no "continue anyway" — running + * out of disk would leave a half-downloaded model set and a confusing error + * row, so the user must free space or deselect the template first. + * + * Returns `true` when there's room (or nothing to check), `false` when the + * block alert was shown. `estimatedModelBytes` is the template's coarse model + * size; a small headroom multiplier covers the unzip/temp overhead and the + * estimate's imprecision. + */ +const TEMPLATE_DISK_HEADROOM = 1.1 + +/** Bytes the volume must have free to safely fit `estimatedModelBytes` of + * template models (model size + a headroom for temp/unzip + estimate slop). + * Pure — the threshold math, isolated so it's unit-testable. */ +export function templateDiskRequiredBytes(estimatedModelBytes: number): number { + if (estimatedModelBytes <= 0) return 0 + return Math.ceil(estimatedModelBytes * TEMPLATE_DISK_HEADROOM) +} + +/** + * Single source of truth for "is the volume too small for this template's + * models?" — used by the picker (to show the alert + disable Install) and by the + * wizard (the same decision). Pure: `false` when there's nothing to check + * (no models, disk space not yet known, or it fits). Keeps the three callers + * from each re-deriving the rule and drifting. + */ +export function isTemplateDiskBlocked( + diskSpace: DiskSpaceInfo | null, + estimatedModelBytes: number, +): boolean { + const required = templateDiskRequiredBytes(estimatedModelBytes) + if (required === 0 || !diskSpace) return false + return diskSpace.free < required +} + +/** + * Smallest model footprint among the model-bearing templates, or 0 when none + * carry models. Drives the "skip the picker entirely when even the cheapest + * template won't fit" gate — there's no point offering a showcase nothing on it + * can install. Zero-model templates are ignored (they need no disk). + */ +export function minTemplateModelBytes(modelByteSizes: number[]): number { + const withModels = modelByteSizes.filter((b) => b > 0) + return withModels.length ? Math.min(...withModels) : 0 +} + +export async function checkTemplateDiskOrBlock(opts: { + path: string + estimatedModelBytes: number + flow: string + alert: (opts: { title: string; message: string }) => Promise + t: (key: string, params?: Record) => string +}): Promise { + if (templateDiskRequiredBytes(opts.estimatedModelBytes) === 0) return true + + let diskSpace: DiskSpaceInfo + try { + diskSpace = await window.api.getDiskSpace(opts.path) + } catch { + // Can't probe — don't block on a failed read; the in-task guard is the net. + return true + } + if (!isTemplateDiskBlocked(diskSpace, opts.estimatedModelBytes)) return true + + trackGuardrailBlocked('template_models_disk', opts.flow, 'save') + await opts.alert({ + title: opts.t('diskSpace.templateBlockTitle'), + message: opts.t('diskSpace.templateBlockMessage', { + required: formatBytes(templateDiskRequiredBytes(opts.estimatedModelBytes)), + free: formatBytes(diskSpace.free) + }) + }) + return false +} + export function createDiskSpaceChecker() { const diskSpace = ref(null) const diskSpaceLoading = ref(false) diff --git a/src/renderer/src/lib/progressViewModel.ts b/src/renderer/src/lib/progressViewModel.ts index a8beeace3..08a695198 100644 --- a/src/renderer/src/lib/progressViewModel.ts +++ b/src/renderer/src/lib/progressViewModel.ts @@ -15,4 +15,7 @@ export interface ProgressStepVM { /** Determinate fill for the active row's mini-bar, or null for a spinner * (unbounded phase). */ subPercent: number | null + /** Non-fatal failure on the active row — renders the detail line in an error + * style (red/bold + X) without failing the op. Only ever true for `active`. */ + isError: boolean } diff --git a/src/renderer/src/stores/progressStore.test.ts b/src/renderer/src/stores/progressStore.test.ts index 1edc2daa2..56af3e61a 100644 --- a/src/renderer/src/stores/progressStore.test.ts +++ b/src/renderer/src/stores/progressStore.test.ts @@ -32,6 +32,7 @@ vi.stubGlobal('window', { onInstallationsVersionsUpdated: vi.fn(), onInstallProgress: vi.fn(() => vi.fn()), onComfyOutput: vi.fn(() => vi.fn()), + logsSnapshot: vi.fn().mockResolvedValue(''), cancelOperation: vi.fn(), stopComfyUI: vi.fn(), getRunningInstances: vi.fn().mockResolvedValue([]), @@ -74,7 +75,7 @@ describe('useProgressStore', () => { }) it('returns flat status and percent when no steps defined', () => { - const apiCall = () => new Promise(() => {}) // never resolves + const apiCall = () => new Promise(() => { }) // never resolves store.startOperation({ installationId: 'inst-1', title: 'Install', @@ -90,7 +91,7 @@ describe('useProgressStore', () => { }) it('returns step-based status when steps and activePhase are set', () => { - const apiCall = () => new Promise(() => {}) + const apiCall = () => new Promise(() => { }) store.startOperation({ installationId: 'inst-1', title: 'Install', @@ -111,7 +112,7 @@ describe('useProgressStore', () => { }) it('falls back to step label, not the raw phase id, when lastStatus has no entry', () => { - const apiCall = () => new Promise(() => {}) + const apiCall = () => new Promise(() => { }) store.startOperation({ installationId: 'inst-1', title: 'Install', @@ -131,7 +132,7 @@ describe('useProgressStore', () => { }) it('falls back to title when flatStatus is empty', () => { - const apiCall = () => new Promise(() => {}) + const apiCall = () => new Promise(() => { }) store.startOperation({ installationId: 'inst-1', title: 'Install', @@ -157,14 +158,14 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-1', title: 'Delete', - apiCall: () => new Promise(() => {}) + apiCall: () => new Promise(() => { }) }) expect(sessionStore.errorInstances.has('inst-1')).toBe(false) }) it('creates an operation and sets up session', () => { - const apiCall = () => new Promise(() => {}) + const apiCall = () => new Promise(() => { }) store.startOperation({ installationId: 'inst-1', title: 'Install ComfyUI', @@ -179,7 +180,7 @@ describe('useProgressStore', () => { }) it('subscribes to progress and output IPC', () => { - const apiCall = () => new Promise(() => {}) + const apiCall = () => new Promise(() => { }) store.startOperation({ installationId: 'inst-1', title: 'Install', @@ -282,13 +283,13 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-1', title: 'First', - apiCall: () => new Promise(() => {}) + apiCall: () => new Promise(() => { }) }) store.startOperation({ installationId: 'inst-1', title: 'Second', - apiCall: () => new Promise(() => {}) + apiCall: () => new Promise(() => { }) }) expect(store.operations.get('inst-1')?.title).toBe('Second') @@ -336,7 +337,7 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-1', title: 'Install', - apiCall: () => new Promise(() => {}) + apiCall: () => new Promise(() => { }) }) store.cancelOperation('inst-1') @@ -409,7 +410,7 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-1', title: 'Install', - apiCall: () => new Promise(() => {}) + apiCall: () => new Promise(() => { }) }) store.cleanupOperation('inst-1') @@ -428,7 +429,7 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-1', title: 'Installing — DevFixture', - apiCall: () => new Promise(() => {}), + apiCall: () => new Promise(() => { }), chainSpan: 'install' }) expect(store.operations.get('inst-1')?.chainSpan).toBe('install') @@ -438,7 +439,7 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-2', title: 'Launching — DevFixture', - apiCall: () => new Promise(() => {}), + apiCall: () => new Promise(() => { }), chainSpan: 'launch' }) expect(store.operations.get('inst-2')?.chainSpan).toBe('launch') @@ -448,12 +449,37 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-3', title: 'Standalone op', - apiCall: () => new Promise(() => {}) + apiCall: () => new Promise(() => { }) }) // null (not undefined) so the Operation literal stays well-typed. expect(store.operations.get('inst-3')?.chainSpan).toBeNull() }) + it('seeds the launch leg terminalOutput from logsSnapshot (log continuity)', async () => { + vi.mocked(window.api.logsSnapshot).mockResolvedValueOnce('install-leg lines\n') + store.startOperation({ + installationId: 'inst-snap', + title: 'Launching', + apiCall: () => new Promise(() => { }), + chainSpan: 'launch' + }) + expect(window.api.logsSnapshot).toHaveBeenCalledWith('inst-snap') + await Promise.resolve() // let the snapshot promise + its .then settle + await Promise.resolve() + expect(store.operations.get('inst-snap')?.terminalOutput).toBe('install-leg lines\n') + }) + + it('does not seed from logsSnapshot for a non-launch op', () => { + vi.mocked(window.api.logsSnapshot).mockClear() + store.startOperation({ + installationId: 'inst-nosnap', + title: 'Install', + apiCall: () => new Promise(() => { }), + chainSpan: 'install' + }) + expect(window.api.logsSnapshot).not.toHaveBeenCalled() + }) + it('caps the install leg of a chain at 0–70% of the continuous bar', () => { // globalProgressFor now folds the install/launch split in directly: a // chainSpan='install' op fills 0–70 (reserving 70–100 for the launch @@ -461,7 +487,7 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-4', title: 'Installing', - apiCall: () => new Promise(() => {}), + apiCall: () => new Promise(() => { }), chainSpan: 'install' }) const op = store.operations.get('inst-4')! @@ -485,7 +511,7 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-5', title: 'Installing', - apiCall: () => new Promise(() => {}), + apiCall: () => new Promise(() => { }), chainSpan: 'install' }) const op = store.operations.get('inst-5')! @@ -500,7 +526,7 @@ describe('useProgressStore', () => { store.startOperation({ installationId: 'inst-6', title: 'Updating', - apiCall: () => new Promise(() => {}) + apiCall: () => new Promise(() => { }) }) const op = store.operations.get('inst-6')! op.finished = true diff --git a/src/renderer/src/stores/progressStore.ts b/src/renderer/src/stores/progressStore.ts index cae183ac8..c44b358dd 100644 --- a/src/renderer/src/stores/progressStore.ts +++ b/src/renderer/src/stores/progressStore.ts @@ -47,6 +47,9 @@ export interface Operation { activePhase: string | null activePercent: number lastStatus: Record + /** Phases the producer flagged as non-fatally failed — drives the active + * row's error styling without failing the op. Keyed by phase id. */ + phaseErrors: Record flatStatus: string flatPercent: number terminalOutput: string @@ -215,6 +218,7 @@ export const useProgressStore = defineStore('progress', () => { activePhase: null, activePercent: -1, lastStatus: {}, + phaseErrors: {}, flatStatus: t('progress.starting'), flatPercent: -1, terminalOutput: '', @@ -234,6 +238,20 @@ export const useProgressStore = defineStore('progress', () => { operations.set(installationId, op) const rop = operations.get(installationId)! + // Log continuity across a chain: the launch leg's `terminalOutput` starts + // empty, but a background template-model download may have logged lines + // during the install leg. Seed from the durable ring buffer so "View logs" + // shows the full history. Async + guarded against a newer op replacing this + // one mid-fetch. Prepended so any lines that streamed in before the snapshot + // resolved aren't clobbered. + if (chainSpan === 'launch' && typeof window.api.logsSnapshot === 'function') { + void window.api.logsSnapshot(installationId).then((snapshot) => { + if (!snapshot) return + if (operations.get(installationId) !== rop) return + rop.terminalOutput = snapshot + rop.terminalOutput + }).catch(() => {}) + } + rop.unsubProgress = window.api.onInstallProgress((data: ProgressData) => { if (data.installationId !== installationId) return @@ -254,6 +272,7 @@ export const useProgressStore = defineStore('progress', () => { if (stepIndex === -1) return rop.activePhase = data.phase rop.lastStatus[data.phase] = data.status || data.phase + rop.phaseErrors[data.phase] = data.error === true rop.activePercent = data.percent ?? -1 return } diff --git a/src/renderer/src/views/InstallWizardModal.vue b/src/renderer/src/views/InstallWizardModal.vue index 150d4a34f..960696b7b 100644 --- a/src/renderer/src/views/InstallWizardModal.vue +++ b/src/renderer/src/views/InstallWizardModal.vue @@ -1,7 +1,7 @@ + + + + diff --git a/src/renderer/src/composables/useTemplateTabs.test.ts b/src/renderer/src/composables/useTemplateTabs.test.ts new file mode 100644 index 000000000..a2954548f --- /dev/null +++ b/src/renderer/src/composables/useTemplateTabs.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from 'vitest' +import { ref } from 'vue' + +import type { FieldOption } from '../types/ipc' +import { useTemplateTabs } from './useTemplateTabs' + +const t = (key: string) => key + +const NONE: FieldOption = { value: 'none', label: 'None' } +const opt = (value: string, modality: string, recommended = false): FieldOption => ({ + value, + label: value, + recommended, + data: { modality }, +}) + +const IMG_A = opt('img_a', 'image', true) +const IMG_B = opt('img_b', 'image') +const VID = opt('vid', 'video', true) +const AUD = opt('aud', 'audio') +const THREED = opt('td', '3d') + +function setup(options: FieldOption[], selectedValue: string | null = null) { + return useTemplateTabs(ref(options), 'none', ref(selectedValue), t) +} + +describe('useTemplateTabs', () => { + it('builds one tab per populated modality, in Image → Video → 3D → Audio order', () => { + const { tabs } = setup([NONE, AUD, THREED, VID, IMG_A]) + expect(tabs.value.map((tab) => tab.modality)).toEqual(['image', 'video', '3d', 'audio']) + }) + + it('counts templates per modality', () => { + const { tabs } = setup([NONE, IMG_A, IMG_B, VID]) + expect(tabs.value.find((tab) => tab.modality === 'image')!.count).toBe(2) + expect(tabs.value.find((tab) => tab.modality === 'video')!.count).toBe(1) + }) + + it('defaults the active tab to the selected template\'s modality', () => { + const { activeModality } = setup([NONE, IMG_A, IMG_B, VID], VID.value) + expect(activeModality.value).toBe('video') + }) + + it('defaults to the first populated tab when nothing is selected', () => { + const { activeModality } = setup([NONE, VID, AUD]) + expect(activeModality.value).toBe('video') + }) + + it('exposes only the active tab\'s cards', () => { + const tabs = setup([NONE, IMG_A, IMG_B, VID], IMG_A.value) + expect(tabs.visibleCards.value.map((c) => c.value)).toEqual(['img_a', 'img_b']) + tabs.selectTab('video') + expect(tabs.visibleCards.value.map((c) => c.value)).toEqual(['vid']) + }) + + it('excludes the none sentinel from tabs and cards', () => { + const { tabs, visibleCards } = setup([NONE, IMG_A]) + expect(tabs.value).toHaveLength(1) + expect(visibleCards.value.every((c) => c.value !== 'none')).toBe(true) + }) + + it('returns no tabs when only the none sentinel is present', () => { + const { tabs, activeModality, visibleCards } = setup([NONE]) + expect(tabs.value).toHaveLength(0) + expect(activeModality.value).toBeNull() + expect(visibleCards.value).toHaveLength(0) + }) +}) diff --git a/src/renderer/src/composables/useTemplateTabs.ts b/src/renderer/src/composables/useTemplateTabs.ts new file mode 100644 index 000000000..865815826 --- /dev/null +++ b/src/renderer/src/composables/useTemplateTabs.ts @@ -0,0 +1,102 @@ +import { computed, ref, watch, type Component, type Ref } from 'vue' +import { Image as ImageIcon, Video, AudioLines, Box } from 'lucide-vue-next' +import type { FieldOption } from '../types/ipc' + +/** Modality tab order + its glyph. Mirrors the main-process curated manifest's + * `TEMPLATE_MODALITY_ORDER`; kept here so the renderer owns its own UI ordering + * without reaching across the process boundary. */ +const MODALITY_ORDER = ['image', 'video', '3d', 'audio'] as const +type Modality = (typeof MODALITY_ORDER)[number] + +const MODALITY_GLYPH: Record = { + image: ImageIcon, + video: Video, + audio: AudioLines, + '3d': Box, +} + +export interface TemplateTab { + modality: Modality + label: string + glyph: Component + count: number +} + +function modalityOf(option: FieldOption): Modality | null { + const value = option.data?.modality + return (MODALITY_ORDER as readonly string[]).includes(value as string) ? (value as Modality) : null +} + +/** + * Groups the picker's template options into per-modality tabs and tracks the + * active one. The "None" sentinel is excluded; only modalities with ≥1 template + * get a tab. The active tab follows the selected template (so re-entering the + * step lands on the user's pick) and defaults to the first populated tab. + */ +export function useTemplateTabs( + options: Ref, + noneValue: Ref | string, + selectedValue: Ref, + translate: (key: string) => string +) { + const none = computed(() => (typeof noneValue === 'string' ? noneValue : noneValue.value)) + + const templateCards = computed(() => options.value.filter((o) => o.value !== none.value)) + + const cardsByModality = computed(() => { + const groups = new Map() + for (const card of templateCards.value) { + const modality = modalityOf(card) + if (!modality) continue + const bucket = groups.get(modality) + if (bucket) bucket.push(card) + else groups.set(modality, [card]) + } + return groups + }) + + const tabs = computed(() => + MODALITY_ORDER.filter((modality) => cardsByModality.value.has(modality)).map((modality) => ({ + modality, + label: translate(`standalone.modality.${modality}`), + glyph: MODALITY_GLYPH[modality], + count: cardsByModality.value.get(modality)!.length, + })) + ) + + const modalityOfSelected = computed(() => { + const selected = templateCards.value.find((o) => o.value === selectedValue.value) + return selected ? modalityOf(selected) : null + }) + + const activeModality = ref(null) + + /** Keep the active tab valid: prefer the selected template's modality, then + * the current tab if it still exists, then the first populated tab. */ + watch( + [tabs, modalityOfSelected], + () => { + const available = tabs.value.map((t) => t.modality) + if (available.length === 0) { + activeModality.value = null + return + } + if (modalityOfSelected.value && available.includes(modalityOfSelected.value)) { + activeModality.value = modalityOfSelected.value + } else if (!activeModality.value || !available.includes(activeModality.value)) { + activeModality.value = available[0]! + } + }, + { immediate: true } + ) + + const visibleCards = computed(() => + activeModality.value ? (cardsByModality.value.get(activeModality.value) ?? []) : [] + ) + + function selectTab(modality: Modality): void { + activeModality.value = modality + } + + return { tabs, activeModality, visibleCards, selectTab } +} diff --git a/src/renderer/src/composables/useThumbnailPrefetch.test.ts b/src/renderer/src/composables/useThumbnailPrefetch.test.ts new file mode 100644 index 000000000..d8aa4c910 --- /dev/null +++ b/src/renderer/src/composables/useThumbnailPrefetch.test.ts @@ -0,0 +1,182 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { effectScope } from 'vue' + +import { useThumbnailPrefetch } from './useThumbnailPrefetch' + +// Drive idle callbacks manually so tests control exactly when a queued fetch +// runs — no real timers, no flakiness. +let idleQueue: Array<() => void> = [] +let nextHandle = 1 + +// Track the Image instances the composable creates + their live handlers, so we +// can assert concurrency, completion, and leak-freedom (handlers nulled). +interface FakeImage { + src: string + onload: (() => void) | null + onerror: (() => void) | null +} +let images: FakeImage[] = [] + +beforeEach(() => { + idleQueue = [] + nextHandle = 1 + images = [] + + vi.stubGlobal('requestIdleCallback', (cb: () => void) => { + idleQueue.push(cb) + return nextHandle++ + }) + vi.stubGlobal('cancelIdleCallback', (handle: number) => { + // Mark cancelled by index; flushIdle skips holes. + idleQueue[handle - 1] = undefined as unknown as () => void + }) + vi.stubGlobal( + 'Image', + class { + src = '' + onload: (() => void) | null = null + onerror: (() => void) | null = null + constructor() { + const self = this as unknown as FakeImage + images.push(self) + } + } + ) + // Default: a normal (non-metered) connection. + vi.stubGlobal('navigator', { connection: undefined }) +}) + +afterEach(() => vi.unstubAllGlobals()) + +/** Run all currently-queued idle callbacks (FIFO), as the browser would when idle. + * Idle work that schedules more idle work surfaces on the next flush. */ +function flushIdle(): void { + const pending = idleQueue + idleQueue = [] + for (const cb of pending) cb?.() +} + +/** Settle the Nth in-flight image as loaded. */ +function loadImage(i: number): void { + images[i]!.onload?.() +} + +function run(fn: () => T): { result: T; dispose: () => void } { + const scope = effectScope() + const result = scope.run(fn)! + return { result, dispose: () => scope.stop() } +} + +describe('useThumbnailPrefetch', () => { + it('warms each url exactly once, de-duplicating repeats', () => { + const { result } = run(() => useThumbnailPrefetch({ concurrency: 10 })) + result.prefetch(['a.webp', 'b.webp', 'a.webp', null, undefined]) + flushIdle() + expect(images.map((i) => i.src).sort()).toEqual(['a.webp', 'b.webp']) + + result.prefetch(['a.webp']) // already seen → no new fetch + flushIdle() + expect(images).toHaveLength(2) + }) + + it('caps concurrency and drains as fetches complete', () => { + const { result } = run(() => useThumbnailPrefetch({ concurrency: 2 })) + result.prefetch(['a', 'b', 'c', 'd']) + flushIdle() + expect(images).toHaveLength(2) // only 2 in flight + + loadImage(0) + flushIdle() + expect(images).toHaveLength(3) // a freed a slot + + loadImage(1) + loadImage(2) + flushIdle() + expect(images).toHaveLength(4) // all drained + }) + + it('defers while busy and resumes once important work finishes', () => { + // Fake only the backoff timer; keep our requestIdleCallback stub intact. + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + try { + let busy = true + const { result } = run(() => useThumbnailPrefetch({ isBusy: () => busy, concurrency: 5 })) + result.prefetch(['a', 'b']) + flushIdle() // busy → backs off on a timer, nothing fetched + expect(images).toHaveLength(0) + + busy = false + vi.advanceTimersByTime(1500) // backoff elapses → pump retries + flushIdle() // now-scheduled idle loads run + expect(images.length).toBeGreaterThan(0) + } finally { + vi.useRealTimers() + } + }) + + it('skips entirely under data-saver', () => { + vi.stubGlobal('navigator', { connection: { saveData: true } }) + const { result } = run(() => useThumbnailPrefetch()) + result.prefetch(['a', 'b']) + flushIdle() + expect(images).toHaveLength(0) + }) + + it('skips entirely on a 2g connection', () => { + vi.stubGlobal('navigator', { connection: { effectiveType: '2g' } }) + const { result } = run(() => useThumbnailPrefetch()) + result.prefetch(['a']) + flushIdle() + expect(images).toHaveLength(0) + }) + + it('releases image handlers on settle (no leak)', () => { + const { result } = run(() => useThumbnailPrefetch({ concurrency: 1 })) + result.prefetch(['a']) + flushIdle() + loadImage(0) + expect(images[0]!.onload).toBeNull() + expect(images[0]!.onerror).toBeNull() + }) + + it('detaches handlers of still-loading images on dispose (no leak)', () => { + const { result, dispose } = run(() => useThumbnailPrefetch({ concurrency: 1 })) + result.prefetch(['a']) + flushIdle() + expect(images[0]!.onload).not.toBeNull() // in flight + dispose() + expect(images[0]!.onload).toBeNull() + expect(images[0]!.onerror).toBeNull() + }) + + it('cancels pending work and queues nothing more after dispose', () => { + const { result, dispose } = run(() => useThumbnailPrefetch({ concurrency: 1 })) + result.prefetch(['a', 'b', 'c']) + flushIdle() + expect(images).toHaveLength(1) + + dispose() + loadImage(0) // completing the in-flight one must not pump the queue + flushIdle() + expect(images).toHaveLength(1) + + result.prefetch(['d']) // disposed → ignored + flushIdle() + expect(images).toHaveLength(1) + }) + + it('falls back to setTimeout when requestIdleCallback is unavailable', () => { + vi.stubGlobal('requestIdleCallback', undefined) + vi.stubGlobal('cancelIdleCallback', undefined) + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + try { + const { result } = run(() => useThumbnailPrefetch({ concurrency: 5 })) + result.prefetch(['a', 'b']) + expect(images).toHaveLength(0) // deferred, not run synchronously + vi.advanceTimersByTime(50) + expect(images.map((i) => i.src).sort()).toEqual(['a', 'b']) + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/renderer/src/composables/useThumbnailPrefetch.ts b/src/renderer/src/composables/useThumbnailPrefetch.ts new file mode 100644 index 000000000..fdac88fc1 --- /dev/null +++ b/src/renderer/src/composables/useThumbnailPrefetch.ts @@ -0,0 +1,126 @@ +import { onScopeDispose } from 'vue' + +/** + * Warms image URLs into the browser HTTP cache during idle time so a later + * `` renders instantly; defers entirely while `isBusy()` or on a metered + * link, so it never competes with real work on a low-spec machine. + */ + +interface PrefetchOptions { + /** Returns true when something more important is running; prefetch defers. */ + isBusy?: () => boolean + /** Max concurrent image fetches. */ + concurrency?: number + /** Idle-callback deadline (ms) so a never-idle main thread still drains. */ + idleTimeoutMs?: number +} + +type IdleHandle = number +interface IdleWindow { + requestIdleCallback?: (cb: () => void, opts?: { timeout: number }) => IdleHandle + cancelIdleCallback?: (handle: IdleHandle) => void +} + +/** Delay (ms) for the `setTimeout` fallback when `requestIdleCallback` is absent. + * A few frames — enough to yield to interactive work without forcing the full + * idle deadline (which is a max, not a delay). */ +const FALLBACK_DELAY_MS = 50 + +/** Schedule on the idle queue, falling back to a low-priority timeout. Returns a + * canceller so callers don't branch on which path ran. */ +function scheduleIdle(fn: () => void, timeoutMs: number): () => void { + const w = window as unknown as IdleWindow + if (typeof w.requestIdleCallback === 'function') { + const handle = w.requestIdleCallback(fn, { timeout: timeoutMs }) + return () => w.cancelIdleCallback?.(handle) + } + const id = window.setTimeout(fn, FALLBACK_DELAY_MS) + return () => window.clearTimeout(id) +} + +/** True on a metered / very slow connection where speculative fetching would + * hurt more than help. Conservative: only bails on explicit data-saver or 2g. */ +function shouldSkipForNetwork(): boolean { + const conn = (navigator as unknown as { + connection?: { saveData?: boolean; effectiveType?: string } + }).connection + if (!conn) return false + if (conn.saveData) return true + return conn.effectiveType === '2g' || conn.effectiveType === 'slow-2g' +} + +export function useThumbnailPrefetch(options: PrefetchOptions = {}): { + prefetch: (urls: readonly (string | null | undefined)[]) => void +} { + const { isBusy = () => false, concurrency = 3, idleTimeoutMs = 3000 } = options + + /** How long to wait before re-checking the busy gate, so a sustained + * install/launch doesn't busy-spin the idle queue. */ + const BUSY_BACKOFF_MS = 1500 + + const queue: string[] = [] + const seen = new Set() + const cancellers = new Set<() => void>() + const inFlightImages = new Set() + let inFlight = 0 + let disposed = false + + function pump(): void { + if (disposed || queue.length === 0) return + // Important work wins: back off (timer, not idle) and re-check later rather + // than competing for the network/CPU now. + if (isBusy()) { + const id = window.setTimeout(() => { cancellers.delete(cancel); pump() }, BUSY_BACKOFF_MS) + const cancel = (): void => window.clearTimeout(id) + cancellers.add(cancel) + return + } + while (inFlight < concurrency && queue.length > 0) { + const url = queue.shift()! + inFlight++ + const cancel = scheduleIdle(() => { + cancellers.delete(cancel) + if (!disposed) load(url) + }, idleTimeoutMs) + cancellers.add(cancel) + } + } + + function load(url: string): void { + const img = new Image() + inFlightImages.add(img) + const done = (): void => { + img.onload = null + img.onerror = null + inFlightImages.delete(img) + inFlight-- + if (!disposed) pump() + } + img.onload = done + img.onerror = done + img.src = url + } + + function prefetch(urls: readonly (string | null | undefined)[]): void { + if (disposed || shouldSkipForNetwork()) return + for (const url of urls) { + if (!url || seen.has(url)) continue + seen.add(url) + queue.push(url) + } + pump() + } + + onScopeDispose(() => { + disposed = true + queue.length = 0 + for (const cancel of cancellers) cancel() + cancellers.clear() + // Detach handlers on still-loading images so their closures can be GC'd + // without waiting for the request to settle. + for (const img of inFlightImages) { img.onload = null; img.onerror = null } + inFlightImages.clear() + }) + + return { prefetch } +} diff --git a/src/renderer/src/panel/PanelApp.vue b/src/renderer/src/panel/PanelApp.vue index a5e394030..23da6e7e5 100644 --- a/src/renderer/src/panel/PanelApp.vue +++ b/src/renderer/src/panel/PanelApp.vue @@ -37,6 +37,7 @@ import { SUCCESS_ACTION_GO_DASHBOARD, SUCCESS_ACTION_OPEN_INSTANCE } from '../lib/progressTerminalPresets' +import { useThumbnailPrefetch } from '../composables/useThumbnailPrefetch' import type { Installation } from '../types/ipc' const { t } = useI18n() @@ -157,6 +158,26 @@ const { switchPanel } = overlays +// Warm picker thumbnails during idle so the install wizard shows images, not +// loaders; defers while an instance/overlay is active so it never competes. +const { prefetch: prefetchThumbnails } = useThumbnailPrefetch({ + isBusy: () => sessionStore.runningTabCount > 0 || currentOverlay.value !== null +}) + +async function warmTemplateThumbnails(): Promise { + try { + const options = await window.api.getFieldOptions('standalone', 'bundledTemplate', {}, {}) + prefetchThumbnails( + options.map((o) => { + const url = o.data?.thumbnailUrl + return typeof url === 'string' ? url : null + }) + ) + } catch { + // Best-effort warm-up; the picker still loads thumbnails on demand. + } +} + // E2E surface: tests drive UI-level flows (e.g. inject a finished // failed op to render ProgressModal's error state) by calling into // `handleShowProgress` from outside the Vue tree. Gated on the @@ -487,6 +508,8 @@ onMounted(async () => { // after a partial-bootstrap failure. resolveBootstrap?.() resolveBootstrap = null + // Fire-and-forget after the panel is interactive; self-defers when busy. + void warmTemplateThumbnails() } }) diff --git a/src/renderer/src/views/InstallWizardModal.vue b/src/renderer/src/views/InstallWizardModal.vue index 960696b7b..bc9b23c7d 100644 --- a/src/renderer/src/views/InstallWizardModal.vue +++ b/src/renderer/src/views/InstallWizardModal.vue @@ -193,11 +193,13 @@ function selectTemplate(option: FieldOption): void { * disk too small, or the `skipTemplatePickerStep` opt-out). */ async function handleConfigureContinue(): Promise { if (shouldShowPickerStep.value) { - // Default the selection to the first real template (Image) rather than the - // "None" sentinel, so the showcase leads. - const firstReal = templateOptions.value.find((o) => o.value !== NO_TEMPLATE_VALUE) - if (firstReal && selections.value.bundledTemplate?.value === NO_TEMPLATE_VALUE) { - selections.value.bundledTemplate = firstReal + // Lead with a real template rather than the "None" sentinel — prefer the + // recommended pick (the lightest "wow"), falling back to the first real one. + if (selections.value.bundledTemplate?.value === NO_TEMPLATE_VALUE) { + const lead = + templateOptions.value.find((o) => o.value !== NO_TEMPLATE_VALUE && o.recommended) ?? + templateOptions.value.find((o) => o.value !== NO_TEMPLATE_VALUE) + if (lead) selections.value.bundledTemplate = lead } if (instPath.value) fetchDiskSpace(instPath.value) step.value = 'template' @@ -947,12 +949,12 @@ defineExpose({ open }) role="radio" :aria-checked="currentSource?.id === s.id" :class="[ - 'config-method', - { 'config-method--selected': currentSource?.id === s.id } + 'brand-pill', + { 'brand-pill--selected': currentSource?.id === s.id } ]" @click="selectSourceCard(s)" > - {{ s.label }} + {{ s.label }} {{ $t('newInstall.recommended') }} @@ -1178,7 +1180,7 @@ defineExpose({ open }) height: 100%; max-height: 100%; width: 100%; - max-width: 640px; + max-width: 960px; margin: 0 auto; display: flex; flex-direction: column; @@ -1190,8 +1192,7 @@ defineExpose({ open }) } .template-card { width: 100%; - min-height: clamp(360px, 56vh, 560px); - max-height: min(70vh, 100%); + max-height: min(80vh, 100%); } .template-alerts { display: flex; @@ -1495,43 +1496,14 @@ defineExpose({ open }) transform: translateY(0); } -/* Install-method chips: pill picker inside Advanced for swapping source without leaving the brand chrome. */ +/* Install-method chips: pill picker inside Advanced for swapping source without + * leaving the brand chrome. Chips use the shared `.brand-pill` in main.css. */ .config-method-row { display: flex; flex-wrap: wrap; gap: 8px; margin-bottom: 16px; } -.config-method { - display: inline-flex; - align-items: center; - gap: 6px; - padding: 8px 14px; - border: 1px solid var(--brand-surface-border); - border-radius: 6px; - background: var(--brand-surface-bg); - color: var(--neutral-200); - font: inherit; - font-size: 13px; - cursor: pointer; - transition: - border-color 160ms ease, - background 160ms ease, - color 160ms ease; -} -.config-method:hover { - border-color: var(--brand-surface-border-hover); - background: var(--brand-surface-bg-hover); -} -.config-method:focus-visible { - outline: 2px solid var(--focus-ring); - outline-offset: 2px; -} -.config-method--selected { - border-color: var(--accent); - background: color-mix(in srgb, var(--accent) 14%, transparent); - color: var(--neutral-100); -} .config-continue { min-width: 120px; diff --git a/src/renderer/src/views/chooser/ChooserInstallTile.vue b/src/renderer/src/views/chooser/ChooserInstallTile.vue index 136f57873..cbe190a11 100644 --- a/src/renderer/src/views/chooser/ChooserInstallTile.vue +++ b/src/renderer/src/views/chooser/ChooserInstallTile.vue @@ -1,11 +1,11 @@ @@ -575,12 +613,15 @@ async function handleExpandedPrimaryAction(restartInPlace: boolean): Promise ({ })) vi.mock('../../views/comfyUISettings/MoreMenu.vue', () => ({ default: { - props: ['open'], + props: ['open', 'actions'], template: '
menu
', }, })) @@ -528,37 +528,114 @@ describe('ComfyUISettingsContent', () => { }) } - it('labels "Start" and emits restartInPlace=false when not running', async () => { - const w = await mountContent({ activeInstallationId: 'inst-1' }) + // The footer label still follows run-state (Start/Restart/Switch); the emit + // now carries the resolved NavDecision instead of a bare boolean. + function emittedDecision(wrapper: VueWrapper) { + const calls = wrapper.emitted('primary-action') as unknown[][] | undefined + return calls?.[0]?.[0] as { window: string; verb: string } | undefined + } + + it('labels "Start" and emits a same-window switch decision when not running', async () => { + // Dashboard host (no active install) selecting a stopped local install. + const w = await mountContent({ currentView: 'dashboard', currentCategory: null, activeInstallationId: null }) expect(w.find('.settings-v2-relaunch').text()).toBe('Start') await w.find('.settings-v2-relaunch').trigger('click') - expect(w.emitted('primary-action')).toEqual([[false]]) + expect(emittedDecision(w)).toMatchObject({ window: 'same', verb: 'switch' }) }) - it('labels "Restart" and emits restartInPlace=true when running in THIS window', async () => { + it('labels "Restart" and emits a restart decision when running in THIS window', async () => { + // An install running in this window IS an instance host (computeViewKind). markRunning('inst-1') - const w = await mountContent({ activeInstallationId: 'inst-1' }) + const w = await mountContent({ currentView: 'instance', currentCategory: 'local', activeInstallationId: 'inst-1' }) expect(w.find('.settings-v2-relaunch').text()).toBe('Restart') await w.find('.settings-v2-relaunch').trigger('click') - expect(w.emitted('primary-action')).toEqual([[true]]) + expect(emittedDecision(w)).toMatchObject({ window: 'same', verb: 'restart' }) }) - it('labels "Switch" and emits restartInPlace=false when running in ANOTHER window', async () => { - // Host attached to 'other'; selected 'inst-1' runs elsewhere. + it('labels "Switch" and emits a focus decision when running in ANOTHER window', async () => { + // Host attached to 'other'; selected 'inst-1' runs elsewhere → focus it. markRunning('inst-1') - const w = await mountContent({ activeInstallationId: 'other' }) + const w = await mountContent({ currentView: 'instance', currentCategory: 'local', activeInstallationId: 'other' }) expect(w.find('.settings-v2-relaunch').text()).toBe('Switch') await w.find('.settings-v2-relaunch').trigger('click') - expect(w.emitted('primary-action')).toEqual([[false]]) + expect(emittedDecision(w)).toMatchObject({ verb: 'focus' }) }) it('treats a running install as "Switch" on an install-less (dashboard) host', async () => { - // No activeInstallationId → no in-place session to restart, so always Switch. + // No activeInstallationId → no in-place session to restart, so Switch/focus. markRunning('inst-1') - const w = await mountContent({ activeInstallationId: null }) + const w = await mountContent({ currentView: 'dashboard', currentCategory: null, activeInstallationId: null }) expect(w.find('.settings-v2-relaunch').text()).toBe('Switch') await w.find('.settings-v2-relaunch').trigger('click') - expect(w.emitted('primary-action')).toEqual([[false]]) + expect(emittedDecision(w)).toMatchObject({ verb: 'focus' }) + }) + }) + + // A running-elsewhere target (verb `focus`) can't open a second window, so the + // caret offers Stop (remote, which has a stop action) or nothing (cloud, which + // doesn't) — never "Open in new window". + describe('caret for a running-elsewhere target', () => { + function markRunning(installId: string): void { + useSessionStore().runningInstances.set(installId, { + installationId: installId, + installationName: 'X', + mode: '', + }) + } + // The caret split-button (and its MoreMenu) render only when caretActions is + // non-empty. Read the caret menu's `actions` prop; the caret carries `stop` + // or `nav:*` ids, distinguishing it from the pinBottom More menu. + function caretActions(wrapper: VueWrapper): { id: string }[] | undefined { + if (!wrapper.find('.settings-v2-cta-caret').exists()) return undefined + const menu = wrapper + .findAllComponents({ name: 'MoreMenu' }) + .map((m) => m.props('actions') as { id: string }[] | undefined) + .find((acts) => acts?.some((a) => a.id === 'stop' || a.id.startsWith('nav:'))) + return menu + } + + it('remote running elsewhere → caret offers Stop, not "Open in new window"', async () => { + // Remote install gets a `stop` action (only cloud is excluded). + useComfyUISettingsState.pinBottomActions.value = [{ id: 'stop', label: 'Stop' }] + markRunning('inst-1') + const w = await mountContent({ + currentView: 'instance', + currentCategory: 'local', + activeInstallationId: 'other', + installation: { ...SAMPLE_INSTALL, sourceCategory: 'remote' }, + }) + expect(w.find('.settings-v2-relaunch').text()).toBe('Switch') + const actions = caretActions(w) + expect(actions?.map((a) => a.id)).toEqual(['stop']) + }) + + it('cloud running elsewhere → no caret (cloud has no stop action)', async () => { + // Cloud is excluded from the synthetic Stop action, so no caret at all. + useComfyUISettingsState.pinBottomActions.value = [] + markRunning('inst-1') + const w = await mountContent({ + currentView: 'instance', + currentCategory: 'local', + activeInstallationId: 'other', + installation: { ...SAMPLE_INSTALL, sourceCategory: 'cloud' }, + }) + expect(w.find('.settings-v2-relaunch').text()).toBe('Switch') + expect(w.find('.settings-v2-cta-caret').exists()).toBe(false) + }) + + it('stopped cloud target (dashboard host) → caret still offers "Open in new window"', async () => { + // Not running → a new window IS openable, so the nav caret stays. The + // dashboard→cloud(stopped) cell is the one that carries the new-window + // secondary (instance→cloud(stopped) is primary open-new, no caret). + useComfyUISettingsState.pinBottomActions.value = [] + const w = await mountContent({ + currentView: 'dashboard', + currentCategory: null, + activeInstallationId: null, + installation: { ...SAMPLE_INSTALL, sourceCategory: 'cloud' }, + }) + const actions = caretActions(w) + expect(actions?.some((a) => a.id.startsWith('nav:'))).toBe(true) }) }) diff --git a/src/renderer/src/components/settings/ComfyUISettingsContent.vue b/src/renderer/src/components/settings/ComfyUISettingsContent.vue index dd50aab61..a13a1522e 100644 --- a/src/renderer/src/components/settings/ComfyUISettingsContent.vue +++ b/src/renderer/src/components/settings/ComfyUISettingsContent.vue @@ -1,12 +1,42 @@ + + diff --git a/src/renderer/src/components/settings/ComfyUISettingsContent.vue b/src/renderer/src/components/settings/ComfyUISettingsContent.vue index a13a1522e..3290b7ab4 100644 --- a/src/renderer/src/components/settings/ComfyUISettingsContent.vue +++ b/src/renderer/src/components/settings/ComfyUISettingsContent.vue @@ -440,6 +440,9 @@ function openArgsPage(): void { subPageTransition.value = 'subpage-push' subPage.value = 'args' } +function handleOpenPath(path: string): void { + if (path) void window.api.openPath(path) +} function closeSubPage(): void { subPageTransition.value = 'subpage-pop' subPage.value = null @@ -1067,6 +1070,7 @@ defineExpose({ @update-field="updateField" @run-action="runAction" @open-args-page="openArgsPage" + @open-path="handleOpenPath" />
diff --git a/src/renderer/src/lib/openablePath.test.ts b/src/renderer/src/lib/openablePath.test.ts new file mode 100644 index 000000000..027feeb83 --- /dev/null +++ b/src/renderer/src/lib/openablePath.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import { isOpenablePathString } from './openablePath' + +describe('isOpenablePathString', () => { + it('accepts POSIX absolute and home paths', () => { + expect(isOpenablePathString('/home/user/ComfyUI')).toBe(true) + expect(isOpenablePathString('~/ComfyUI')).toBe(true) + }) + + it('accepts Windows drive and UNC paths', () => { + expect(isOpenablePathString('C:\\Users\\me\\ComfyUI')).toBe(true) + expect(isOpenablePathString('C:/Users/me/ComfyUI')).toBe(true) + expect(isOpenablePathString('\\\\server\\share\\models')).toBe(true) + }) + + it('rejects empty and placeholder values', () => { + expect(isOpenablePathString('')).toBe(false) + expect(isOpenablePathString(' ')).toBe(false) + expect(isOpenablePathString('—')).toBe(false) + }) + + it('rejects URLs', () => { + expect(isOpenablePathString('https://github.com/comfyanonymous/ComfyUI')).toBe(false) + expect(isOpenablePathString('file:///home/user/x')).toBe(false) + expect(isOpenablePathString('http://localhost:8188/')).toBe(false) + }) + + it('rejects SSH / scp-style git remotes', () => { + expect(isOpenablePathString('git@github.com:comfyanonymous/ComfyUI.git')).toBe(false) + }) + + it('rejects date-like values that merely contain slashes', () => { + expect(isOpenablePathString('2024/01/02')).toBe(false) + expect(isOpenablePathString('01-02-2024')).toBe(false) + }) + + it('keeps date-prefixed paths openable', () => { + expect(isOpenablePathString('2024/01/02/models')).toBe(true) + expect(isOpenablePathString('2024-01-02/models')).toBe(true) + }) + + it('rejects plain text without separators', () => { + expect(isOpenablePathString('master')).toBe(false) + expect(isOpenablePathString('ComfyUI')).toBe(false) + expect(isOpenablePathString('v1.2.3')).toBe(false) + }) +}) diff --git a/src/renderer/src/lib/openablePath.ts b/src/renderer/src/lib/openablePath.ts new file mode 100644 index 000000000..3f20c19b0 --- /dev/null +++ b/src/renderer/src/lib/openablePath.ts @@ -0,0 +1,19 @@ +/** + * Shared guard for "is this string safe to open in the OS file manager?". + * + * Used by the readonly path displays so that only real local filesystem paths + * become clickable open-folder targets — never URLs, SSH/Git remotes, or dates + * that merely contain slashes. + * + * Windows notes: drive paths (`C:\…`, `C:/…`) and UNC paths (`\\server\share`) + * are openable. A bare drive-relative value like `C:foo` (no separator) is not + * recognised as a path and stays non-clickable. + */ +export function isOpenablePathString(value: string): boolean { + const v = value.trim() + if (!v || v === '—') return false + if (/^[a-z][a-z0-9+.-]*:\/\//i.test(v)) return false // URL scheme: http://, file://, … + if (/^[^\s/\\@]+@[^\s/\\@]+:/.test(v)) return false // scp/SSH remote: git@github.com:owner/repo + if (/^\d{1,4}[-/.]\d{1,2}[-/.]\d{1,4}$/.test(v)) return false // bare date-ish value only: 2024/01/02, 01-02-2024 + return v.includes('/') || v.includes('\\') || v.startsWith('~') +} diff --git a/src/renderer/src/views/InstallWizardModal.test.ts b/src/renderer/src/views/InstallWizardModal.test.ts new file mode 100644 index 000000000..6d21a8e35 --- /dev/null +++ b/src/renderer/src/views/InstallWizardModal.test.ts @@ -0,0 +1,61 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { flushPromises, mount } from '@vue/test-utils' +import { createI18n } from 'vue-i18n' + +import { en } from '../lib/i18nMessages.ts' +import InstallWizardModal from './InstallWizardModal.vue' + +function makeI18n() { + return createI18n({ legacy: false, locale: 'en', messages: { en } }) +} + +function mountModal() { + return mount(InstallWizardModal, { + global: { + plugins: [makeI18n()], + stubs: { BrandTakeoverLayout: { template: '
' } }, + }, + }) +} + +beforeEach(() => { + window.api = { + openPath: vi.fn().mockResolvedValue(undefined), + browseFolder: vi.fn().mockResolvedValue('/home/user/Picked'), + detectGPU: vi.fn().mockResolvedValue(null), + getDefaultInstallDir: vi.fn().mockResolvedValue('/home/user/ComfyUI'), + getSources: vi.fn().mockResolvedValue([]), + validateHardware: vi.fn().mockResolvedValue({ supported: true }), + getSetting: vi.fn().mockResolvedValue(false), + getInstallationsSummary: vi.fn().mockResolvedValue({ localCount: 0 }), + getUniqueName: vi.fn().mockResolvedValue('ComfyUI'), + getDiskSpace: vi.fn().mockResolvedValue(null), + validateInstallPath: vi.fn().mockResolvedValue([]), + } as unknown as typeof window.api +}) + +describe('InstallWizardModal install-location field', () => { + it('renders the default install location as a clickable path that opens the folder', async () => { + const wrapper = mountModal() + ;(wrapper.vm as unknown as { open: () => Promise }).open() + await flushPromises() + + const pathBtn = wrapper.find('button.config-path-open') + expect(pathBtn.exists()).toBe(true) + expect(pathBtn.text()).toBe('/home/user/ComfyUI') + + await pathBtn.trigger('click') + expect(window.api.openPath).toHaveBeenCalledWith('/home/user/ComfyUI') + }) + + it('renders a blank install location as inert (non-clickable, never opens a folder)', async () => { + ;(window.api.getDefaultInstallDir as ReturnType).mockResolvedValue('') + const wrapper = mountModal() + ;(wrapper.vm as unknown as { open: () => Promise }).open() + await flushPromises() + + expect(wrapper.find('button.config-path-open').exists()).toBe(false) + expect(wrapper.find('.config-path-open--static').exists()).toBe(true) + expect(window.api.openPath).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/views/InstallWizardModal.vue b/src/renderer/src/views/InstallWizardModal.vue index bc9b23c7d..a2130efe0 100644 --- a/src/renderer/src/views/InstallWizardModal.vue +++ b/src/renderer/src/views/InstallWizardModal.vue @@ -661,6 +661,10 @@ async function handleBrowse(): Promise { if (chosen) instPath.value = chosen } +function handleOpenInstPath(): void { + if (instPath.value) void window.api.openPath(instPath.value) +} + async function handleSave(): Promise { const source = currentSource.value if (!source) return @@ -891,19 +895,21 @@ defineExpose({ open }) class="config-field" :class="{ 'config-field--disabled': currentSource?.skipInstall }" > - +
diff --git a/src/renderer/src/views/TrackModal.test.ts b/src/renderer/src/views/TrackModal.test.ts index 32ce3bd66..1a821724e 100644 --- a/src/renderer/src/views/TrackModal.test.ts +++ b/src/renderer/src/views/TrackModal.test.ts @@ -6,10 +6,11 @@ import TrackModal from './TrackModal.vue' import type { ProbeResult } from '../types/ipc' /** - * The Install Directory field is browse-only: typing/pasting must not - * change it. The only way to populate it is the Browse button, which - * runs the probe and enables the "Track Install" button when an - * existing install is detected. + * The Install Directory field is browse-only: it has no editable input, so + * typing/pasting is impossible. The only way to populate it is the Browse + * button, which runs the probe and enables the "Track Install" button when an + * existing install is detected. Once populated, the path text is clickable to + * open the folder in the OS file manager. */ // Minimal catalog covering the keys the template reads. Missing keys fall @@ -50,6 +51,7 @@ interface MockApi { browseFolder: ReturnType probeInstallation: ReturnType trackInstallation: ReturnType + openPath: ReturnType } const gitProbe: ProbeResult = { @@ -67,6 +69,7 @@ function installMockApi(overrides: Partial = {}): MockApi { browseFolder: vi.fn().mockResolvedValue(undefined), probeInstallation: vi.fn().mockResolvedValue([gitProbe]), trackInstallation: vi.fn().mockResolvedValue({ ok: true }), + openPath: vi.fn().mockResolvedValue(undefined), ...overrides, } ;(window as unknown as { api: MockApi }).api = api @@ -102,28 +105,33 @@ describe('TrackModal — browse-only install directory', () => { vi.restoreAllMocks() }) - it('renders the install-directory input as readonly', async () => { + it('renders a non-clickable placeholder (no editable input) before a folder is picked', async () => { const wrapper = mountTrack() ;(wrapper.vm as unknown as { open: () => void }).open() await flushPromises() - const input = wrapper.get('#track-path') - expect(input.attributes('readonly')).toBeDefined() + // Browse-only: the directory has no editable text input at all. + expect(wrapper.find('.track-path-input input').exists()).toBe(false) + // Empty → muted placeholder, not a clickable open button. + expect(wrapper.find('button.track-path-open').exists()).toBe(false) + expect(wrapper.find('.track-path-placeholder').exists()).toBe(true) }) - it('does not probe when the user attempts to type into the field', async () => { - const api = installMockApi() + it('opens the folder in the file manager when the populated path text is clicked', async () => { + const api = installMockApi({ + browseFolder: vi.fn().mockResolvedValue('/Users/jo/ComfyUI'), + }) const wrapper = mountTrack() ;(wrapper.vm as unknown as { open: () => void }).open() await flushPromises() - // Programmatically dispatch an input event — readonly prevents real - // keyboard input but a stray input dispatch must not reach probe(). - await wrapper.get('#track-path').trigger('input') + await wrapper.get('button.brand-tertiary').trigger('click') await flushPromises() - expect(api.probeInstallation).not.toHaveBeenCalled() - expect(trackButton(wrapper).attributes('disabled')).toBeDefined() + const pathBtn = wrapper.get('button.track-path-open') + expect(pathBtn.text()).toBe('/Users/jo/ComfyUI') + await pathBtn.trigger('click') + expect(api.openPath).toHaveBeenCalledWith('/Users/jo/ComfyUI') }) it('probes and enables Track Install when a folder is picked via Browse', async () => { diff --git a/src/renderer/src/views/TrackModal.vue b/src/renderer/src/views/TrackModal.vue index 0a4e00645..9cf2a2d96 100644 --- a/src/renderer/src/views/TrackModal.vue +++ b/src/renderer/src/views/TrackModal.vue @@ -92,6 +92,10 @@ async function handleBrowse(): Promise { } } +function handleOpenTrackPath(): void { + if (trackPath.value) void window.api.openPath(trackPath.value) +} + async function probe(dirPath: string): Promise { const generation = ++probeGeneration probing.value = true @@ -237,17 +241,21 @@ defineExpose({ open }) >
- +