Skip to content

Commit

Permalink
Ignore missing dialog (#743)
Browse files Browse the repository at this point in the history
* Do not report missing nodes/models warning repeatedly

* Add playwright tests

* cast finalOptions, add comments to interface

* Use old menu in tests to not break top left click methods

* Assert no dialog on undo and on redo separately

* nit

* nit

---------

Co-authored-by: christian-byrne <abolkonsky.rem@gmail.com>
  • Loading branch information
huchenlei and christian-byrne authored Sep 5, 2024
1 parent 743683c commit 9d855d6
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 5 deletions.
7 changes: 7 additions & 0 deletions browser_tests/ComfyPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,13 @@ export class ComfyPage {
await this.nextFrame()
}

async ctrlY() {
await this.page.keyboard.down('Control')
await this.page.keyboard.press('KeyY')
await this.page.keyboard.up('Control')
await this.nextFrame()
}

async ctrlArrowUp() {
await this.page.keyboard.down('Control')
await this.page.keyboard.press('ArrowUp')
Expand Down
42 changes: 42 additions & 0 deletions browser_tests/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,48 @@ test.describe('Load workflow warning', () => {
})
})

test('Does not report warning when switching between opened workflows', async ({
comfyPage
}) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
await comfyPage.loadWorkflow('missing_nodes')
await comfyPage.page.locator('.p-dialog-close-button').click()

// Load default workflow
const workflowSelector = comfyPage.page.locator(
'button.comfyui-workflows-button'
)
await workflowSelector.hover()
await workflowSelector.click()
await comfyPage.page.locator('button[title="Load default workflow"]').click()

// Switch back to the missing_nodes workflow
await workflowSelector.click()
await comfyPage.page.locator('span:has-text("missing_nodes")').first().click()
await comfyPage.nextFrame()

await expect(comfyPage.page.locator('.comfy-missing-nodes')).not.toBeVisible()
await comfyPage.setSetting('Comfy.UseNewMenu', 'Disabled')
})

test('Does not report warning on undo/redo', async ({ comfyPage }) => {
await comfyPage.loadWorkflow('missing_nodes')
await comfyPage.page.locator('.p-dialog-close-button').click()
await comfyPage.nextFrame()

// Make a change to the graph
await comfyPage.setSetting('Comfy.NodeSearchBoxImpl', 'default')
await comfyPage.page.waitForTimeout(256)
await comfyPage.doubleClickCanvas()
await comfyPage.searchBox.fillAndSelectFirstNode('KSampler')

// Undo and redo the change
await comfyPage.ctrlZ()
await expect(comfyPage.page.locator('.comfy-missing-nodes')).not.toBeVisible()
await comfyPage.ctrlY()
await expect(comfyPage.page.locator('.comfy-missing-nodes')).not.toBeVisible()
})

test.describe('Execution error', () => {
test('Should display an error message when an execution error occurs', async ({
comfyPage
Expand Down
7 changes: 4 additions & 3 deletions src/scripts/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2206,7 +2206,8 @@ export class ComfyApp {
graphData?: ComfyWorkflowJSON,
clean: boolean = true,
restore_view: boolean = true,
workflow: string | null | ComfyWorkflow = null
workflow: string | null | ComfyWorkflow = null,
{ showMissingNodesDialog = true, showMissingModelsDialog = true } = {}
) {
if (clean !== false) {
this.clean()
Expand Down Expand Up @@ -2398,10 +2399,10 @@ export class ComfyApp {
}

// TODO: Properly handle if both nodes and models are missing (sequential dialogs?)
if (missingNodeTypes.length) {
if (missingNodeTypes.length && showMissingNodesDialog) {
this.showMissingNodesError(missingNodeTypes)
}
if (missingModels.length) {
if (missingModels.length && showMissingModelsDialog) {
this.showMissingModelsError(missingModels)
}
await this.#invokeExtensionsAsync('afterConfigureGraph', missingNodeTypes)
Expand Down
5 changes: 4 additions & 1 deletion src/scripts/changeTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ export class ChangeTracker {
if (prevState) {
target.push(this.activeState)
this.isOurLoad = true
await this.app.loadGraphData(prevState, false, false, this.workflow)
await this.app.loadGraphData(prevState, false, false, this.workflow, {
showMissingModelsDialog: false,
showMissingNodesDialog: false
})
this.activeState = prevState
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/scripts/workflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,11 @@ export class ComfyWorkflow {
this.changeTracker.activeState,
true,
true,
this
this,
{
showMissingModelsDialog: false,
showMissingNodesDialog: false
}
)
} else {
const data = await this.getWorkflowData()
Expand Down

0 comments on commit 9d855d6

Please sign in to comment.