From 34fc28a39d169b94395e529d18d33f53066fb7e9 Mon Sep 17 00:00:00 2001 From: Simula_r <18093452+simula-r@users.noreply.github.com> Date: Tue, 27 Jan 2026 20:28:44 -0800 Subject: [PATCH 01/12] Feat/workspaces 5 auth gate check (#8350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fix auth related race conditions with a new WorkspaceAuthGate in App.vue - De dup initialization calls - Add state machine to track state of refreshRemoteConfig - Fix websocket not using new workspace jwt - Misc improvments ## Changes - **What**: Mainly WorkspaceAuthGate.vue - **Breaking**: - **Dependencies**: ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8350-Feat-workspaces-5-auth-gate-check-2f66d73d365081b1a49afcd418fab3e7) by [Unito](https://www.unito.io) --- src/App.vue | 17 +- src/components/auth/WorkspaceAuthGate.test.ts | 206 ++++++++++++++++++ src/components/auth/WorkspaceAuthGate.vue | 126 +++++++++++ src/components/graph/GraphCanvas.vue | 16 +- src/composables/useFeatureFlags.ts | 16 +- src/extensions/core/cloudRemoteConfig.ts | 4 +- .../remoteConfig/refreshRemoteConfig.ts | 12 +- src/platform/remoteConfig/remoteConfig.ts | 24 +- src/scripts/api.ts | 3 +- src/scripts/app.ts | 5 +- src/stores/firebaseAuthStore.ts | 28 ++- src/views/GraphView.vue | 22 -- 12 files changed, 428 insertions(+), 51 deletions(-) create mode 100644 src/components/auth/WorkspaceAuthGate.test.ts create mode 100644 src/components/auth/WorkspaceAuthGate.vue diff --git a/src/App.vue b/src/App.vue index 7c11c4c7b..b5e17500f 100644 --- a/src/App.vue +++ b/src/App.vue @@ -1,11 +1,13 @@ diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 1122fe85f..08e7573a9 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -502,19 +502,9 @@ onMounted(async () => { await workflowPersistence.loadTemplateFromUrlIfPresent() // Accept workspace invite from URL if present (e.g., ?invite=TOKEN) - // Uses watch because feature flags load asynchronously - flag may be false initially - // then become true once remoteConfig or websocket features are loaded - if (inviteUrlLoader) { - const stopWatching = watch( - () => flags.teamWorkspacesEnabled, - async (enabled) => { - if (enabled) { - stopWatching() - await inviteUrlLoader.loadInviteFromUrl() - } - }, - { immediate: true } - ) + // WorkspaceAuthGate ensures flag state is resolved before GraphCanvas mounts + if (inviteUrlLoader && flags.teamWorkspacesEnabled) { + await inviteUrlLoader.loadInviteFromUrl() } // Initialize release store to fetch releases from comfy-api (fire-and-forget) diff --git a/src/composables/useFeatureFlags.ts b/src/composables/useFeatureFlags.ts index ca54bb9c6..e0e69b4c9 100644 --- a/src/composables/useFeatureFlags.ts +++ b/src/composables/useFeatureFlags.ts @@ -1,7 +1,10 @@ import { computed, reactive, readonly } from 'vue' import { isCloud } from '@/platform/distribution/types' -import { remoteConfig } from '@/platform/remoteConfig/remoteConfig' +import { + isAuthenticatedConfigLoaded, + remoteConfig +} from '@/platform/remoteConfig/remoteConfig' import { api } from '@/scripts/api' /** @@ -95,9 +98,20 @@ export function useFeatureFlags() { ) ) }, + /** + * Whether team workspaces feature is enabled. + * IMPORTANT: Returns false until authenticated remote config is loaded. + * This ensures we never use workspace tokens when the feature is disabled, + * and prevents race conditions during initialization. + */ get teamWorkspacesEnabled() { if (!isCloud) return false + // Only return true if authenticated config has been loaded. + // This prevents race conditions where code checks this flag before + // WorkspaceAuthGate has refreshed the config with auth. + if (!isAuthenticatedConfigLoaded.value) return false + return ( remoteConfig.value.team_workspaces_enabled ?? api.getServerFeature(ServerFeatureFlag.TEAM_WORKSPACES_ENABLED, false) diff --git a/src/extensions/core/cloudRemoteConfig.ts b/src/extensions/core/cloudRemoteConfig.ts index 6d775389e..d3c1dc7ac 100644 --- a/src/extensions/core/cloudRemoteConfig.ts +++ b/src/extensions/core/cloudRemoteConfig.ts @@ -16,13 +16,15 @@ useExtensionService().registerExtension({ const { isLoggedIn } = useCurrentUser() const { isActiveSubscription } = useSubscription() + // Refresh config when subscription status changes + // Initial auth-aware refresh happens in WorkspaceAuthGate before app renders watchDebounced( [isLoggedIn, isActiveSubscription], () => { if (!isLoggedIn.value) return void refreshRemoteConfig() }, - { debounce: 256, immediate: true } + { debounce: 256 } ) // Poll for config updates every 10 minutes (with auth) diff --git a/src/platform/remoteConfig/refreshRemoteConfig.ts b/src/platform/remoteConfig/refreshRemoteConfig.ts index 8c87bfcdc..86b499090 100644 --- a/src/platform/remoteConfig/refreshRemoteConfig.ts +++ b/src/platform/remoteConfig/refreshRemoteConfig.ts @@ -1,6 +1,6 @@ import { api } from '@/scripts/api' -import { remoteConfig } from './remoteConfig' +import { remoteConfig, remoteConfigState } from './remoteConfig' interface RefreshRemoteConfigOptions { /** @@ -12,7 +12,12 @@ interface RefreshRemoteConfigOptions { /** * Loads remote configuration from the backend /features endpoint - * and updates the reactive remoteConfig ref + * and updates the reactive remoteConfig ref. + * + * Sets remoteConfigState to: + * - 'anonymous' when loaded without auth + * - 'authenticated' when loaded with auth + * - 'error' when load fails */ export async function refreshRemoteConfig( options: RefreshRemoteConfigOptions = {} @@ -28,6 +33,7 @@ export async function refreshRemoteConfig( const config = await response.json() window.__CONFIG__ = config remoteConfig.value = config + remoteConfigState.value = useAuth ? 'authenticated' : 'anonymous' return } @@ -35,10 +41,12 @@ export async function refreshRemoteConfig( if (response.status === 401 || response.status === 403) { window.__CONFIG__ = {} remoteConfig.value = {} + remoteConfigState.value = 'error' } } catch (error) { console.error('Failed to fetch remote config:', error) window.__CONFIG__ = {} remoteConfig.value = {} + remoteConfigState.value = 'error' } } diff --git a/src/platform/remoteConfig/remoteConfig.ts b/src/platform/remoteConfig/remoteConfig.ts index 40f36522f..3b6ecb96c 100644 --- a/src/platform/remoteConfig/remoteConfig.ts +++ b/src/platform/remoteConfig/remoteConfig.ts @@ -10,10 +10,32 @@ * This module is tree-shaken in OSS builds. */ -import { ref } from 'vue' +import { computed, ref } from 'vue' import type { RemoteConfig } from './types' +/** + * Load state for remote configuration. + * - 'unloaded': No config loaded yet + * - 'anonymous': Config loaded without auth (bootstrap) + * - 'authenticated': Config loaded with auth (user-specific flags available) + * - 'error': Failed to load config + */ +type RemoteConfigState = 'unloaded' | 'anonymous' | 'authenticated' | 'error' + +/** + * Current load state of remote configuration + */ +export const remoteConfigState = ref('unloaded') + +/** + * Whether the authenticated config has been loaded. + * Use this to gate access to user-specific feature flags like teamWorkspacesEnabled. + */ +export const isAuthenticatedConfigLoaded = computed( + () => remoteConfigState.value === 'authenticated' +) + /** * Reactive remote configuration * Updated whenever config is loaded from the server diff --git a/src/scripts/api.ts b/src/scripts/api.ts index 41310b0bd..9a8c13f7d 100644 --- a/src/scripts/api.ts +++ b/src/scripts/api.ts @@ -521,10 +521,11 @@ export class ComfyApi extends EventTarget { } // Get auth token and set cloud params if available + // Uses workspace token (if enabled) or Firebase token if (isCloud) { try { const authStore = await this.getAuthStore() - const authToken = await authStore?.getIdToken() + const authToken = await authStore?.getAuthToken() if (authToken) { params.set('token', authToken) } diff --git a/src/scripts/app.ts b/src/scripts/app.ts index 106603a93..796965b27 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -1349,8 +1349,9 @@ export class ComfyApp { const executionStore = useExecutionStore() executionStore.lastNodeErrors = null - let comfyOrgAuthToken = await useFirebaseAuthStore().getIdToken() - let comfyOrgApiKey = useApiKeyAuthStore().getApiKey() + // Get auth token for backend nodes - uses workspace token if enabled, otherwise Firebase token + const comfyOrgAuthToken = await useFirebaseAuthStore().getAuthToken() + const comfyOrgApiKey = useApiKeyAuthStore().getApiKey() try { while (this.queueItems.length) { diff --git a/src/stores/firebaseAuthStore.ts b/src/stores/firebaseAuthStore.ts index 6073a4304..41bae52a2 100644 --- a/src/stores/firebaseAuthStore.ts +++ b/src/stores/firebaseAuthStore.ts @@ -212,6 +212,31 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => { return token ? { Authorization: `Bearer ${token}` } : null } + /** + * Returns the raw auth token (not wrapped in a header object). + * Priority: workspace token > Firebase token. + * Use this for WebSocket connections and backend node auth. + */ + const getAuthToken = async (): Promise => { + if (flags.teamWorkspacesEnabled) { + const workspaceToken = sessionStorage.getItem( + WORKSPACE_STORAGE_KEYS.TOKEN + ) + const expiresAt = sessionStorage.getItem( + WORKSPACE_STORAGE_KEYS.EXPIRES_AT + ) + + if (workspaceToken && expiresAt) { + const expiryTime = parseInt(expiresAt, 10) + if (Date.now() < expiryTime) { + return workspaceToken + } + } + } + + return await getIdToken() + } + const fetchBalance = async (): Promise => { isFetchingBalance.value = true try { @@ -513,6 +538,7 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => { updatePassword: _updatePassword, deleteAccount: _deleteAccount, getAuthHeader, - getFirebaseAuthHeader + getFirebaseAuthHeader, + getAuthToken } }) diff --git a/src/views/GraphView.vue b/src/views/GraphView.vue index e543a13af..ca03f22a6 100644 --- a/src/views/GraphView.vue +++ b/src/views/GraphView.vue @@ -52,7 +52,6 @@ import { useProgressFavicon } from '@/composables/useProgressFavicon' import { SERVER_CONFIG_ITEMS } from '@/constants/serverConfig' import { i18n, loadLocale } from '@/i18n' import ModelImportProgressDialog from '@/platform/assets/components/ModelImportProgressDialog.vue' -import { useFeatureFlags } from '@/composables/useFeatureFlags' import { isCloud } from '@/platform/distribution/types' import { useSettingStore } from '@/platform/settings/settingStore' import { useTelemetry } from '@/platform/telemetry' @@ -246,27 +245,6 @@ const onReconnected = () => { } } -// Initialize workspace store when feature flag and auth become available -// Uses watch because remoteConfig loads asynchronously after component mount -if (isCloud) { - const { flags } = useFeatureFlags() - - watch( - () => [flags.teamWorkspacesEnabled, firebaseAuthStore.isAuthenticated], - async ([enabled, isAuthenticated]) => { - if (!enabled || !isAuthenticated) return - - const { useTeamWorkspaceStore } = - await import('@/platform/workspace/stores/teamWorkspaceStore') - const workspaceStore = useTeamWorkspaceStore() - if (workspaceStore.initState === 'uninitialized') { - await workspaceStore.initialize() - } - }, - { immediate: true } - ) -} - useEventListener(api, 'status', onStatus) useEventListener(api, 'execution_success', onExecutionSuccess) useEventListener(api, 'reconnecting', onReconnecting) From e8b088ce50cfab937d1ce8c80498de5a00867f55 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Tue, 27 Jan 2026 20:31:00 -0800 Subject: [PATCH 02/12] feat: add composable to determine if user is eligible for nightly survey(s) (#8189) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Adds `useSurveyEligibility` composable that determines whether a user should see a survey based on multiple criteria: nightly localhost build only (`isNightly && !isCloud && !isDesktop`), configurable usage threshold (default 3), 14-day global cooldown between any surveys, once-per-feature-ever display, optional percentage-based sampling, and user opt-out support. All state persists to localStorage. Includes extensive unit tests covering all eligibility conditions. See: - https://github.com/Comfy-Org/ComfyUI_frontend/pull/8149 - https://github.com/Comfy-Org/ComfyUI_frontend/pull/8175 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8189-feat-add-composable-to-determine-if-user-is-eligible-for-nightly-survey-s-2ee6d73d365081f088f2fd76032cc60a) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action Co-authored-by: Alexander Brown --- .../surveys/useSurveyEligibility.test.ts | 259 ++++++++++++++++++ src/platform/surveys/useSurveyEligibility.ts | 97 +++++++ 2 files changed, 356 insertions(+) create mode 100644 src/platform/surveys/useSurveyEligibility.test.ts create mode 100644 src/platform/surveys/useSurveyEligibility.ts diff --git a/src/platform/surveys/useSurveyEligibility.test.ts b/src/platform/surveys/useSurveyEligibility.test.ts new file mode 100644 index 000000000..a99a6e1d9 --- /dev/null +++ b/src/platform/surveys/useSurveyEligibility.test.ts @@ -0,0 +1,259 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { useSurveyEligibility } from './useSurveyEligibility' + +const SURVEY_STATE_KEY = 'Comfy.SurveyState' +const FEATURE_USAGE_KEY = 'Comfy.FeatureUsage' + +const mockDistribution = vi.hoisted(() => ({ + isNightly: true, + isCloud: false, + isDesktop: false +})) + +vi.mock('@/platform/distribution/types', () => ({ + get isNightly() { + return mockDistribution.isNightly + }, + get isCloud() { + return mockDistribution.isCloud + }, + get isDesktop() { + return mockDistribution.isDesktop + } +})) + +describe('useSurveyEligibility', () => { + const defaultConfig = { + featureId: 'test-feature', + typeformId: 'abc123' + } + + beforeEach(() => { + localStorage.clear() + vi.useFakeTimers() + vi.setSystemTime(new Date('2024-06-15T12:00:00Z')) + + mockDistribution.isNightly = true + mockDistribution.isCloud = false + mockDistribution.isDesktop = false + }) + + afterEach(() => { + localStorage.clear() + vi.useRealTimers() + }) + + function setFeatureUsage(featureId: string, useCount: number) { + const existing = JSON.parse(localStorage.getItem(FEATURE_USAGE_KEY) ?? '{}') + existing[featureId] = { + useCount, + firstUsed: Date.now() - 1000, + lastUsed: Date.now() + } + localStorage.setItem(FEATURE_USAGE_KEY, JSON.stringify(existing)) + } + + describe('eligibility checks', () => { + it('is not eligible when not nightly', () => { + mockDistribution.isNightly = false + setFeatureUsage('test-feature', 5) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(false) + }) + + it('is not eligible on cloud', () => { + mockDistribution.isCloud = true + setFeatureUsage('test-feature', 5) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(false) + }) + + it('is not eligible on desktop', () => { + mockDistribution.isDesktop = true + setFeatureUsage('test-feature', 5) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(false) + }) + + it('is not eligible below threshold', () => { + setFeatureUsage('test-feature', 2) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(false) + }) + + it('is eligible when all conditions met', () => { + setFeatureUsage('test-feature', 3) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(true) + }) + + it('respects custom threshold', () => { + setFeatureUsage('test-feature', 5) + + const { isEligible } = useSurveyEligibility({ + ...defaultConfig, + triggerThreshold: 10 + }) + + expect(isEligible.value).toBe(false) + }) + + it('is not eligible when survey already seen', () => { + setFeatureUsage('test-feature', 5) + localStorage.setItem( + SURVEY_STATE_KEY, + JSON.stringify({ + optedOut: false, + seenSurveys: { 'test-feature': Date.now() } + }) + ) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(false) + }) + + it('is not eligible during global cooldown', () => { + setFeatureUsage('test-feature', 5) + const threeDaysAgo = Date.now() - 3 * 24 * 60 * 60 * 1000 + localStorage.setItem( + SURVEY_STATE_KEY, + JSON.stringify({ + optedOut: false, + seenSurveys: { 'other-feature': threeDaysAgo } + }) + ) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(false) + }) + + it('is eligible after global cooldown expires', () => { + setFeatureUsage('test-feature', 5) + const fiveDaysAgo = Date.now() - 5 * 24 * 60 * 60 * 1000 + localStorage.setItem( + SURVEY_STATE_KEY, + JSON.stringify({ + optedOut: false, + seenSurveys: { 'other-feature': fiveDaysAgo } + }) + ) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(true) + }) + + it('is not eligible when opted out', () => { + setFeatureUsage('test-feature', 5) + localStorage.setItem( + SURVEY_STATE_KEY, + JSON.stringify({ + optedOut: true, + seenSurveys: {} + }) + ) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(false) + }) + + it('is not eligible when config disabled', () => { + setFeatureUsage('test-feature', 5) + + const { isEligible } = useSurveyEligibility({ + ...defaultConfig, + enabled: false + }) + + expect(isEligible.value).toBe(false) + }) + }) + + describe('actions', () => { + it('markSurveyShown makes user ineligible', () => { + setFeatureUsage('test-feature', 5) + + const { isEligible, markSurveyShown } = + useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(true) + + markSurveyShown() + + expect(isEligible.value).toBe(false) + }) + + it('optOut prevents all future surveys', () => { + setFeatureUsage('test-feature', 5) + + const { isEligible, optOut } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(true) + + optOut() + + expect(isEligible.value).toBe(false) + }) + + it('resetState restores eligibility', () => { + setFeatureUsage('test-feature', 5) + localStorage.setItem( + SURVEY_STATE_KEY, + JSON.stringify({ + optedOut: true, + seenSurveys: { 'test-feature': Date.now() } + }) + ) + + const { isEligible, resetState } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(false) + + resetState() + + expect(isEligible.value).toBe(true) + }) + }) + + describe('config values', () => { + it('exposes delayMs from config', () => { + const { delayMs } = useSurveyEligibility({ + ...defaultConfig, + delayMs: 10000 + }) + + expect(delayMs.value).toBe(10000) + }) + }) + + describe('persistence', () => { + it('loads existing state from localStorage', () => { + setFeatureUsage('test-feature', 5) + localStorage.setItem( + SURVEY_STATE_KEY, + JSON.stringify({ + optedOut: false, + seenSurveys: { 'test-feature': 1000 } + }) + ) + + const { isEligible } = useSurveyEligibility(defaultConfig) + + expect(isEligible.value).toBe(false) + }) + }) +}) diff --git a/src/platform/surveys/useSurveyEligibility.ts b/src/platform/surveys/useSurveyEligibility.ts new file mode 100644 index 000000000..37929e25c --- /dev/null +++ b/src/platform/surveys/useSurveyEligibility.ts @@ -0,0 +1,97 @@ +import { useStorage } from '@vueuse/core' +import type { MaybeRefOrGetter } from 'vue' +import { computed, toValue } from 'vue' + +import { isCloud, isDesktop, isNightly } from '@/platform/distribution/types' + +import { useFeatureUsageTracker } from './useFeatureUsageTracker' + +interface FeatureSurveyConfig { + /** Feature identifier. Must remain static after initialization. */ + featureId: string + typeformId: string + triggerThreshold?: number + delayMs?: number + enabled?: boolean +} + +interface SurveyState { + optedOut: boolean + seenSurveys: Record +} + +const STORAGE_KEY = 'Comfy.SurveyState' +const GLOBAL_COOLDOWN_MS = 4 * 24 * 60 * 60 * 1000 // 4 days +const DEFAULT_THRESHOLD = 3 +const DEFAULT_DELAY_MS = 5000 + +export function useSurveyEligibility( + config: MaybeRefOrGetter +) { + const state = useStorage(STORAGE_KEY, { + optedOut: false, + seenSurveys: {} + }) + const resolvedConfig = computed(() => toValue(config)) + + const { useCount } = useFeatureUsageTracker(resolvedConfig.value.featureId) + + const threshold = computed( + () => resolvedConfig.value.triggerThreshold ?? DEFAULT_THRESHOLD + ) + const delayMs = computed( + () => resolvedConfig.value.delayMs ?? DEFAULT_DELAY_MS + ) + const isSurveyEnabled = computed(() => resolvedConfig.value.enabled ?? true) + + const isNightlyLocalhost = computed(() => isNightly && !isCloud && !isDesktop) + + const hasReachedThreshold = computed(() => useCount.value >= threshold.value) + + const hasSeenSurvey = computed( + () => !!state.value.seenSurveys[resolvedConfig.value.featureId] + ) + + const isInGlobalCooldown = computed(() => { + const timestamps = Object.values(state.value.seenSurveys) + if (timestamps.length === 0) return false + const lastShown = Math.max(...timestamps) + return Date.now() - lastShown < GLOBAL_COOLDOWN_MS + }) + + const hasOptedOut = computed(() => state.value.optedOut) + + const isEligible = computed(() => { + if (!isSurveyEnabled.value) return false + if (!isNightlyLocalhost.value) return false + if (!hasReachedThreshold.value) return false + if (hasSeenSurvey.value) return false + if (isInGlobalCooldown.value) return false + if (hasOptedOut.value) return false + + return true + }) + + function markSurveyShown() { + state.value.seenSurveys[resolvedConfig.value.featureId] = Date.now() + } + + function optOut() { + state.value.optedOut = true + } + + function resetState() { + state.value = { + optedOut: false, + seenSurveys: {} + } + } + + return { + isEligible, + delayMs, + markSurveyShown, + optOut, + resetState + } +} From 4cf55a7215e28d156b14a4bef89813ae97c2bd6c Mon Sep 17 00:00:00 2001 From: Jin Yi Date: Wed, 28 Jan 2026 14:11:46 +0900 Subject: [PATCH 03/12] [refactor] Manager dialog design improvements (#8247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Refactored Manager dialog components for cleaner button rendering logic and improved visibility. ## Changes - **InfoPanelHeader**: Centralized button rendering logic (try update, uninstall, install) instead of using slot overrides - **PackTryUpdateButton**: Changed variant from `textonly` to `inverted` for better visibility - **InfoPanel**: Removed slot override and unused imports, simplified component - **ManagerDialog**: Design modifications for improved UX - **PackCard/PackBanner**: Component refinements - **useManagerDisplayPacks**: New composable for display pack management ## Review Focus - Button visibility and styling with `inverted` variant - Centralized button logic in InfoPanelHeader ## Before [manager-before.webm](https://github.com/user-attachments/assets/4cef5f10-9cb2-4a31-a095-b170643e481d) ## After [manager-after.webm](https://github.com/user-attachments/assets/9b693b32-59ca-4c88-b7e7-9a78aba19df7) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8247-refactor-Manager-dialog-design-improvements-2f06d73d365081bf9f07efa043caa378) by [Unito](https://www.unito.io) --- src/components/widget/nav/NavItem.vue | 2 +- src/locales/en/main.json | 15 + .../components/manager/ManagerDialog.vue | 391 +++++++----------- .../manager/button/PackInstallButton.vue | 3 +- .../manager/button/PackTryUpdateButton.vue | 3 +- .../manager/button/PackUninstallButton.vue | 8 +- .../manager/button/PackUpdateButton.vue | 7 +- .../manager/infoPanel/InfoPanel.vue | 207 ++++++---- .../manager/infoPanel/InfoPanelHeader.vue | 97 ----- .../manager/infoPanel/InfoPanelMultiItem.vue | 126 +++--- .../components/manager/infoPanel/InfoTabs.vue | 89 ---- .../manager/infoPanel/InfoTextSection.vue | 38 -- .../manager/infoPanel/MetadataRow.vue | 15 - .../tabs/DescriptionTabPanel.test.ts | 50 +-- .../infoPanel/tabs/DescriptionTabPanel.vue | 98 +++-- .../manager/infoPanel/tabs/NodesTabPanel.vue | 16 +- .../infoPanel/tabs/WarningTabPanel.vue | 9 +- .../manager/packBanner/PackBanner.vue | 2 +- .../manager/packCard/PackCard.test.ts | 14 +- .../components/manager/packCard/PackCard.vue | 77 ++-- .../manager/packCard/PackCardFooter.vue | 1 + .../components/manager/packIcon/PackIcon.vue | 52 --- .../manager/packIcon/PackIconStacked.vue | 33 -- .../composables/useManagerDisplayPacks.ts | 217 ++++++++++ .../manager/composables/useRegistrySearch.ts | 7 +- .../manager/types/comfyManagerTypes.ts | 8 +- 26 files changed, 727 insertions(+), 858 deletions(-) delete mode 100644 src/workbench/extensions/manager/components/manager/infoPanel/InfoPanelHeader.vue delete mode 100644 src/workbench/extensions/manager/components/manager/infoPanel/InfoTabs.vue delete mode 100644 src/workbench/extensions/manager/components/manager/infoPanel/InfoTextSection.vue delete mode 100644 src/workbench/extensions/manager/components/manager/infoPanel/MetadataRow.vue delete mode 100644 src/workbench/extensions/manager/components/manager/packIcon/PackIcon.vue delete mode 100644 src/workbench/extensions/manager/components/manager/packIcon/PackIconStacked.vue create mode 100644 src/workbench/extensions/manager/composables/useManagerDisplayPacks.ts diff --git a/src/components/widget/nav/NavItem.vue b/src/components/widget/nav/NavItem.vue index f7b8bbcf1..19579074d 100644 --- a/src/components/widget/nav/NavItem.vue +++ b/src/components/widget/nav/NavItem.vue @@ -3,7 +3,7 @@ v-tooltip.right="{ value: tooltipText, disabled: !isOverflowing, - pt: { text: { class: 'whitespace-nowrap' } } + pt: { text: { class: 'w-max whitespace-nowrap' } } }" class="flex cursor-pointer select-none items-center-safe gap-2 rounded-md px-4 py-3 text-sm transition-colors text-base-foreground" :class=" diff --git a/src/locales/en/main.json b/src/locales/en/main.json index 1cbe43d10..b3465916b 100644 --- a/src/locales/en/main.json +++ b/src/locales/en/main.json @@ -282,6 +282,10 @@ }, "manager": { "title": "Nodes Manager", + "nodePackInfo": "Node Pack Info", + "basicInfo": "Basic Info", + "actions": "Actions", + "selected": "Selected", "legacyMenuNotAvailable": "Legacy manager menu is not available, defaulting to the new manager menu.", "legacyManagerUI": "Use Legacy UI", "legacyManagerUIDescription": "To use the legacy Manager UI, start ComfyUI with --enable-manager-legacy-ui", @@ -295,6 +299,17 @@ "changingVersion": "Changing version from {from} to {to}", "dependencies": "Dependencies", "inWorkflow": "In Workflow", + "nav": { + "allExtensions": "All Extensions", + "notInstalled": "Not Installed", + "installedSection": "INSTALLED", + "allInstalled": "All installed", + "updatesAvailable": "Updates Available", + "conflicting": "Conflicting", + "inWorkflowSection": "IN WORKFLOW", + "allInWorkflow": "All in: {workflowName}", + "missingNodes": "Missing Nodes" + }, "infoPanelEmpty": "Click an item to see the info", "applyChanges": "Apply Changes", "restartToApplyChanges": "To apply changes, please restart ComfyUI", diff --git a/src/workbench/extensions/manager/components/manager/ManagerDialog.vue b/src/workbench/extensions/manager/components/manager/ManagerDialog.vue index 887243315..97f41da56 100644 --- a/src/workbench/extensions/manager/components/manager/ManagerDialog.vue +++ b/src/workbench/extensions/manager/components/manager/ManagerDialog.vue @@ -2,6 +2,7 @@ @@ -76,37 +109,18 @@ - -
-
- - -
- - -
- - - -
+ +
+ + +
@@ -115,7 +129,7 @@
-
+
workflowStore.activeWorkflow?.filename ?? t('manager.inWorkflow') +) + // Navigation items for LeftSidePanel -const navItems = computed(() => [ - { id: ManagerTab.All, label: t('g.all'), icon: 'pi pi-list' }, - { id: ManagerTab.Installed, label: t('g.installed'), icon: 'pi pi-box' }, +const navItems = computed<(NavItemData | NavGroupData)[]>(() => [ { - id: ManagerTab.Workflow, - label: t('manager.inWorkflow'), - icon: 'pi pi-folder' + id: ManagerTab.All, + label: t('manager.nav.allExtensions'), + icon: 'icon-[lucide--list]' }, { - id: ManagerTab.Missing, - label: t('g.missing'), - icon: 'pi pi-exclamation-circle' + id: ManagerTab.NotInstalled, + label: t('manager.nav.notInstalled'), + icon: 'icon-[lucide--globe]' }, { - id: ManagerTab.UpdateAvailable, - label: t('g.updateAvailable'), - icon: 'pi pi-sync' + title: t('manager.nav.installedSection'), + items: [ + { + id: ManagerTab.AllInstalled, + label: t('manager.nav.allInstalled'), + icon: 'icon-[lucide--download]' + }, + { + id: ManagerTab.UpdateAvailable, + label: t('manager.nav.updatesAvailable'), + icon: 'icon-[lucide--refresh-cw]' + }, + { + id: ManagerTab.Conflicting, + label: t('manager.nav.conflicting'), + icon: 'icon-[lucide--triangle-alert]', + badge: conflictDetectionStore.conflictedPackages.length || undefined + } + ] + }, + { + title: t('manager.nav.inWorkflowSection'), + items: [ + { + id: ManagerTab.Workflow, + label: t('manager.nav.allInWorkflow', { + workflowName: workflowName.value + }), + icon: 'icon-[lucide--share-2]' + }, + { + id: ManagerTab.Missing, + label: t('manager.nav.missingNodes'), + icon: 'icon-[lucide--triangle-alert]' + } + ] } ]) const initialTabId = initialTab ?? initialState.selectedTabId ?? ManagerTab.All const selectedNavId = ref(initialTabId) +// Helper to find a nav item by id in the nested structure +const findNavItemById = ( + items: (NavItemData | NavGroupData)[], + id: string | null +): NavItemData | undefined => { + for (const item of items) { + if ('items' in item) { + const found = item.items.find((subItem) => subItem.id === id) + if (found) return found + } else if (item.id === id) { + return item + } + } + return undefined +} + const selectedTab = computed(() => - navItems.value.find((item) => item.id === selectedNavId.value) + findNavItemById(navItems.value, selectedNavId.value) ) const { @@ -315,120 +383,20 @@ const isInitialLoad = computed( () => searchResults.value.length === 0 && searchQuery.value === '' ) -const isEmptySearch = computed(() => searchQuery.value === '') -const displayPacks = ref([]) - +// Use the new composable for tab-based display packs const { - startFetchInstalled, - filterInstalledPack, - installedPacks, - isLoading: isLoadingInstalled, - isReady: installedPacksReady -} = useInstalledPacks() - -const { - startFetchWorkflowPacks, - filterWorkflowPack, - workflowPacks, - isLoading: isLoadingWorkflow, - isReady: workflowPacksReady -} = useWorkflowPacks() - -const filterMissingPacks = (packs: components['schemas']['Node'][]) => - packs.filter((pack) => !comfyManagerStore.isPackInstalled(pack.id)) + displayPacks, + isLoading: isTabLoading, + workflowPacks +} = useManagerDisplayPacks(selectedNavId, searchResults, searchQuery, sortField) +// Tab helpers for template const isUpdateAvailableTab = computed( () => selectedTab.value?.id === ManagerTab.UpdateAvailable ) -const isInstalledTab = computed( - () => selectedTab.value?.id === ManagerTab.Installed -) const isMissingTab = computed( () => selectedTab.value?.id === ManagerTab.Missing ) -const isWorkflowTab = computed( - () => selectedTab.value?.id === ManagerTab.Workflow -) -const isAllTab = computed(() => selectedTab.value?.id === ManagerTab.All) - -const isOutdatedPack = (pack: components['schemas']['Node']) => { - const { isUpdateAvailable } = usePackUpdateStatus(pack) - return isUpdateAvailable.value === true -} -const filterOutdatedPacks = (packs: components['schemas']['Node'][]) => - packs.filter(isOutdatedPack) - -watch( - [isUpdateAvailableTab, installedPacks], - async () => { - if (!isUpdateAvailableTab.value) return - - if (!isEmptySearch.value) { - displayPacks.value = filterOutdatedPacks(installedPacks.value) - } else if ( - !installedPacks.value.length && - !installedPacksReady.value && - !isLoadingInstalled.value - ) { - await startFetchInstalled() - } else { - displayPacks.value = filterOutdatedPacks(installedPacks.value) - } - }, - { immediate: true } -) - -watch( - [isInstalledTab, installedPacks], - async () => { - if (!isInstalledTab.value) return - - if (!isEmptySearch.value) { - displayPacks.value = filterInstalledPack(searchResults.value) - } else if ( - !installedPacks.value.length && - !installedPacksReady.value && - !isLoadingInstalled.value - ) { - await startFetchInstalled() - } else { - displayPacks.value = installedPacks.value - } - }, - { immediate: true } -) - -watch( - [isMissingTab, isWorkflowTab, workflowPacks, installedPacks], - async () => { - if (!isWorkflowTab.value && !isMissingTab.value) return - - if (!isEmptySearch.value) { - displayPacks.value = isMissingTab.value - ? filterMissingPacks(filterWorkflowPack(searchResults.value)) - : filterWorkflowPack(searchResults.value) - } else if ( - !workflowPacks.value.length && - !isLoadingWorkflow.value && - !workflowPacksReady.value - ) { - await startFetchWorkflowPacks() - if (isMissingTab.value) { - await startFetchInstalled() - } - } else { - displayPacks.value = isMissingTab.value - ? filterMissingPacks(workflowPacks.value) - : workflowPacks.value - } - }, - { immediate: true } -) - -watch([isAllTab, searchResults], () => { - if (!isAllTab.value) return - displayPacks.value = searchResults.value -}) const onClickWarningLink = () => { window.open( @@ -439,49 +407,9 @@ const onClickWarningLink = () => { ) } -const onResultsChange = () => { - switch (selectedTab.value?.id) { - case ManagerTab.Installed: - displayPacks.value = isEmptySearch.value - ? installedPacks.value - : filterInstalledPack(searchResults.value) - break - case ManagerTab.Workflow: - displayPacks.value = isEmptySearch.value - ? workflowPacks.value - : filterWorkflowPack(searchResults.value) - break - case ManagerTab.Missing: - if (!isEmptySearch.value) { - displayPacks.value = filterMissingPacks( - filterWorkflowPack(searchResults.value) - ) - } - break - case ManagerTab.UpdateAvailable: - displayPacks.value = isEmptySearch.value - ? filterOutdatedPacks(installedPacks.value) - : filterOutdatedPacks(searchResults.value) - break - default: - displayPacks.value = searchResults.value - } -} - -watch(searchResults, onResultsChange, { flush: 'post' }) -watch(() => comfyManagerStore.installedPacksIds, onResultsChange) - const isLoading = computed(() => { if (isSearchLoading.value) return searchResults.value.length === 0 - if (selectedTab.value?.id === ManagerTab.Installed) { - return isLoadingInstalled.value - } - if ( - selectedTab.value?.id === ManagerTab.Workflow || - selectedTab.value?.id === ManagerTab.Missing - ) { - return isLoadingWorkflow.value - } + if (isTabLoading.value) return true return isInitialLoad.value }) @@ -501,14 +429,16 @@ const isRightPanelOpen = ref(false) watch( () => selectedNodePacks.value.length, - (length) => { - isRightPanelOpen.value = length > 0 + (length, oldLength) => { + if (length > 0 && oldLength === 0) { + isRightPanelOpen.value = true + } } ) const getLoadingCount = () => { switch (selectedTab.value?.id) { - case ManagerTab.Installed: + case ManagerTab.AllInstalled: return comfyManagerStore.installedPacksIds?.size case ManagerTab.Workflow: return workflowPacks.value?.length @@ -578,10 +508,6 @@ whenever(selectedNodePack, async () => { if (packIndex !== -1) { selectedNodePacks.value.splice(packIndex, 1, mergedPack) } - const idx = displayPacks.value.findIndex((p) => p.id === mergedPack.id) - if (idx !== -1) { - displayPacks.value.splice(idx, 1, mergedPack) - } } }) @@ -595,6 +521,7 @@ watch([searchQuery, selectedNavId], () => { pageNumber.value = 0 gridContainer.scrollTop = 0 } + unSelectItems() }) watchEffect(() => { diff --git a/src/workbench/extensions/manager/components/manager/button/PackInstallButton.vue b/src/workbench/extensions/manager/components/manager/button/PackInstallButton.vue index 3000a4a33..0aeab79fe 100644 --- a/src/workbench/extensions/manager/components/manager/button/PackInstallButton.vue +++ b/src/workbench/extensions/manager/components/manager/button/PackInstallButton.vue @@ -1,6 +1,6 @@ diff --git a/src/workbench/extensions/manager/components/manager/button/PackTryUpdateButton.vue b/src/workbench/extensions/manager/components/manager/button/PackTryUpdateButton.vue index 218aaf35f..201259560 100644 --- a/src/workbench/extensions/manager/components/manager/button/PackTryUpdateButton.vue +++ b/src/workbench/extensions/manager/components/manager/button/PackTryUpdateButton.vue @@ -1,7 +1,7 @@ diff --git a/src/workbench/extensions/manager/components/manager/button/PackUninstallButton.vue b/src/workbench/extensions/manager/components/manager/button/PackUninstallButton.vue index 7ff3e42aa..cac261f91 100644 --- a/src/workbench/extensions/manager/components/manager/button/PackUninstallButton.vue +++ b/src/workbench/extensions/manager/components/manager/button/PackUninstallButton.vue @@ -1,10 +1,6 @@ diff --git a/src/workbench/extensions/manager/components/manager/infoPanel/InfoPanel.vue b/src/workbench/extensions/manager/components/manager/infoPanel/InfoPanel.vue index 9f25902cd..9537b540b 100644 --- a/src/workbench/extensions/manager/components/manager/infoPanel/InfoPanel.vue +++ b/src/workbench/extensions/manager/components/manager/infoPanel/InfoPanel.vue @@ -1,62 +1,110 @@