Skip to content

Commit 6b9c1b7

Browse files
christian-byrnechristian-byrne
authored
Fix group node bookmarking (#950)
* 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
1 parent b21c0f5 commit 6b9c1b7

File tree

5 files changed

+77
-32
lines changed

5 files changed

+77
-32
lines changed

‎browser_tests/ComfyPage.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,6 @@ export class ComfyPage {
707707
})
708708
await this.canvas.press('Control+a')
709709
const node = await this.getFirstNodeRef()
710-
expect(node).not.toBeNull()
711710
await node!.clickContextMenuOption('Convert to Group Node')
712711
await this.nextFrame()
713712
}
@@ -874,7 +873,7 @@ class NodeReference {
874873
await this.clickContextMenuOption('Convert to Group Node')
875874
await this.comfyPage.nextFrame()
876875
const nodes = await this.comfyPage.getNodeRefsByType(
877-
`workflow/${groupNodeName}`
876+
`workflow>${groupNodeName}`
878877
)
879878
if (nodes.length !== 1) {
880879
throw new Error(`Did not find single group node (found=${nodes.length})`)

‎browser_tests/groupNode.spec.ts

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,36 +7,82 @@ test.describe('Group Node', () => {
77
})
88

99
test.describe('Node library sidebar', () => {
10+
const groupNodeName = 'DefautWorkflowGroupNode'
11+
const groupNodeCategory = 'group nodes>workflow'
12+
const groupNodeBookmarkName = `workflow>${groupNodeName}`
13+
let libraryTab
14+
1015
test.beforeEach(async ({ comfyPage }) => {
1116
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
17+
libraryTab = comfyPage.menu.nodeLibraryTab
18+
await comfyPage.convertAllNodesToGroupNode(groupNodeName)
19+
await libraryTab.open()
1220
})
1321

1422
test('Is added to node library sidebar', async ({ comfyPage }) => {
15-
const groupNodeName = 'DefautWorkflowGroupNode'
16-
await comfyPage.convertAllNodesToGroupNode(groupNodeName)
17-
const tab = comfyPage.menu.nodeLibraryTab
18-
await tab.open()
19-
expect(await tab.getFolder('group nodes').count()).toBe(1)
23+
expect(await libraryTab.getFolder('group nodes').count()).toBe(1)
2024
})
2125

2226
test('Can be added to canvas using node library sidebar', async ({
2327
comfyPage
2428
}) => {
25-
const groupNodeName = 'DefautWorkflowGroupNode'
26-
await comfyPage.convertAllNodesToGroupNode(groupNodeName)
2729
const initialNodeCount = await comfyPage.getGraphNodesCount()
2830

2931
// Add group node from node library sidebar
30-
const tab = comfyPage.menu.nodeLibraryTab
31-
await tab.open()
32-
await tab.getFolder('group nodes').click()
33-
await tab.getFolder('workflow').click()
34-
await tab.getFolder('workflow').last().click()
35-
await tab.getNode(groupNodeName).click()
32+
await libraryTab.getFolder(groupNodeCategory).first().click()
33+
await libraryTab.getNode(groupNodeName).first().click()
3634

3735
// Verify the node is added to the canvas
3836
expect(await comfyPage.getGraphNodesCount()).toBe(initialNodeCount + 1)
3937
})
38+
39+
test('Can be bookmarked and unbookmarked', async ({ comfyPage }) => {
40+
await libraryTab.getFolder(groupNodeCategory).click()
41+
await libraryTab
42+
.getNode(groupNodeName)
43+
.locator('.bookmark-button')
44+
.first()
45+
.click()
46+
47+
// Verify the node is added to the bookmarks tab
48+
expect(
49+
await comfyPage.getSetting('Comfy.NodeLibrary.Bookmarks.V2')
50+
).toEqual([groupNodeBookmarkName])
51+
// Verify the bookmark node with the same name is added to the tree
52+
expect(await libraryTab.getNode(groupNodeName).count()).not.toBe(0)
53+
54+
// Unbookmark the node
55+
await libraryTab
56+
.getNode(groupNodeName)
57+
.locator('.bookmark-button')
58+
.first()
59+
.click()
60+
61+
// Verify the node is removed from the bookmarks tab
62+
expect(
63+
await comfyPage.getSetting('Comfy.NodeLibrary.Bookmarks.V2')
64+
).toHaveLength(0)
65+
await comfyPage.setSetting('Comfy.NodeLibrary.Bookmarks.V2', [])
66+
})
67+
68+
test('Displays preview on bookmark hover', async ({ comfyPage }) => {
69+
await libraryTab.getFolder(groupNodeCategory).click()
70+
await libraryTab
71+
.getNode(groupNodeName)
72+
.locator('.bookmark-button')
73+
.first()
74+
.click()
75+
await comfyPage.page.hover('.p-tree-node-label.tree-explorer-node-label')
76+
expect(await comfyPage.page.isVisible('.node-lib-node-preview')).toBe(
77+
true
78+
)
79+
await libraryTab
80+
.getNode(groupNodeName)
81+
.locator('.bookmark-button')
82+
.first()
83+
.click()
84+
await comfyPage.setSetting('Comfy.NodeLibrary.Bookmarks.V2', [])
85+
})
4086
})
4187

4288
test('Can be added to canvas using search', async ({ comfyPage }) => {

‎src/extensions/core/groupNode.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const Workflow = {
1515
InWorkflow: 2
1616
},
1717
isInUseGroupNode(name) {
18-
const id = `workflow/${name}`
18+
const id = `workflow>${name}`
1919
// Check if lready registered/in use in this workflow
2020
if (app.graph.extra?.groupNodes?.[name]) {
2121
if (app.graph.nodes.find((n) => n.type === id)) {
@@ -191,9 +191,9 @@ export class GroupNodeConfig {
191191
output_name: [],
192192
output_is_list: [],
193193
output_is_hidden: [],
194-
name: source + '/' + this.name,
194+
name: source + '>' + this.name,
195195
display_name: this.name,
196-
category: 'group nodes' + ('/' + source),
196+
category: 'group nodes' + ('>' + source),
197197
input: { required: {} },
198198
description: `Group node combining ${this.nodeData.nodes
199199
.map((n) => n.type)
@@ -216,7 +216,7 @@ export class GroupNodeConfig {
216216
p()
217217
}
218218
this.#convertedToProcess = null
219-
await app.registerNodeDef('workflow/' + this.name, this.nodeDef)
219+
await app.registerNodeDef('workflow>' + this.name, this.nodeDef)
220220
useNodeDefStore().addNodeDef(this.nodeDef)
221221
}
222222

@@ -1380,7 +1380,7 @@ export class GroupNodeHandler {
13801380
const config = new GroupNodeConfig(name, nodeData)
13811381
await config.registerType()
13821382

1383-
const groupNode = LiteGraph.createNode(`workflow/${name}`)
1383+
const groupNode = LiteGraph.createNode(`workflow>${name}`)
13841384
// Reuse the existing nodes for this instance
13851385
groupNode.setInnerNodes(builder.nodes)
13861386
groupNode[GROUP].populateWidgets()

‎src/extensions/core/groupNodeManage.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {
102102

103103
getGroupData() {
104104
this.groupNodeType =
105-
LiteGraph.registered_node_types['workflow/' + this.selectedGroup]
105+
LiteGraph.registered_node_types['workflow>' + this.selectedGroup]
106106
this.groupNodeDef = this.groupNodeType.nodeData
107107
this.groupData = GroupNodeHandler.getGroupData(this.groupNodeType)
108108
}
@@ -367,7 +367,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {
367367
groupNodes.map((g) =>
368368
$el('option', {
369369
textContent: g,
370-
selected: 'workflow/' + g === type,
370+
selected: 'workflow>' + g === type,
371371
value: g
372372
})
373373
)
@@ -389,7 +389,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {
389389
{
390390
onclick: (e) => {
391391
const node = app.graph.nodes.find(
392-
(n) => n.type === 'workflow/' + this.selectedGroup
392+
(n) => n.type === 'workflow>' + this.selectedGroup
393393
)
394394
if (node) {
395395
alert(
@@ -403,7 +403,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {
403403
)
404404
) {
405405
delete app.graph.extra.groupNodes[this.selectedGroup]
406-
LiteGraph.unregisterNodeType('workflow/' + this.selectedGroup)
406+
LiteGraph.unregisterNodeType('workflow>' + this.selectedGroup)
407407
}
408408
this.show()
409409
}
@@ -476,7 +476,7 @@ export class ManageGroupDialog extends ComfyDialog<HTMLDialogElement> {
476476
}, {})
477477
}
478478

479-
const nodes = nodesByType['workflow/' + g]
479+
const nodes = nodesByType['workflow>' + g]
480480
if (nodes) recreateNodes.push(...nodes)
481481
}
482482

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

504504
this.element.replaceChildren(outer)
505505
this.changeGroup(
506-
type ? groupNodes.find((g) => 'workflow/' + g === type) : groupNodes[0]
506+
type ? groupNodes.find((g) => 'workflow>' + g === type) : groupNodes[0]
507507
)
508508
this.element.showModal()
509509

‎tests-ui/tests/groupNode.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ describe('group node', () => {
4646
expect(n.isRemoved).toBeTruthy()
4747
}
4848

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

5151
return graph.find(groupNode)
5252
}
@@ -520,7 +520,7 @@ describe('group node', () => {
520520
group1.menu.Clone.call()
521521
expect(app.graph.nodes).toHaveLength(4)
522522
const group2 = graph.find(app.graph.nodes[3])
523-
expect(group2.node.type).toEqual('workflow/test')
523+
expect(group2.node.type).toEqual('workflow>test')
524524
expect(group2.id).not.toEqual(group1.id)
525525

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

687687
// Reconnect ckpt
@@ -741,7 +741,7 @@ describe('group node', () => {
741741
resetEnv: true
742742
}))
743743
// Ensure the node isnt registered
744-
expect(() => ez['workflow/test']).toThrow()
744+
expect(() => ez['workflow>test']).toThrow()
745745

746746
// Reload the workflow
747747
await app.loadGraphData(JSON.parse(workflow))
@@ -768,7 +768,7 @@ describe('group node', () => {
768768
nodes: [
769769
{
770770
id: 3,
771-
type: 'workflow/testerror'
771+
type: 'workflow>testerror'
772772
}
773773
],
774774
links: [],
@@ -796,7 +796,7 @@ describe('group node', () => {
796796
expect(call).toContain('the following node types were not found')
797797
expect(call).toContain('NotKSampler')
798798
expect(call).toContain('NotVAEDecode')
799-
expect(call).toContain('workflow/testerror')
799+
expect(call).toContain('workflow>testerror')
800800
})
801801
test('maintains widget inputs on conversion back to nodes', async () => {
802802
const { ez, graph, app } = await start()

0 commit comments

Comments
 (0)