Show confirm dialog on workflow path conflict (Save As) (#1590)

* Show confirm dialog on workflow path conflict (Save As)

* Fix closeworkflow

* nit

* Add playwright tests

* nit

* nit

* Move workflows dir cleanup
This commit is contained in:
Chenlei Hu
2024-11-18 23:07:24 -05:00
committed by GitHub
parent 0058691579
commit 6352cd86ee
5 changed files with 95 additions and 9 deletions

View File

@@ -737,6 +737,19 @@ export class ComfyPage {
) )
} }
async confirmDialog(prompt: string, text: string = 'Yes') {
const modal = this.page.locator(
`.comfy-modal-content:has-text("${prompt}")`
)
await expect(modal).toBeVisible()
await modal
.locator('.comfyui-button', {
hasText: text
})
.click()
await expect(modal).toBeHidden()
}
async convertAllNodesToGroupNode(groupNodeName: string) { async convertAllNodesToGroupNode(groupNodeName: string) {
this.page.on('dialog', async (dialog) => { this.page.on('dialog', async (dialog) => {
await dialog.accept(groupNodeName) await dialog.accept(groupNodeName)

View File

@@ -103,6 +103,12 @@ export class WorkflowsSidebarTab extends SidebarTab {
.allInnerTexts() .allInnerTexts()
} }
async getActiveWorkflowName() {
return await this.page
.locator('.comfyui-workflows-open .p-tree-node-selected .node-label')
.innerText()
}
async getTopLevelSavedWorkflowNames() { async getTopLevelSavedWorkflowNames() {
return await this.page return await this.page
.locator('.comfyui-workflows-browse .node-label') .locator('.comfyui-workflows-browse .node-label')

View File

@@ -379,7 +379,9 @@ test.describe('Menu', () => {
// Open the sidebar // Open the sidebar
const tab = comfyPage.menu.workflowsTab const tab = comfyPage.menu.workflowsTab
await tab.open() await tab.open()
})
test.afterEach(async ({ comfyPage }) => {
await comfyPage.setupWorkflowsDirectory({}) await comfyPage.setupWorkflowsDirectory({})
}) })
@@ -450,6 +452,43 @@ test.describe('Menu', () => {
).toEqual(['*Unsaved Workflow.json', 'workflow3.json', 'workflow4.json']) ).toEqual(['*Unsaved Workflow.json', 'workflow3.json', 'workflow4.json'])
}) })
test('Can save workflow as with same name', async ({ comfyPage }) => {
await comfyPage.menu.topbar.saveWorkflow('workflow5.json')
expect(
await comfyPage.menu.workflowsTab.getOpenedWorkflowNames()
).toEqual(['workflow5.json'])
await comfyPage.menu.topbar.saveWorkflowAs('workflow5.json')
await comfyPage.confirmDialog('Overwrite existing file?', 'Yes')
expect(
await comfyPage.menu.workflowsTab.getOpenedWorkflowNames()
).toEqual(['workflow5.json'])
})
test('Can overwrite other workflows with save as', async ({
comfyPage
}) => {
const topbar = comfyPage.menu.topbar
await topbar.saveWorkflow('workflow1.json')
await topbar.saveWorkflowAs('workflow2.json')
expect(
await comfyPage.menu.workflowsTab.getOpenedWorkflowNames()
).toEqual(['workflow1.json', 'workflow2.json'])
expect(await comfyPage.menu.workflowsTab.getActiveWorkflowName()).toEqual(
'workflow2.json'
)
await topbar.saveWorkflowAs('workflow1.json')
await comfyPage.confirmDialog('Overwrite existing file?', 'Yes')
// The old workflow1.json should be deleted and the new one should be saved.
expect(
await comfyPage.menu.workflowsTab.getOpenedWorkflowNames()
).toEqual(['workflow2.json', 'workflow1.json'])
expect(await comfyPage.menu.workflowsTab.getActiveWorkflowName()).toEqual(
'workflow1.json'
)
})
test('Does not report warning when switching between opened workflows', async ({ test('Does not report warning when switching between opened workflows', async ({
comfyPage comfyPage
}) => { }) => {
@@ -475,7 +514,7 @@ test.describe('Menu', () => {
`tempWorkflow-${test.info().title}` `tempWorkflow-${test.info().title}`
) )
const closeButton = comfyPage.page.locator( const closeButton = comfyPage.page.locator(
'.comfyui-workflows-open .p-button-icon.pi-times' '.comfyui-workflows-open .close-workflow-button'
) )
await closeButton.click() await closeButton.click()
expect( expect(

View File

@@ -54,6 +54,7 @@
</template> </template>
<template #actions="{ node }"> <template #actions="{ node }">
<Button <Button
class="close-workflow-button"
icon="pi pi-times" icon="pi pi-times"
text text
:severity=" :severity="

View File

@@ -63,17 +63,36 @@ export const workflowService = {
const newPath = workflow.directory + '/' + appendJsonExt(newFilename) const newPath = workflow.directory + '/' + appendJsonExt(newFilename)
const newKey = newPath.substring(ComfyWorkflow.basePath.length) const newKey = newPath.substring(ComfyWorkflow.basePath.length)
const workflowStore = useWorkflowStore()
const existingWorkflow = workflowStore.getWorkflowByPath(newPath)
if (existingWorkflow) {
const res = (await ComfyAsyncDialog.prompt({
title: 'Overwrite existing file?',
message: `"${newPath}" already exists. Do you want to overwrite it?`,
actions: ['Yes', 'No']
})) as 'Yes' | 'No'
if (res === 'No') return
if (existingWorkflow.path === workflow.path) {
await this.saveWorkflow(workflow)
return
}
const deleted = await this.deleteWorkflow(existingWorkflow)
if (!deleted) return
}
if (workflow.isTemporary) { if (workflow.isTemporary) {
await this.renameWorkflow(workflow, newPath) await this.renameWorkflow(workflow, newPath)
await useWorkflowStore().saveWorkflow(workflow) await workflowStore.saveWorkflow(workflow)
} else { } else {
const tempWorkflow = useWorkflowStore().createTemporary( const tempWorkflow = workflowStore.createTemporary(
newKey, newKey,
workflow.activeState as ComfyWorkflowJSON workflow.activeState as ComfyWorkflowJSON
) )
await this.openWorkflow(tempWorkflow) await this.openWorkflow(tempWorkflow)
await useWorkflowStore().saveWorkflow(tempWorkflow) await workflowStore.saveWorkflow(tempWorkflow)
} }
}, },
@@ -131,9 +150,9 @@ export const workflowService = {
async closeWorkflow( async closeWorkflow(
workflow: ComfyWorkflow, workflow: ComfyWorkflow,
options: { warnIfUnsaved: boolean } = { warnIfUnsaved: true } options: { warnIfUnsaved: boolean } = { warnIfUnsaved: true }
): Promise<void> { ): Promise<boolean> {
if (!workflow.isLoaded) { if (!workflow.isLoaded) {
return return true
} }
if (workflow.isModified && options.warnIfUnsaved) { if (workflow.isModified && options.warnIfUnsaved) {
@@ -146,7 +165,7 @@ export const workflowService = {
if (res === 'Yes') { if (res === 'Yes') {
await this.saveWorkflow(workflow) await this.saveWorkflow(workflow)
} else if (res === 'Cancel') { } else if (res === 'Cancel') {
return return false
} }
} }
@@ -161,18 +180,26 @@ export const workflowService = {
} }
await workflowStore.closeWorkflow(workflow) await workflowStore.closeWorkflow(workflow)
return true
}, },
async renameWorkflow(workflow: ComfyWorkflow, newPath: string) { async renameWorkflow(workflow: ComfyWorkflow, newPath: string) {
await useWorkflowStore().renameWorkflow(workflow, newPath) await useWorkflowStore().renameWorkflow(workflow, newPath)
}, },
async deleteWorkflow(workflow: ComfyWorkflow) { /**
* Delete a workflow
* @param workflow The workflow to delete
* @returns true if the workflow was deleted, false if the user cancelled
*/
async deleteWorkflow(workflow: ComfyWorkflow): Promise<boolean> {
const workflowStore = useWorkflowStore() const workflowStore = useWorkflowStore()
if (workflowStore.isOpen(workflow)) { if (workflowStore.isOpen(workflow)) {
await this.closeWorkflow(workflow) const closed = await this.closeWorkflow(workflow)
if (!closed) return false
} }
await workflowStore.deleteWorkflow(workflow) await workflowStore.deleteWorkflow(workflow)
return true
}, },
/** /**