Skip to content

Commit

Permalink
Fix group node bookmarking (#950)
Browse files Browse the repository at this point in the history 
* Resolves #926 group node bookmark

* Remove expect outside scope of test

* Update unit tests

* Update group node manager path separators

* Update group node path sepator in fixture
  • Loading branch information
@christian-byrne
christian-byrne committed Sep 24, 2024
1 parent b21c0f5 commit 6b9c1b7
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 32 deletions.
3 changes: 1 addition & 2 deletions browser_tests/ComfyPage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,6 @@ export class ComfyPage {
})
await this.canvas.press('Control+a')
const node = await this.getFirstNodeRef()
expect(node).not.toBeNull()
await node!.clickContextMenuOption('Convert to Group Node')
await this.nextFrame()
}
Expand Down Expand Up @@ -874,7 +873,7 @@ class NodeReference {
await this.clickContextMenuOption('Convert to Group Node')
await this.comfyPage.nextFrame()
const nodes = await this.comfyPage.getNodeRefsByType(
`workflow/${groupNodeName}`
`workflow>${groupNodeName}`
)
if (nodes.length !== 1) {
throw new Error(`Did not find single group node (found=${nodes.length})`)
Expand Down
72 changes: 59 additions & 13 deletions browser_tests/groupNode.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,82 @@ test.describe('Group Node', () => {
})

test.describe('Node library sidebar', () => {
const groupNodeName = 'DefautWorkflowGroupNode'
const groupNodeCategory = 'group nodes>workflow'
const groupNodeBookmarkName = `workflow>${groupNodeName}`
let libraryTab

test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
libraryTab = comfyPage.menu.nodeLibraryTab
await comfyPage.convertAllNodesToGroupNode(groupNodeName)
await libraryTab.open()
})

test('Is added to node library sidebar', async ({ comfyPage }) => {
const groupNodeName = 'DefautWorkflowGroupNode'
await comfyPage.convertAllNodesToGroupNode(groupNodeName)
const tab = comfyPage.menu.nodeLibraryTab
await tab.open()
expect(await tab.getFolder('group nodes').count()).toBe(1)
expect(await libraryTab.getFolder('group nodes').count()).toBe(1)
})

test('Can be added to canvas using node library sidebar', async ({
comfyPage
}) => {
const groupNodeName = 'DefautWorkflowGroupNode'
await comfyPage.convertAllNodesToGroupNode(groupNodeName)
const initialNodeCount = await comfyPage.getGraphNodesCount()

// Add group node from node library sidebar
const tab = comfyPage.menu.nodeLibraryTab
await tab.open()
await tab.getFolder('group nodes').click()
await tab.getFolder('workflow').click()
await tab.getFolder('workflow').last().click()
await tab.getNode(groupNodeName).click()
await libraryTab.getFolder(groupNodeCategory).first().click()
await libraryTab.getNode(groupNodeName).first().click()

// Verify the node is added to the canvas
expect(await comfyPage.getGraphNodesCount()).toBe(initialNodeCount + 1)
})

test('Can be bookmarked and unbookmarked', async ({ comfyPage }) => {
await libraryTab.getFolder(groupNodeCategory).click()
await libraryTab
.getNode(groupNodeName)
.locator('.bookmark-button')
.first()
.click()

// Verify the node is added to the bookmarks tab
expect(
await comfyPage.getSetting('Comfy.NodeLibrary.Bookmarks.V2')
).toEqual([groupNodeBookmarkName])
// Verify the bookmark node with the same name is added to the tree
expect(await libraryTab.getNode(groupNodeName).count()).not.toBe(0)

// Unbookmark the node
await libraryTab
.getNode(groupNodeName)
.locator('.bookmark-button')
.first()
.click()

// Verify the node is removed from the bookmarks tab
expect(
await comfyPage.getSetting('Comfy.NodeLibrary.Bookmarks.V2')
).toHaveLength(0)
await comfyPage.setSetting('Comfy.NodeLibrary.Bookmarks.V2', [])
})

test('Displays preview on bookmark hover', async ({ comfyPage }) => {
await libraryTab.getFolder(groupNodeCategory).click()
await libraryTab
.getNode(groupNodeName)
.locator('.bookmark-button')
.first()
.click()
await comfyPage.page.hover('.p-tree-node-label.tree-explorer-node-label')
expect(await comfyPage.page.isVisible('.node-lib-node-preview')).toBe(
true
)
await libraryTab
.getNode(groupNodeName)
.locator('.bookmark-button')
.first()
.click()
await comfyPage.setSetting('Comfy.NodeLibrary.Bookmarks.V2', [])
})
})

test('Can be added to canvas using search', async ({ comfyPage }) => {
Expand Down
10 changes: 5 additions & 5 deletions src/extensions/core/groupNode.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const Workflow = {
InWorkflow: 2
},
isInUseGroupNode(name) {
const id = `workflow/${name}`
const id = `workflow>${name}`
// Check if lready registered/in use in this workflow
if (app.graph.extra?.groupNodes?.[name]) {
if (app.graph.nodes.find((n) => n.type === id)) {
Expand Down Expand Up @@ -191,9 +191,9 @@ export class GroupNodeConfig {
output_name: [],
output_is_list: [],
output_is_hidden: [],
name: source + '/' + this.name,
name: source + '>' + this.name,
display_name: this.name,
category: 'group nodes' + ('/' + source),
category: 'group nodes' + ('>' + source),
input: { required: {} },
description: `Group node combining ${this.nodeData.nodes
.map((n) => n.type)
Expand All @@ -216,7 +216,7 @@ export class GroupNodeConfig {
p()
}
this.#convertedToProcess = null
await app.registerNodeDef('workflow/' + this.name, this.nodeDef)
await app.registerNodeDef('workflow>' + this.name, this.nodeDef)
useNodeDefStore().addNodeDef(this.nodeDef)
}

Expand Down Expand Up @@ -1380,7 +1380,7 @@ export class GroupNodeHandler {
const config = new GroupNodeConfig(name, nodeData)
await config.registerType()

const groupNode = LiteGraph.createNode(`workflow/${name}`)
const groupNode = LiteGraph.createNode(`workflow>${name}`)
// Reuse the existing nodes for this instance
groupNode.setInnerNodes(builder.nodes)
groupNode[GROUP].populateWidgets()
Expand Down
12 changes: 6 additions & 6 deletions src/extensions/core/groupNodeManage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {

getGroupData() {
this.groupNodeType =
LiteGraph.registered_node_types['workflow/' + this.selectedGroup]
LiteGraph.registered_node_types['workflow>' + this.selectedGroup]
this.groupNodeDef = this.groupNodeType.nodeData
this.groupData = GroupNodeHandler.getGroupData(this.groupNodeType)
}
Expand Down Expand Up @@ -367,7 +367,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {
groupNodes.map((g) =>
$el('option', {
textContent: g,
selected: 'workflow/' + g === type,
selected: 'workflow>' + g === type,
value: g
})
)
Expand All @@ -389,7 +389,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {
{
onclick: (e) => {
const node = app.graph.nodes.find(
(n) => n.type === 'workflow/' + this.selectedGroup
(n) => n.type === 'workflow>' + this.selectedGroup
)
if (node) {
alert(
Expand All @@ -403,7 +403,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {
)
) {
delete app.graph.extra.groupNodes[this.selectedGroup]
LiteGraph.unregisterNodeType('workflow/' + this.selectedGroup)
LiteGraph.unregisterNodeType('workflow>' + this.selectedGroup)
}
this.show()
}
Expand Down Expand Up @@ -476,7 +476,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {
}, {})
}

const nodes = nodesByType['workflow/' + g]
const nodes = nodesByType['workflow>' + g]
if (nodes) recreateNodes.push(...nodes)
}

Expand All @@ -503,7 +503,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {

this.element.replaceChildren(outer)
this.changeGroup(
type ? groupNodes.find((g) => 'workflow/' + g === type) : groupNodes[0]
type ? groupNodes.find((g) => 'workflow>' + g === type) : groupNodes[0]
)
this.element.showModal()

Expand Down
12 changes: 6 additions & 6 deletions tests-ui/tests/groupNode.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('group node', () => {
expect(n.isRemoved).toBeTruthy()
}

expect(groupNode.type).toEqual('workflow/' + name)
expect(groupNode.type).toEqual('workflow>' + name)

return graph.find(groupNode)
}
Expand Down Expand Up @@ -520,7 +520,7 @@ describe('group node', () => {
group1.menu.Clone.call()
expect(app.graph.nodes).toHaveLength(4)
const group2 = graph.find(app.graph.nodes[3])
expect(group2.node.type).toEqual('workflow/test')
expect(group2.node.type).toEqual('workflow>test')
expect(group2.id).not.toEqual(group1.id)

group1.outputs.VAE.connectTo(group2.inputs.VAE)
Expand Down Expand Up @@ -681,7 +681,7 @@ describe('group node', () => {
group1.menu.Clone.call()
expect(app.graph.nodes).toHaveLength(3)
const group2 = graph.find(app.graph.nodes[2])
expect(group2.node.type).toEqual('workflow/test')
expect(group2.node.type).toEqual('workflow>test')
expect(group2.id).not.toEqual(group1.id)

// Reconnect ckpt
Expand Down Expand Up @@ -741,7 +741,7 @@ describe('group node', () => {
resetEnv: true
}))
// Ensure the node isnt registered
expect(() => ez['workflow/test']).toThrow()
expect(() => ez['workflow>test']).toThrow()

// Reload the workflow
await app.loadGraphData(JSON.parse(workflow))
Expand All @@ -768,7 +768,7 @@ describe('group node', () => {
nodes: [
{
id: 3,
type: 'workflow/testerror'
type: 'workflow>testerror'
}
],
links: [],
Expand Down Expand Up @@ -796,7 +796,7 @@ describe('group node', () => {
expect(call).toContain('the following node types were not found')
expect(call).toContain('NotKSampler')
expect(call).toContain('NotVAEDecode')
expect(call).toContain('workflow/testerror')
expect(call).toContain('workflow>testerror')
})
test('maintains widget inputs on conversion back to nodes', async () => {
const { ez, graph, app } = await start()
Expand Down

0 comments on commit 6b9c1b7

Please sign in to comment.