Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/components/topbar/CurrentUserButton.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,4 +217,15 @@ describe('CurrentUserButton', () => {
expect(screen.getByText('WorkspaceProfilePic')).toBeInTheDocument()
expect(screen.queryByText('Avatar')).not.toBeInTheDocument()
})

it('shows WorkspaceProfilePic for an active local team workspace', () => {
mockTeamWorkspaceStore.initState.value = 'ready'
mockTeamWorkspaceStore.isInPersonalWorkspace.value = false
mockTeamWorkspaceStore.workspaceName.value = 'My Team'

renderComponent()

expect(screen.getByText('WorkspaceProfilePic')).toBeInTheDocument()
expect(screen.queryByText('Avatar')).not.toBeInTheDocument()
})
})
2 changes: 1 addition & 1 deletion src/components/topbar/CurrentUserButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const showWorkspaceSkeleton = computed(
() => isCloud && initState.value === 'loading'
)
const showWorkspaceIcon = computed(
() => isCloud && initState.value === 'ready' && !isInPersonalWorkspace.value
() => initState.value === 'ready' && !isInPersonalWorkspace.value
)

const workspaceName = computed(() => {
Expand Down
95 changes: 93 additions & 2 deletions src/components/topbar/CurrentUserPopoverLegacy.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { createTestingPinia } from '@pinia/testing'
import { render, screen } from '@testing-library/vue'
import userEvent from '@testing-library/user-event'
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
Expand Down Expand Up @@ -79,6 +80,7 @@ vi.mock('@/composables/billing/useBillingContext', () => ({
subscription: mockSubscription,
balance: mockBalance,
isLoading: mockIsLoading,
isTeamPlan: ref(false),
fetchStatus: mockFetchStatus,
fetchBalance: mockFetchBalance
}))
Expand Down Expand Up @@ -164,7 +166,7 @@ describe('CurrentUserPopoverLegacy', () => {
mockIsLoading.value = false
})

function renderComponent() {
function renderComponent(teamWorkspaceState?: Record<string, unknown>) {
const i18n = createI18n({
legacy: false,
locale: 'en',
Expand All @@ -175,7 +177,15 @@ describe('CurrentUserPopoverLegacy', () => {

render(CurrentUserPopoverLegacy, {
global: {
plugins: [i18n],
plugins: [
i18n,
createTestingPinia({
createSpy: vi.fn,
initialState: teamWorkspaceState
? { teamWorkspace: teamWorkspaceState }
: {}
})
],
stubs: {
Divider: true
}
Expand Down Expand Up @@ -531,4 +541,85 @@ describe('CurrentUserPopoverLegacy', () => {
expect(screen.getByTestId('logout-menu-item')).toBeInTheDocument()
})
})

describe('workspace selector (non-cloud)', () => {
const workspace = (overrides: Record<string, unknown>) => ({
isSubscribed: false,
subscriptionPlan: null,
subscriptionTier: null,
members: [],
pendingInvites: [],
...overrides
})

const readyWorkspaceState = {
initState: 'ready',
activeWorkspaceId: 'ws-personal',
isFetchingWorkspaces: false,
workspaces: [
workspace({
id: 'ws-personal',
name: 'Personal Workspace',
type: 'personal',
role: 'owner'
}),
workspace({
id: 'ws-team',
name: 'Team Comfy',
type: 'team',
role: 'member'
})
]
}

beforeEach(() => {
mockIsCloud.value = false
})

it('stays hidden while the workspace store is not hydrated', () => {
renderComponent()

expect(screen.queryByTestId('workspace-switcher-trigger')).toBeNull()
})

it.for(['ready', 'error'])(
'stays hidden when workspace initialization is %s without workspaces',
(initState) => {
renderComponent({
initState,
activeWorkspaceId: null,
isFetchingWorkspaces: false,
workspaces: []
})

expect(screen.queryByTestId('workspace-switcher-trigger')).toBeNull()
}
)

it('shows the trigger and opens the switcher once the store is ready', async () => {
const { user } = renderComponent(readyWorkspaceState)

const trigger = screen.getByTestId('workspace-switcher-trigger')
expect(trigger).toHaveAttribute('aria-expanded', 'false')
expect(screen.queryByTestId('workspace-switcher-panel')).toBeNull()

await user.click(trigger)

expect(screen.getByTestId('workspace-switcher-panel')).toBeInTheDocument()
expect(trigger).toHaveAttribute('aria-expanded', 'true')
})

it('keeps credits visible but hides top-up for workspace members', () => {
mockCanAccessSubscriptionFeatures.value = false
renderComponent({
...readyWorkspaceState,
activeWorkspaceId: 'ws-team'
})

expect(screen.getByTestId('manage-plan-menu-item')).toHaveTextContent(
enMessages.credits.credits
)
expect(screen.queryByTestId('add-credits-button')).toBeNull()
})
})
})
58 changes: 54 additions & 4 deletions src/components/topbar/CurrentUserPopoverLegacy.vue
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,39 @@
</span>
</div>

<div v-if="showWorkspaceSwitcher" class="relative">
<Button
v-tooltip="{ value: workspaceName, showDelay: 300 }"
variant="muted-textonly"
class="flex h-auto w-full items-center justify-between rounded-lg px-4 py-2 hover:bg-secondary-background-hover"
:aria-expanded="isWorkspaceSwitcherOpen"
data-testid="workspace-switcher-trigger"
@click="isWorkspaceSwitcherOpen = !isWorkspaceSwitcherOpen"
>
<div class="flex w-0 flex-1 items-center gap-2">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pos-test

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): The trigger button sets aria-expanded correctly but is missing aria-controls, aria-haspopup, and an Escape key handler. The panel has no id or role.

Minimum fix:

 <Button
   :aria-expanded="isWorkspaceSwitcherOpen"
+  aria-haspopup="true"
+  aria-controls="workspace-switcher-panel"
+  @keydown.escape.stop="isWorkspaceSwitcherOpen = false"
   ...
 />
 <div
+  id="workspace-switcher-panel"
+  role="menu"
   v-if="isWorkspaceSwitcherOpen"
   ...
 />

The Cloud version has the same gap -- good opportunity to fix both.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in c96280f for both variants. The triggers now expose aria-expanded, aria-haspopup="menu", and aria-controls; the panels have matching IDs and role="menu"; Escape closes the flyout. I also changed the Cloud trigger from a non-semantic div to a native button so the keyboard behavior has a real focus target.

<WorkspaceProfilePic
class="size-6 shrink-0 text-xs"
:workspace-name="workspaceName"
/>
<span class="truncate text-sm text-base-foreground">
{{ workspaceName }}
</span>
</div>
<i class="pi pi-chevron-down shrink-0 text-sm text-muted-foreground" />
</Button>

<div
v-if="isWorkspaceSwitcherOpen"
class="absolute top-0 right-full z-10 mr-4 rounded-lg border border-border-default bg-base-background shadow-[1px_1px_8px_0_rgba(0,0,0,0.4)]"
data-testid="workspace-switcher-panel"
>
<WorkspaceSwitcherPopover @select="isWorkspaceSwitcherOpen = false" />
</div>
</div>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pos-test

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: isWorkspaceSwitcherOpen is a local ref that is never reset when the outer PrimeVue Popover closes. If the user opens the workspace selector, closes the outer popover (Escape or click-outside), then reopens it -- the flyout is already visible. There is also no click-outside handler, so clicking anywhere else in the popover body while the flyout is open does not close it.

The Cloud version (CurrentUserPopoverWorkspace) uses useTemplateRef + onClickOutside to handle this. Either: (a) propagate a @hide event from the parent Popover to reset isWorkspaceSwitcherOpen = false, or (b) add onClickOutside mirroring the Cloud version.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in c96280f. The Local flyout now mirrors the Cloud implementation with useTemplateRef + onClickOutside, excluding the trigger so toggle clicks remain stable. Added coverage for Escape and clicks elsewhere in the outer popover.


<!-- Credits Section -->
<div
v-if="canAccessSubscriptionFeatures"
v-if="canAccessSubscriptionFeatures || showWorkspaceSwitcher"
class="flex items-center gap-2 px-4 py-2"
>
<i class="icon-[lucide--component] text-sm text-credit" />
Expand Down Expand Up @@ -60,7 +90,7 @@
{{ $t('subscription.upgradeToAddCredits') }}
</Button>
<Button
v-else
v-else-if="showAddCredits"
variant="secondary"
size="sm"
class="text-base-foreground"
Expand Down Expand Up @@ -114,7 +144,7 @@
</div>

<div
v-if="canAccessSubscriptionFeatures"
v-if="canAccessSubscriptionFeatures || showWorkspaceSwitcher"
class="flex cursor-pointer items-center gap-2 px-4 py-2 hover:bg-secondary-background-hover"
data-testid="manage-plan-menu-item"
@click="handleOpenPlanAndCreditsSettings"
Expand Down Expand Up @@ -160,9 +190,10 @@

<script setup lang="ts">
import { cn } from '@comfyorg/tailwind-utils'
import { storeToRefs } from 'pinia'
import Divider from 'primevue/divider'
import Skeleton from 'primevue/skeleton'
import { computed, onMounted } from 'vue'
import { computed, onMounted, ref } from 'vue'
import { useI18n } from 'vue-i18n'

import { formatCreditsFromCents } from '@/base/credits/comfyCredits'
Expand All @@ -176,7 +207,11 @@ import { useSubscriptionDialog } from '@/platform/cloud/subscription/composables
import { isCloud } from '@/platform/distribution/types'
import { useTelemetry } from '@/platform/telemetry'
import { useSettingsDialog } from '@/platform/settings/composables/useSettingsDialog'
import WorkspaceProfilePic from '@/platform/workspace/components/WorkspaceProfilePic.vue'
import WorkspaceSwitcherPopover from '@/platform/workspace/components/WorkspaceSwitcherPopover.vue'
import { useWorkspaceTierLabel } from '@/platform/workspace/composables/useWorkspaceTierLabel'
import { useWorkspaceUI } from '@/platform/workspace/composables/useWorkspaceUI'
import { useTeamWorkspaceStore } from '@/platform/workspace/stores/teamWorkspaceStore'
import { useDialogService } from '@/services/dialogService'

const emit = defineEmits<{
Expand All @@ -203,6 +238,21 @@ const { formatTierName } = useWorkspaceTierLabel()
const subscriptionDialog = useSubscriptionDialog()
const { locale, t } = useI18n()

const { initState, workspaces, workspaceName } = storeToRefs(
useTeamWorkspaceStore()
)
const isWorkspaceSwitcherOpen = ref(false)
const showWorkspaceSwitcher = computed(
() => initState.value === 'ready' && workspaces.value.length > 0
)

const { permissions } = useWorkspaceUI()
const showAddCredits = computed(() =>
showWorkspaceSwitcher.value
? permissions.value.canTopUp
: canAccessSubscriptionFeatures.value
)

const subscriptionTierName = computed(() =>
formatTierName(tier.value, subscription.value?.duration === 'ANNUAL')
)
Expand Down
Loading
Loading