Skip to content

Commit c96280f

Browse files
dante01yoonampagent
andcommitted
fix: address workspace switcher review feedback
Amp-Thread-ID: https://ampcode.com/threads/T-01a016ea-c8f0-77da-842d-58fe769bdd11 Co-authored-by: Amp <amp@ampcode.com>
1 parent 60c4033 commit c96280f

8 files changed

Lines changed: 113 additions & 9 deletions

File tree

src/components/topbar/CurrentUserPopoverLegacy.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,14 +448,35 @@ describe('CurrentUserPopoverLegacy', () => {
448448

449449
const trigger = screen.getByTestId('workspace-switcher-trigger')
450450
expect(trigger).toHaveAttribute('aria-expanded', 'false')
451+
expect(trigger).toHaveAttribute('aria-haspopup', 'menu')
452+
expect(trigger).toHaveAttribute(
453+
'aria-controls',
454+
'workspace-switcher-panel'
455+
)
451456
expect(screen.queryByTestId('workspace-switcher-panel')).toBeNull()
452457

453458
await user.click(trigger)
454459

455-
expect(screen.getByTestId('workspace-switcher-panel')).toBeInTheDocument()
460+
const panel = screen.getByTestId('workspace-switcher-panel')
461+
expect(panel).toBeInTheDocument()
462+
expect(panel).toHaveAttribute('id', 'workspace-switcher-panel')
463+
expect(panel).toHaveAttribute('role', 'menu')
456464
expect(trigger).toHaveAttribute('aria-expanded', 'true')
457465
})
458466

467+
it('closes the switcher on Escape or a click elsewhere', async () => {
468+
const { user } = renderComponent(readyWorkspaceState)
469+
const trigger = screen.getByTestId('workspace-switcher-trigger')
470+
471+
await user.click(trigger)
472+
await user.keyboard('{Escape}')
473+
expect(screen.queryByTestId('workspace-switcher-panel')).toBeNull()
474+
475+
await user.click(trigger)
476+
await user.click(screen.getByText('Test User'))
477+
expect(screen.queryByTestId('workspace-switcher-panel')).toBeNull()
478+
})
479+
459480
it('keeps credits visible but hides top-up for workspace members', () => {
460481
mockCanAccessSubscriptionFeatures.value = false
461482
renderComponent({

src/components/topbar/CurrentUserPopoverLegacy.vue

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,16 @@
3232

3333
<div v-if="showWorkspaceSwitcher" class="relative">
3434
<Button
35+
ref="workspaceSwitcherTrigger"
3536
v-tooltip="{ value: workspaceName, showDelay: 300 }"
3637
variant="muted-textonly"
3738
class="flex h-auto w-full items-center justify-between rounded-lg px-4 py-2 hover:bg-secondary-background-hover"
3839
:aria-expanded="isWorkspaceSwitcherOpen"
40+
aria-haspopup="menu"
41+
aria-controls="workspace-switcher-panel"
3942
data-testid="workspace-switcher-trigger"
4043
@click="isWorkspaceSwitcherOpen = !isWorkspaceSwitcherOpen"
44+
@keydown.escape.stop="isWorkspaceSwitcherOpen = false"
4145
>
4246
<div class="flex w-0 flex-1 items-center gap-2">
4347
<WorkspaceProfilePic
@@ -53,6 +57,9 @@
5357

5458
<div
5559
v-if="isWorkspaceSwitcherOpen"
60+
id="workspace-switcher-panel"
61+
ref="workspaceSwitcherPanel"
62+
role="menu"
5663
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)]"
5764
data-testid="workspace-switcher-panel"
5865
>
@@ -145,10 +152,11 @@
145152
</template>
146153

147154
<script setup lang="ts">
155+
import { onClickOutside } from '@vueuse/core'
148156
import { storeToRefs } from 'pinia'
149157
import Divider from 'primevue/divider'
150158
import Skeleton from 'primevue/skeleton'
151-
import { computed, onMounted, ref } from 'vue'
159+
import { computed, onMounted, ref, useTemplateRef } from 'vue'
152160
import { useI18n } from 'vue-i18n'
153161
154162
import { formatCreditsFromCents } from '@/base/credits/comfyCredits'
@@ -191,6 +199,17 @@ const { initState, workspaces, workspaceName } = storeToRefs(
191199
useTeamWorkspaceStore()
192200
)
193201
const isWorkspaceSwitcherOpen = ref(false)
202+
const workspaceSwitcherTrigger = useTemplateRef('workspaceSwitcherTrigger')
203+
const workspaceSwitcherPanel = useTemplateRef('workspaceSwitcherPanel')
204+
205+
onClickOutside(
206+
workspaceSwitcherPanel,
207+
() => {
208+
isWorkspaceSwitcherOpen.value = false
209+
},
210+
{ ignore: [workspaceSwitcherTrigger] }
211+
)
212+
194213
const showWorkspaceSwitcher = computed(
195214
() => initState.value === 'ready' && workspaces.value.length > 0
196215
)

src/platform/workspace/components/CurrentUserPopoverWorkspace.test.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,15 +174,35 @@ describe('CurrentUserPopoverWorkspace', () => {
174174
it('toggles the workspace switcher panel from the selector row', async () => {
175175
const user = userEvent.setup()
176176
renderComponent()
177+
const trigger = screen.getByTestId('workspace-switcher-trigger')
177178

178179
expect(
179180
screen.queryByTestId('workspace-switcher-panel')
180181
).not.toBeInTheDocument()
182+
expect(trigger).toHaveAttribute('aria-expanded', 'false')
183+
expect(trigger).toHaveAttribute('aria-haspopup', 'menu')
184+
expect(trigger).toHaveAttribute('aria-controls', 'workspace-switcher-panel')
181185

182-
await user.click(screen.getByTestId('workspace-switcher-trigger'))
183-
expect(screen.getByTestId('workspace-switcher-panel')).toBeInTheDocument()
186+
await user.click(trigger)
187+
const panel = screen.getByTestId('workspace-switcher-panel')
188+
expect(panel).toHaveAttribute('id', 'workspace-switcher-panel')
189+
expect(panel).toHaveAttribute('role', 'menu')
190+
expect(trigger).toHaveAttribute('aria-expanded', 'true')
191+
192+
await user.click(trigger)
193+
expect(
194+
screen.queryByTestId('workspace-switcher-panel')
195+
).not.toBeInTheDocument()
196+
})
197+
198+
it('closes the workspace switcher panel on Escape', async () => {
199+
const user = userEvent.setup()
200+
renderComponent()
201+
const trigger = screen.getByTestId('workspace-switcher-trigger')
202+
203+
await user.click(trigger)
204+
await user.keyboard('{Escape}')
184205

185-
await user.click(screen.getByTestId('workspace-switcher-trigger'))
186206
expect(
187207
screen.queryByTestId('workspace-switcher-panel')
188208
).not.toBeInTheDocument()

src/platform/workspace/components/CurrentUserPopoverWorkspace.vue

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,17 @@
2626

2727
<!-- Workspace Selector -->
2828
<div v-if="!accountActionsOnly" class="relative">
29-
<div
29+
<button
3030
ref="workspaceSwitcherTrigger"
3131
v-tooltip="{ value: workspaceName, showDelay: 300 }"
32-
class="flex cursor-pointer items-center justify-between rounded-lg px-4 py-2 hover:bg-secondary-background-hover"
32+
type="button"
33+
class="flex w-full cursor-pointer items-center justify-between rounded-lg px-4 py-2 hover:bg-secondary-background-hover"
34+
:aria-expanded="isWorkspaceSwitcherOpen"
35+
aria-haspopup="menu"
36+
aria-controls="workspace-switcher-panel"
3337
data-testid="workspace-switcher-trigger"
3438
@click="toggleWorkspaceSwitcher"
39+
@keydown.escape.stop="isWorkspaceSwitcherOpen = false"
3540
>
3641
<div class="flex w-0 flex-1 items-center gap-2">
3742
<WorkspaceProfilePic
@@ -43,11 +48,13 @@
4348
</span>
4449
</div>
4550
<i class="pi pi-chevron-down shrink-0 text-sm text-muted-foreground" />
46-
</div>
51+
</button>
4752

4853
<div
4954
v-if="isWorkspaceSwitcherOpen"
55+
id="workspace-switcher-panel"
5056
ref="workspaceSwitcherPanel"
57+
role="menu"
5158
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)]"
5259
data-testid="workspace-switcher-panel"
5360
>

src/platform/workspace/stores/teamWorkspaceStore.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,19 @@ describe('useTeamWorkspaceStore', () => {
370370
expect(store.activeWorkspaceId).toBe(mockMemberWorkspace.id)
371371
})
372372

373+
it('clears cached billing rails when identity state is reset', async () => {
374+
const store = useTeamWorkspaceStore()
375+
await store.initialize()
376+
store.setWorkspaceBillingRail(mockPersonalWorkspace.id, 'legacy_stripe')
377+
expect(store.activeWorkspaceBillingRail).toBe('legacy_stripe')
378+
379+
store.resetForIdentityChange()
380+
await store.initialize()
381+
382+
expect(store.activeWorkspaceId).toBe(mockPersonalWorkspace.id)
383+
expect(store.activeWorkspaceBillingRail).toBeNull()
384+
})
385+
373386
it('does not let a previous user initialization overwrite the next user', async () => {
374387
let resolveFirstList: (value: unknown) => void = () => {}
375388
mockWorkspaceApi.list
@@ -475,6 +488,8 @@ describe('useTeamWorkspaceStore', () => {
475488
)
476489
const store = useTeamWorkspaceStore()
477490
await store.initialize()
491+
store.setWorkspaceBillingRail(mockPersonalWorkspace.id, 'legacy_stripe')
492+
expect(store.activeWorkspaceBillingRail).toBe('legacy_stripe')
478493

479494
await store.switchWorkspace(mockTeamWorkspace.id)
480495

@@ -488,6 +503,10 @@ describe('useTeamWorkspaceStore', () => {
488503
).not.toHaveBeenCalled()
489504
expect(mockReload).not.toHaveBeenCalled()
490505
expect(store.isSwitching).toBe(false)
506+
expect(store.activeWorkspaceBillingRail).toBeNull()
507+
508+
store.setWorkspaceBillingRail(mockTeamWorkspace.id, 'metronome')
509+
expect(store.activeWorkspaceBillingRail).toBe('metronome')
491510
})
492511

493512
it('rejects an overlapping local switch', async () => {

src/platform/workspace/stores/teamWorkspaceStore.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,7 @@ export const useTeamWorkspaceStore = defineStore('teamWorkspace', () => {
905905
initState.value = 'uninitialized'
906906
workspaces.value = []
907907
mutableActiveWorkspaceId.value = null
908+
billingRailByWorkspaceId.value = {}
908909
error.value = null
909910
isCreating.value = false
910911
isDeleting.value = false

src/stores/__tests__/authTokenPriority.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,22 @@ describe('auth token priority chain', () => {
323323
expect(mockUser.getIdToken).not.toHaveBeenCalled()
324324
expect(mockEnsureWorkspaceToken).not.toHaveBeenCalled()
325325
})
326+
327+
it('retries queue authentication after a previous initialization error', async () => {
328+
mockDistributionTypes.isCloud = false
329+
mockTeamWorkspaceInitState = 'error'
330+
mockInitializeWorkspaces.mockImplementation(async () => {
331+
mockActiveWorkspaceId = 'workspace-123'
332+
mockTeamWorkspaceInitState = 'ready'
333+
})
334+
mockEnsureWorkspaceToken.mockResolvedValue('workspace-raw-token')
335+
336+
await expect(store.getWorkspaceAuthToken()).resolves.toBe(
337+
'workspace-raw-token'
338+
)
339+
expect(mockInitializeWorkspaces).toHaveBeenCalledOnce()
340+
expect(mockEnsureWorkspaceToken).toHaveBeenCalledWith('workspace-123')
341+
})
326342
})
327343

328344
describe('unified login mint wiring', () => {

src/stores/authStore.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,8 @@ export const useAuthStore = defineStore('auth', () => {
337337
currentUser.value &&
338338
!teamWorkspaceStore.activeWorkspaceId &&
339339
(teamWorkspaceStore.initState === 'uninitialized' ||
340-
teamWorkspaceStore.initState === 'loading')
340+
teamWorkspaceStore.initState === 'loading' ||
341+
teamWorkspaceStore.initState === 'error')
341342
) {
342343
try {
343344
await teamWorkspaceStore.initialize()

0 commit comments

Comments
 (0)