Fix closing saved workflows (#1049)

* Fix workflow save. Resolves #996 Resolves #1048 * Add test on closing tabs * Add test on closing open workflows in sidebar
Comfy-Org · Sep 30, 2024 · 04e1344 · 04e1344
1 parent 0117964
commit 04e1344
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 0 deletions.
diff --git a/browser_tests/ComfyPage.ts b/browser_tests/ComfyPage.ts
@@ -230,6 +230,34 @@ class Topbar {
       .allInnerTexts()
   }
 
+  async getMenuItem(itemLabel: string): Promise<Locator> {
+    return this.page.locator(`.p-menubar-item-label:text-is("${itemLabel}")`)
+  }
+
+  async getWorkflowTab(tabName: string): Promise<Locator> {
+    return this.page
+      .locator(`.workflow-tabs .workflow-label:has-text("${tabName}")`)
+      .locator('..')
+  }
+
+  async closeWorkflowTab(tabName: string) {
+    const tab = await this.getWorkflowTab(tabName)
+    await tab.locator('.close-button').click({ force: true })
+  }
+
+  async saveWorkflow(workflowName: string) {
+    this.page.on('dialog', async (dialog) => {
+      await dialog.accept(workflowName)
+    })
+    const workflowMenuItem = await this.getMenuItem('Workflow')
+    workflowMenuItem.click()
+    await this.page.evaluate(() => {
+      return new Promise<number>(requestAnimationFrame)
+    })
+    const saveButton = await this.getMenuItem('Save')
+    await saveButton.click()
+  }
+
   async triggerTopbarCommand(path: string[]) {
     if (path.length < 2) {
       throw new Error('Path is too short')

diff --git a/browser_tests/menu.spec.ts b/browser_tests/menu.spec.ts
@@ -437,6 +437,19 @@ test.describe('Menu', () => {
         comfyPage.page.locator('.comfy-missing-nodes')
       ).not.toBeVisible()
     })
+
+    test('Can close saved-workflows from the open workflows section', async ({
+      comfyPage
+    }) => {
+      await comfyPage.menu.topbar.saveWorkflow('deault')
+      const closeButton = comfyPage.page.locator(
+        '.comfyui-workflows-open .p-button-icon.pi-times'
+      )
+      await closeButton.click()
+      expect(
+        await comfyPage.menu.workflowsTab.getOpenedWorkflowNames()
+      ).toEqual(['*Unsaved Workflow (2).json'])
+    })
   })
 
   test.describe('Workflows topbar tabs', () => {
@@ -447,11 +460,30 @@ test.describe('Menu', () => {
       )
     })
 
+    test.afterEach(async ({ comfyPage }) => {
+      // Delete the saved workflow for cleanup.
+      await comfyPage.page.evaluate(async () => {
+        window['app'].workflowManager.activeWorkflow.delete()
+      })
+    })
+
     test('Can show opened workflows', async ({ comfyPage }) => {
       expect(await comfyPage.menu.topbar.getTabNames()).toEqual([
         'Unsaved Workflow'
       ])
     })
+
+    test('Can close saved-workflow tabs', async ({ comfyPage }) => {
+      const savedWorkflowName = 'default'
+      await comfyPage.menu.topbar.saveWorkflow(savedWorkflowName)
+      expect(await comfyPage.menu.topbar.getTabNames()).toEqual([
+        savedWorkflowName
+      ])
+      await comfyPage.menu.topbar.closeWorkflowTab('default')
+      expect(await comfyPage.menu.topbar.getTabNames()).toEqual([
+        'Unsaved Workflow (2)'
+      ])
+    })
   })
 
   // Only test 'Top' to reduce test time.

diff --git a/src/scripts/workflows.ts b/src/scripts/workflows.ts
@@ -405,7 +405,13 @@ export class ComfyWorkflow {
 
     if (!this.path) {
       // Saved new workflow, patch this instance
+      const oldKey = this.key
       this.updatePath(path, null)
+
+      // Update workflowLookup: change the key from the old unsaved path to the new saved path
+      delete this.manager.workflowStore.workflowLookup[oldKey]
+      this.manager.workflowStore.workflowLookup[this.key] = this
+
       await this.manager.loadWorkflows()
       this.unsaved = false
       this.manager.dispatchEvent(new CustomEvent('rename', { detail: this }))