From 069dc67c30a88f0a5e2230c1b611938de5b89fd0 Mon Sep 17 00:00:00 2001 From: Chenlei Hu Date: Sat, 23 Nov 2024 06:49:12 -0800 Subject: [PATCH] Reland "Fix undo / redo filling with empty steps" (#1653) * Revert "Revert "Fix undo / redo filling with empty steps (#1649)" (#1652)" This reverts commit 76238101666597ca589585d08253e71f6fcf629e. * Update test expectations * Add dirty flag if workflow is not persisted * Add dirty flag to other UI areas for new workflows * Remove redundant code * Fix regression: undo / redo steps lost on refresh The history is still be cleared, but any changes made by issuing undo / redo comands prior to refresh are not lost. * Update test expectations Partially reverts f8cc2c0d677f55e81f21d0da9f45e773739a9745 - adds dirty flags back to unsaved workflows. --------- Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com> --- browser_tests/browserTabTitle.spec.ts | 2 +- browser_tests/changeTracker.spec.ts | 12 ++++----- browser_tests/menu.spec.ts | 5 ++-- src/components/BrowserTabTitle.vue | 5 +++- .../sidebar/tabs/WorkflowsSidebarTab.vue | 4 ++- src/components/topbar/WorkflowTabs.vue | 5 +++- src/scripts/changeTracker.ts | 27 +++++++------------ 7 files changed, 30 insertions(+), 30 deletions(-) diff --git a/browser_tests/browserTabTitle.spec.ts b/browser_tests/browserTabTitle.spec.ts index 43439bbd9e..068963eab8 100644 --- a/browser_tests/browserTabTitle.spec.ts +++ b/browser_tests/browserTabTitle.spec.ts @@ -11,7 +11,7 @@ test.describe('Browser tab title', () => { const workflowName = await comfyPage.page.evaluate(async () => { return window['app'].extensionManager.workflow.activeWorkflow.filename }) - expect(await comfyPage.page.title()).toBe(`${workflowName} - ComfyUI`) + expect(await comfyPage.page.title()).toBe(`*${workflowName} - ComfyUI`) }) // Failing on CI diff --git a/browser_tests/changeTracker.spec.ts b/browser_tests/changeTracker.spec.ts index a76c5f6c95..188526540a 100644 --- a/browser_tests/changeTracker.spec.ts +++ b/browser_tests/changeTracker.spec.ts @@ -56,9 +56,7 @@ test.describe('Change Tracker', () => { expect(await comfyPage.getToastErrorCount()).toBe(0) expect(await isModified()).toBe(false) - // TODO(huchenlei): Investigate why saving the workflow is causing the - // undo queue to be triggered. - expect(await getUndoQueueSize()).toBe(1) + expect(await getUndoQueueSize()).toBe(0) expect(await getRedoQueueSize()).toBe(0) const node = (await comfyPage.getFirstNodeRef())! @@ -66,25 +64,25 @@ test.describe('Change Tracker', () => { await node.click('collapse') await expect(node).toBeCollapsed() expect(await isModified()).toBe(true) - expect(await getUndoQueueSize()).toBe(2) + expect(await getUndoQueueSize()).toBe(1) expect(await getRedoQueueSize()).toBe(0) await comfyPage.ctrlB() await expect(node).toBeBypassed() expect(await isModified()).toBe(true) - expect(await getUndoQueueSize()).toBe(3) + expect(await getUndoQueueSize()).toBe(2) expect(await getRedoQueueSize()).toBe(0) await comfyPage.ctrlZ() await expect(node).not.toBeBypassed() expect(await isModified()).toBe(true) - expect(await getUndoQueueSize()).toBe(2) + expect(await getUndoQueueSize()).toBe(1) expect(await getRedoQueueSize()).toBe(1) await comfyPage.ctrlZ() await expect(node).not.toBeCollapsed() expect(await isModified()).toBe(false) - expect(await getUndoQueueSize()).toBe(1) + expect(await getUndoQueueSize()).toBe(0) expect(await getRedoQueueSize()).toBe(2) }) }) diff --git a/browser_tests/menu.spec.ts b/browser_tests/menu.spec.ts index 3b956a04fe..7c6edda155 100644 --- a/browser_tests/menu.spec.ts +++ b/browser_tests/menu.spec.ts @@ -394,7 +394,7 @@ test.describe('Menu', () => { await tab.newBlankWorkflowButton.click() expect(await tab.getOpenedWorkflowNames()).toEqual([ '*Unsaved Workflow.json', - 'Unsaved Workflow (2).json' + '*Unsaved Workflow (2).json' ]) }) @@ -471,6 +471,7 @@ test.describe('Menu', () => { const topbar = comfyPage.menu.topbar await topbar.saveWorkflow('workflow1.json') await topbar.saveWorkflowAs('workflow2.json') + await comfyPage.nextFrame() expect( await comfyPage.menu.workflowsTab.getOpenedWorkflowNames() ).toEqual(['workflow1.json', 'workflow2.json']) @@ -519,7 +520,7 @@ test.describe('Menu', () => { await closeButton.click() expect( await comfyPage.menu.workflowsTab.getOpenedWorkflowNames() - ).toEqual(['Unsaved Workflow.json']) + ).toEqual(['*Unsaved Workflow.json']) }) }) diff --git a/src/components/BrowserTabTitle.vue b/src/components/BrowserTabTitle.vue index 33647e2f68..2a651fcbb7 100644 --- a/src/components/BrowserTabTitle.vue +++ b/src/components/BrowserTabTitle.vue @@ -26,7 +26,10 @@ const betaMenuEnabled = computed( const workflowStore = useWorkflowStore() const isUnsavedText = computed(() => - workflowStore.activeWorkflow?.isModified ? ' *' : '' + workflowStore.activeWorkflow?.isModified || + !workflowStore.activeWorkflow?.isPersisted + ? ' *' + : '' ) const workflowNameText = computed(() => { const workflowName = workflowStore.activeWorkflow?.filename diff --git a/src/components/sidebar/tabs/WorkflowsSidebarTab.vue b/src/components/sidebar/tabs/WorkflowsSidebarTab.vue index 3f4ad70489..72e90e1401 100644 --- a/src/components/sidebar/tabs/WorkflowsSidebarTab.vue +++ b/src/components/sidebar/tabs/WorkflowsSidebarTab.vue @@ -50,7 +50,9 @@