Skip to content

Commit cb52a38

Browse files
authored
fix groups dragging children with control held (#12867)
When control is held, an active drag operation should cease applying movements to nodes contained by selected groups. This functionality was broken in vue mode because of unnecessary reimplementation of the code for calculating items contained by groups during drag operations
1 parent 7a877d0 commit cb52a38

2 files changed

Lines changed: 27 additions & 54 deletions

File tree

browser_tests/tests/vueNodes/groups/groups.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,30 @@ test.describe('Vue Node Groups', { tag: ['@screenshot', '@vue-nodes'] }, () => {
177177
}).toPass({ timeout: 5000 })
178178
})
179179

180+
test('does not drag contents when control is held', async ({ comfyPage }) => {
181+
await comfyPage.keyboard.selectAll()
182+
await comfyPage.page.keyboard.press(CREATE_GROUP_HOTKEY)
183+
const groupCount = () => comfyPage.page.evaluate(() => graph!.groups.length)
184+
await expect.poll(groupCount, 'create group').toBe(1)
185+
await comfyPage.page.mouse.click(100, 100)
186+
187+
const ksampler = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
188+
const initialNodeBounds = await ksampler.boundingBox()
189+
expect(initialNodeBounds).toBeTruthy()
190+
191+
const groupPos = await getGroupTitlePosition(comfyPage, 'Group')
192+
await comfyPage.page.mouse.move(groupPos.x, groupPos.y)
193+
await comfyPage.page.mouse.down()
194+
await comfyPage.page.keyboard.down('Control')
195+
await comfyPage.page.mouse.move(groupPos.x + 100, groupPos.y)
196+
await comfyPage.page.mouse.up()
197+
await comfyPage.page.keyboard.up('Control')
198+
await expect
199+
.poll(() => getGroupTitlePosition(comfyPage, 'Group'))
200+
.not.toEqual(groupPos)
201+
expect(await ksampler.boundingBox()).toEqual(initialNodeBounds)
202+
})
203+
180204
test('should keep groups aligned after loading legacy Vue workflows', async ({
181205
comfyPage
182206
}) => {

src/lib/litegraph/src/LGraphCanvas.ts

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -8931,71 +8931,20 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
89318931
}
89328932
}
89338933

8934-
/**
8935-
* Collect all nodes that are children of groups in the selection
8936-
*/
8937-
private collectNodesInGroups(items: Set<Positionable>): Set<LGraphNode> {
8938-
const nodesInGroups = new Set<LGraphNode>()
8939-
for (const item of items) {
8940-
if (item instanceof LGraphGroup) {
8941-
for (const child of item._children) {
8942-
if (child instanceof LGraphNode) {
8943-
nodesInGroups.add(child)
8944-
}
8945-
}
8946-
}
8947-
}
8948-
return nodesInGroups
8949-
}
8950-
8951-
/**
8952-
* Move group children (both nodes and non-nodes)
8953-
*/
8954-
private moveGroupChildren(
8955-
group: LGraphGroup,
8956-
deltaX: number,
8957-
deltaY: number,
8958-
nodesToMove: Array<{ node: LGraphNode; newPos: { x: number; y: number } }>
8959-
): void {
8960-
for (const child of group._children) {
8961-
if (child instanceof LGraphNode) {
8962-
const node = child as LGraphNode
8963-
nodesToMove.push({
8964-
node,
8965-
newPos: this.calculateNewPosition(node, deltaX, deltaY)
8966-
})
8967-
} else if (!(child instanceof LGraphGroup)) {
8968-
// Non-node, non-group children (reroutes, etc.)
8969-
// Skip groups here - they're already in allItems and will be
8970-
// processed in the main loop of moveChildNodesInGroupVueMode
8971-
child.move(deltaX, deltaY, true)
8972-
}
8973-
}
8974-
}
8975-
89768934
moveChildNodesInGroupVueMode(
89778935
allItems: Set<Positionable>,
89788936
deltaX: number,
89798937
deltaY: number
89808938
) {
8981-
const nodesInMovingGroups = this.collectNodesInGroups(allItems)
89828939
const nodesToMove: NewNodePosition[] = []
89838940

89848941
// First, collect all the moves we need to make
89858942
for (const item of allItems) {
8986-
const isNode = item instanceof LGraphNode
8987-
if (isNode) {
8988-
const node = item as LGraphNode
8989-
if (nodesInMovingGroups.has(node)) {
8990-
continue
8991-
}
8943+
if (item instanceof LGraphNode) {
89928944
nodesToMove.push({
8993-
node,
8994-
newPos: this.calculateNewPosition(node, deltaX, deltaY)
8945+
node: item,
8946+
newPos: this.calculateNewPosition(item, deltaX, deltaY)
89958947
})
8996-
} else if (item instanceof LGraphGroup) {
8997-
item.move(deltaX, deltaY, true)
8998-
this.moveGroupChildren(item, deltaX, deltaY, nodesToMove)
89998948
} else {
90008949
// Other items (reroutes, etc.)
90018950
item.move(deltaX, deltaY, true)

0 commit comments

Comments
 (0)