mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-13 09:00:16 +00:00
fix: remove workspace switching confirmation dialog (#9250)
## Summary Remove the workspace switching confirmation dialog since switching workspaces no longer discards unsaved changes. ## Changes - **What**: Remove `hasUnsavedChanges` check, `dialogService.confirm` call, and unused imports (`useI18n`, `useWorkflowStore`, `useDialogService`) from `useWorkspaceSwitch`. Rename `switchWithConfirmation` to `switchWorkspace`. Update callers (`WorkspaceSwitcherPopover.vue`, `InviteAcceptedToast.vue`). Remove `workspace.unsavedChanges` i18n entries from all 12 locale files. Simplify tests to cover core switching behavior only. ## Review Focus The confirmation dialog was showing inaccurate information (warning about discarding unsaved changes when that no longer happens). This is a pure removal with no new behavior. <!-- Pipeline-Ticket: COM-15441 --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9250-fix-remove-workspace-switching-confirmation-dialog-3136d73d365081d3b959da22e8f151d1) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -139,7 +139,7 @@ const emit = defineEmits<{
|
||||
}>()
|
||||
|
||||
const { t } = useI18n()
|
||||
const { switchWithConfirmation } = useWorkspaceSwitch()
|
||||
const { switchWorkspace } = useWorkspaceSwitch()
|
||||
const { subscription } = useBillingContext()
|
||||
|
||||
const tierKeyMap: Record<string, string> = {
|
||||
@@ -226,7 +226,7 @@ function getTierLabel(workspace: AvailableWorkspace): string | null {
|
||||
}
|
||||
|
||||
async function handleSelectWorkspace(workspace: AvailableWorkspace) {
|
||||
const success = await switchWithConfirmation(workspace.id)
|
||||
const success = await switchWorkspace(workspace.id)
|
||||
if (success) {
|
||||
emit('select', workspace)
|
||||
}
|
||||
|
||||
@@ -33,10 +33,18 @@ import { useWorkspaceSwitch } from '@/platform/workspace/composables/useWorkspac
|
||||
|
||||
const { t } = useI18n()
|
||||
const toast = useToast()
|
||||
const { switchWithConfirmation } = useWorkspaceSwitch()
|
||||
const { switchWorkspace } = useWorkspaceSwitch()
|
||||
|
||||
function viewWorkspace(workspaceId: string) {
|
||||
void switchWithConfirmation(workspaceId)
|
||||
toast.removeGroup('invite-accepted')
|
||||
async function viewWorkspace(workspaceId: string) {
|
||||
const success = await switchWorkspace(workspaceId)
|
||||
if (success) {
|
||||
toast.removeGroup('invite-accepted')
|
||||
} else {
|
||||
toast.add({
|
||||
severity: 'error',
|
||||
summary: t('workspace.switchFailed'),
|
||||
life: 5000
|
||||
})
|
||||
}
|
||||
}
|
||||
</script>
|
||||
|
||||
@@ -20,32 +20,6 @@ vi.mock('pinia', () => ({
|
||||
})
|
||||
}))
|
||||
|
||||
const mockModifiedWorkflows = vi.hoisted(
|
||||
() => [] as Array<{ isModified: boolean }>
|
||||
)
|
||||
|
||||
vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({
|
||||
useWorkflowStore: () => ({
|
||||
get modifiedWorkflows() {
|
||||
return mockModifiedWorkflows
|
||||
}
|
||||
})
|
||||
}))
|
||||
|
||||
const mockConfirm = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('@/services/dialogService', () => ({
|
||||
useDialogService: () => ({
|
||||
confirm: mockConfirm
|
||||
})
|
||||
}))
|
||||
|
||||
vi.mock('vue-i18n', () => ({
|
||||
useI18n: () => ({
|
||||
t: (key: string) => key
|
||||
})
|
||||
}))
|
||||
|
||||
describe('useWorkspaceSwitch', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
@@ -57,103 +31,37 @@ describe('useWorkspaceSwitch', () => {
|
||||
created_at: '2026-01-01T00:00:00Z',
|
||||
joined_at: '2026-01-01T00:00:00Z'
|
||||
}
|
||||
mockModifiedWorkflows.length = 0
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
describe('hasUnsavedChanges', () => {
|
||||
it('returns true when there are modified workflows', () => {
|
||||
mockModifiedWorkflows.push({ isModified: true })
|
||||
const { hasUnsavedChanges } = useWorkspaceSwitch()
|
||||
|
||||
expect(hasUnsavedChanges()).toBe(true)
|
||||
})
|
||||
|
||||
it('returns true when multiple workflows are modified', () => {
|
||||
mockModifiedWorkflows.push({ isModified: true }, { isModified: true })
|
||||
const { hasUnsavedChanges } = useWorkspaceSwitch()
|
||||
|
||||
expect(hasUnsavedChanges()).toBe(true)
|
||||
})
|
||||
|
||||
it('returns false when no workflows are modified', () => {
|
||||
mockModifiedWorkflows.length = 0
|
||||
const { hasUnsavedChanges } = useWorkspaceSwitch()
|
||||
|
||||
expect(hasUnsavedChanges()).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('switchWithConfirmation', () => {
|
||||
describe('switchWorkspace', () => {
|
||||
it('returns true immediately if switching to the same workspace', async () => {
|
||||
const { switchWithConfirmation } = useWorkspaceSwitch()
|
||||
const { switchWorkspace } = useWorkspaceSwitch()
|
||||
|
||||
const result = await switchWithConfirmation('workspace-1')
|
||||
const result = await switchWorkspace('workspace-1')
|
||||
|
||||
expect(result).toBe(true)
|
||||
expect(mockSwitchWorkspace).not.toHaveBeenCalled()
|
||||
expect(mockConfirm).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('switches directly without dialog when no unsaved changes', async () => {
|
||||
mockModifiedWorkflows.length = 0
|
||||
it('switches directly to the new workspace', async () => {
|
||||
mockSwitchWorkspace.mockResolvedValue(undefined)
|
||||
const { switchWithConfirmation } = useWorkspaceSwitch()
|
||||
const { switchWorkspace } = useWorkspaceSwitch()
|
||||
|
||||
const result = await switchWithConfirmation('workspace-2')
|
||||
|
||||
expect(result).toBe(true)
|
||||
expect(mockConfirm).not.toHaveBeenCalled()
|
||||
expect(mockSwitchWorkspace).toHaveBeenCalledWith('workspace-2')
|
||||
})
|
||||
|
||||
it('shows confirmation dialog when there are unsaved changes', async () => {
|
||||
mockModifiedWorkflows.push({ isModified: true })
|
||||
mockConfirm.mockResolvedValue(true)
|
||||
mockSwitchWorkspace.mockResolvedValue(undefined)
|
||||
const { switchWithConfirmation } = useWorkspaceSwitch()
|
||||
|
||||
await switchWithConfirmation('workspace-2')
|
||||
|
||||
expect(mockConfirm).toHaveBeenCalledWith({
|
||||
title: 'workspace.unsavedChanges.title',
|
||||
message: 'workspace.unsavedChanges.message',
|
||||
type: 'dirtyClose'
|
||||
})
|
||||
})
|
||||
|
||||
it('returns false if user cancels the confirmation dialog', async () => {
|
||||
mockModifiedWorkflows.push({ isModified: true })
|
||||
mockConfirm.mockResolvedValue(false)
|
||||
const { switchWithConfirmation } = useWorkspaceSwitch()
|
||||
|
||||
const result = await switchWithConfirmation('workspace-2')
|
||||
|
||||
expect(result).toBe(false)
|
||||
expect(mockSwitchWorkspace).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('calls switchWorkspace after user confirms', async () => {
|
||||
mockModifiedWorkflows.push({ isModified: true })
|
||||
mockConfirm.mockResolvedValue(true)
|
||||
mockSwitchWorkspace.mockResolvedValue(undefined)
|
||||
const { switchWithConfirmation } = useWorkspaceSwitch()
|
||||
|
||||
const result = await switchWithConfirmation('workspace-2')
|
||||
const result = await switchWorkspace('workspace-2')
|
||||
|
||||
expect(result).toBe(true)
|
||||
expect(mockSwitchWorkspace).toHaveBeenCalledWith('workspace-2')
|
||||
})
|
||||
|
||||
it('returns false if switchWorkspace throws an error', async () => {
|
||||
mockModifiedWorkflows.length = 0
|
||||
mockSwitchWorkspace.mockRejectedValue(new Error('Switch failed'))
|
||||
const { switchWithConfirmation } = useWorkspaceSwitch()
|
||||
const { switchWorkspace } = useWorkspaceSwitch()
|
||||
|
||||
const result = await switchWithConfirmation('workspace-2')
|
||||
const result = await switchWorkspace('workspace-2')
|
||||
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
@@ -1,41 +1,18 @@
|
||||
import { storeToRefs } from 'pinia'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
|
||||
import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore'
|
||||
import { useTeamWorkspaceStore } from '@/platform/workspace/stores/teamWorkspaceStore'
|
||||
import { useDialogService } from '@/services/dialogService'
|
||||
|
||||
export function useWorkspaceSwitch() {
|
||||
const { t } = useI18n()
|
||||
const workspaceStore = useTeamWorkspaceStore()
|
||||
const { activeWorkspace } = storeToRefs(workspaceStore)
|
||||
const workflowStore = useWorkflowStore()
|
||||
const dialogService = useDialogService()
|
||||
|
||||
function hasUnsavedChanges(): boolean {
|
||||
return workflowStore.modifiedWorkflows.length > 0
|
||||
}
|
||||
|
||||
async function switchWithConfirmation(workspaceId: string): Promise<boolean> {
|
||||
async function switchWorkspace(workspaceId: string): Promise<boolean> {
|
||||
if (activeWorkspace.value?.id === workspaceId) {
|
||||
return true
|
||||
}
|
||||
|
||||
if (hasUnsavedChanges()) {
|
||||
const confirmed = await dialogService.confirm({
|
||||
title: t('workspace.unsavedChanges.title'),
|
||||
message: t('workspace.unsavedChanges.message'),
|
||||
type: 'dirtyClose'
|
||||
})
|
||||
|
||||
if (!confirmed) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
await workspaceStore.switchWorkspace(workspaceId)
|
||||
// Note: switchWorkspace triggers page reload internally
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
@@ -43,7 +20,6 @@ export function useWorkspaceSwitch() {
|
||||
}
|
||||
|
||||
return {
|
||||
hasUnsavedChanges,
|
||||
switchWithConfirmation
|
||||
switchWorkspace
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user