diff --git a/browser_tests/tests/groupSelectChildren.spec.ts b/browser_tests/tests/groupSelectChildren.spec.ts new file mode 100644 index 0000000000..7530dbd418 --- /dev/null +++ b/browser_tests/tests/groupSelectChildren.spec.ts @@ -0,0 +1,123 @@ +import { expect } from '@playwright/test' + +import { comfyPageFixture as test } from '../fixtures/ComfyPage' +import type { ComfyPage } from '../fixtures/ComfyPage' + +/** + * Returns the client-space position of a group's title bar (for clicking). + */ +async function getGroupTitlePosition( + comfyPage: ComfyPage, + title: string +): Promise<{ x: number; y: number }> { + const pos = await comfyPage.page.evaluate((title) => { + const app = window.app! + const group = app.graph.groups.find( + (g: { title: string }) => g.title === title + ) + if (!group) return null + const clientPos = app.canvasPosToClientPos([ + group.pos[0] + 50, + group.pos[1] + 15 + ]) + return { x: clientPos[0], y: clientPos[1] } + }, title) + if (!pos) throw new Error(`Group "${title}" not found`) + return pos +} + +/** + * Returns {selectedNodeCount, selectedGroupCount, selectedItemCount} + * from the canvas in the browser. + */ +async function getSelectionCounts(comfyPage: ComfyPage) { + return comfyPage.page.evaluate(() => { + const canvas = window.app!.canvas + let selectedNodeCount = 0 + let selectedGroupCount = 0 + for (const item of canvas.selectedItems) { + if ('inputs' in item || 'outputs' in item) selectedNodeCount++ + else selectedGroupCount++ + } + return { + selectedNodeCount, + selectedGroupCount, + selectedItemCount: canvas.selectedItems.size + } + }) +} + +test.describe('Group Select Children', { tag: ['@canvas', '@node'] }, () => { + test.afterEach(async ({ comfyPage }) => { + await comfyPage.canvasOps.resetView() + }) + + test('Setting enabled: clicking outer group selects nested group and inner node', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting( + 'LiteGraph.Group.SelectChildrenOnClick', + true + ) + await comfyPage.workflow.loadWorkflow('groups/nested-groups-1-inner-node') + await comfyPage.nextFrame() + + const outerPos = await getGroupTitlePosition(comfyPage, 'Outer Group') + await comfyPage.canvas.click({ position: outerPos }) + await comfyPage.nextFrame() + + const counts = await getSelectionCounts(comfyPage) + // Outer Group + Inner Group + 1 node = 3 items + expect(counts.selectedItemCount).toBe(3) + expect(counts.selectedGroupCount).toBe(2) + expect(counts.selectedNodeCount).toBe(1) + }) + + test('Setting disabled: clicking outer group selects only the group', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting( + 'LiteGraph.Group.SelectChildrenOnClick', + false + ) + await comfyPage.workflow.loadWorkflow('groups/nested-groups-1-inner-node') + await comfyPage.nextFrame() + + const outerPos = await getGroupTitlePosition(comfyPage, 'Outer Group') + await comfyPage.canvas.click({ position: outerPos }) + await comfyPage.nextFrame() + + const counts = await getSelectionCounts(comfyPage) + expect(counts.selectedItemCount).toBe(1) + expect(counts.selectedGroupCount).toBe(1) + expect(counts.selectedNodeCount).toBe(0) + }) + + test('Deselecting outer group deselects all children', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting( + 'LiteGraph.Group.SelectChildrenOnClick', + true + ) + await comfyPage.workflow.loadWorkflow('groups/nested-groups-1-inner-node') + await comfyPage.nextFrame() + + // Select the outer group (cascades to children) + const outerPos = await getGroupTitlePosition(comfyPage, 'Outer Group') + await comfyPage.canvas.click({ position: outerPos }) + await comfyPage.nextFrame() + + let counts = await getSelectionCounts(comfyPage) + expect(counts.selectedItemCount).toBe(3) + + // Deselect all via page.evaluate to avoid UI overlay interception + await comfyPage.page.evaluate(() => { + window.app!.canvas.deselectAll() + }) + await comfyPage.nextFrame() + + counts = await getSelectionCounts(comfyPage) + expect(counts.selectedItemCount).toBe(0) + }) +}) diff --git a/src/lib/litegraph/src/LGraphCanvas.groupSelection.test.ts b/src/lib/litegraph/src/LGraphCanvas.groupSelection.test.ts new file mode 100644 index 0000000000..ba7ea88b83 --- /dev/null +++ b/src/lib/litegraph/src/LGraphCanvas.groupSelection.test.ts @@ -0,0 +1,363 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { CanvasPointerEvent } from '@/lib/litegraph/src/types/events' + +import { + LGraph, + LGraphCanvas, + LGraphGroup, + LGraphNode +} from '@/lib/litegraph/src/litegraph' + +vi.mock('@/renderer/core/layout/store/layoutStore', () => ({ + layoutStore: { + querySlotAtPoint: vi.fn(), + queryRerouteAtPoint: vi.fn(), + getNodeLayoutRef: vi.fn(() => ({ value: null })), + getSlotLayout: vi.fn(), + setSource: vi.fn(), + batchUpdateNodeBounds: vi.fn() + } +})) + +function createCanvas(graph: LGraph): LGraphCanvas { + const el = document.createElement('canvas') + el.width = 800 + el.height = 600 + + const ctx = { + save: vi.fn(), + restore: vi.fn(), + translate: vi.fn(), + scale: vi.fn(), + fillRect: vi.fn(), + strokeRect: vi.fn(), + fillText: vi.fn(), + measureText: vi.fn().mockReturnValue({ width: 50 }), + beginPath: vi.fn(), + moveTo: vi.fn(), + lineTo: vi.fn(), + stroke: vi.fn(), + fill: vi.fn(), + closePath: vi.fn(), + arc: vi.fn(), + rect: vi.fn(), + clip: vi.fn(), + clearRect: vi.fn(), + setTransform: vi.fn(), + roundRect: vi.fn(), + getTransform: vi + .fn() + .mockReturnValue({ a: 1, b: 0, c: 0, d: 1, e: 0, f: 0 }), + font: '', + fillStyle: '', + strokeStyle: '', + lineWidth: 1, + globalAlpha: 1, + textAlign: 'left' as CanvasTextAlign, + textBaseline: 'alphabetic' as CanvasTextBaseline + } satisfies Partial + + el.getContext = vi + .fn() + .mockReturnValue(ctx as unknown as CanvasRenderingContext2D) + el.getBoundingClientRect = vi.fn().mockReturnValue({ + left: 0, + top: 0, + width: 800, + height: 600 + }) + + return new LGraphCanvas(el, graph, { skip_render: true }) +} + +class TestNode extends LGraphNode { + constructor() { + super('test') + } +} + +describe('LGraphCanvas group selection', () => { + let graph: LGraph + let canvas: LGraphCanvas + let group: LGraphGroup + let nodeA: TestNode + let nodeB: TestNode + + beforeEach(() => { + vi.clearAllMocks() + + graph = new LGraph() + canvas = createCanvas(graph) + + group = new LGraphGroup('TestGroup') + group._bounding.set([0, 0, 500, 500]) + graph.add(group) + + nodeA = new TestNode() + nodeA.pos = [50, 50] + graph.add(nodeA) + + nodeB = new TestNode() + nodeB.pos = [100, 100] + graph.add(nodeB) + + group.recomputeInsideNodes() + }) + + describe('select with groupSelectChildren enabled', () => { + beforeEach(() => { + canvas.groupSelectChildren = true + }) + + it('selects all children when selecting a group', () => { + canvas.select(group) + + expect(group.selected).toBe(true) + expect(nodeA.selected).toBe(true) + expect(nodeB.selected).toBe(true) + expect(canvas.selectedItems.has(group)).toBe(true) + expect(canvas.selectedItems.has(nodeA)).toBe(true) + expect(canvas.selectedItems.has(nodeB)).toBe(true) + }) + + it('recursively selects nested group children', () => { + const innerGroup = new LGraphGroup('InnerGroup') + innerGroup._bounding.set([40, 40, 200, 200]) + graph.add(innerGroup) + + const innerNode = new TestNode() + innerNode.pos = [60, 60] + graph.add(innerNode) + + innerGroup.recomputeInsideNodes() + group.recomputeInsideNodes() + + canvas.select(group) + + expect(innerGroup.selected).toBe(true) + expect(innerNode.selected).toBe(true) + expect(canvas.selectedItems.has(innerGroup)).toBe(true) + expect(canvas.selectedItems.has(innerNode)).toBe(true) + }) + + it('selects descendants of already-selected nested groups', () => { + const innerGroup = new LGraphGroup('InnerGroup') + innerGroup._bounding.set([40, 40, 200, 200]) + graph.add(innerGroup) + + const innerNode = new TestNode() + innerNode.pos = [60, 60] + graph.add(innerNode) + + innerGroup.recomputeInsideNodes() + group.recomputeInsideNodes() + + // Pre-select the inner group before selecting the outer group + canvas.select(innerGroup) + expect(innerGroup.selected).toBe(true) + expect(innerNode.selected).toBeFalsy() + + canvas.select(group) + + expect(innerNode.selected).toBe(true) + expect(canvas.selectedItems.has(innerNode)).toBe(true) + }) + + it('handles deeply nested groups (depth 5)', () => { + const groups: LGraphGroup[] = [group] + const nodes: TestNode[] = [nodeA, nodeB] + + for (let depth = 1; depth <= 5; depth++) { + const offset = depth * 10 + const size = 500 - depth * 20 + + const nestedGroup = new LGraphGroup(`Depth${depth}`) + nestedGroup._bounding.set([offset, offset, size, size]) + graph.add(nestedGroup) + groups.push(nestedGroup) + + const nestedNode = new TestNode() + nestedNode.pos = [offset + 5, offset + 5] + graph.add(nestedNode) + nodes.push(nestedNode) + } + + // Recompute from innermost to outermost + for (let i = groups.length - 1; i >= 0; i--) { + groups[i].recomputeInsideNodes() + } + + canvas.select(group) + + for (const g of groups) { + expect(g.selected).toBe(true) + expect(canvas.selectedItems.has(g)).toBe(true) + } + for (const n of nodes) { + expect(n.selected).toBe(true) + expect(canvas.selectedItems.has(n)).toBe(true) + } + }) + }) + + describe('select with groupSelectChildren disabled', () => { + beforeEach(() => { + canvas.groupSelectChildren = false + }) + + it('does not select children when selecting a group', () => { + canvas.select(group) + + expect(group.selected).toBe(true) + expect(nodeA.selected).toBeFalsy() + expect(nodeB.selected).toBeFalsy() + expect(canvas.selectedItems.has(group)).toBe(true) + expect(canvas.selectedItems.has(nodeA)).toBe(false) + }) + }) + + describe('deselect with groupSelectChildren enabled', () => { + beforeEach(() => { + canvas.groupSelectChildren = true + }) + + it('deselects all children when deselecting a group', () => { + canvas.select(group) + expect(nodeA.selected).toBe(true) + + canvas.deselect(group) + + expect(group.selected).toBe(false) + expect(nodeA.selected).toBe(false) + expect(nodeB.selected).toBe(false) + expect(canvas.selectedItems.has(group)).toBe(false) + expect(canvas.selectedItems.has(nodeA)).toBe(false) + }) + + it('recursively deselects nested group children', () => { + const innerGroup = new LGraphGroup('InnerGroup') + innerGroup._bounding.set([40, 40, 200, 200]) + graph.add(innerGroup) + + const innerNode = new TestNode() + innerNode.pos = [60, 60] + graph.add(innerNode) + + innerGroup.recomputeInsideNodes() + group.recomputeInsideNodes() + + canvas.select(group) + expect(innerNode.selected).toBe(true) + + canvas.deselect(group) + + expect(innerGroup.selected).toBe(false) + expect(innerNode.selected).toBe(false) + }) + + it('handles deeply nested deselection (depth 5)', () => { + const groups: LGraphGroup[] = [group] + const nodes: TestNode[] = [nodeA, nodeB] + + for (let depth = 1; depth <= 5; depth++) { + const offset = depth * 10 + const size = 500 - depth * 20 + + const nestedGroup = new LGraphGroup(`Depth${depth}`) + nestedGroup._bounding.set([offset, offset, size, size]) + graph.add(nestedGroup) + groups.push(nestedGroup) + + const nestedNode = new TestNode() + nestedNode.pos = [offset + 5, offset + 5] + graph.add(nestedNode) + nodes.push(nestedNode) + } + + for (let i = groups.length - 1; i >= 0; i--) { + groups[i].recomputeInsideNodes() + } + + canvas.select(group) + canvas.deselect(group) + + for (const g of groups) { + expect(g.selected).toBe(false) + expect(canvas.selectedItems.has(g)).toBe(false) + } + for (const n of nodes) { + expect(n.selected).toBe(false) + expect(canvas.selectedItems.has(n)).toBe(false) + } + }) + }) + + describe('processSelect modifier-click deselect', () => { + beforeEach(() => { + canvas.groupSelectChildren = true + }) + + it('modifier-click deselects only the group, not its children', () => { + canvas.select(group) + expect(group.selected).toBe(true) + expect(nodeA.selected).toBe(true) + expect(nodeB.selected).toBe(true) + + const shiftEvent = { shiftKey: true } as CanvasPointerEvent + canvas.processSelect(group, shiftEvent) + + expect(group.selected).toBe(false) + expect(canvas.selectedItems.has(group)).toBe(false) + expect(nodeA.selected).toBe(true) + expect(nodeB.selected).toBe(true) + expect(canvas.selectedItems.has(nodeA)).toBe(true) + expect(canvas.selectedItems.has(nodeB)).toBe(true) + }) + + it('ctrl-click deselects only the group, not its children', () => { + canvas.select(group) + + const ctrlEvent = { ctrlKey: true } as CanvasPointerEvent + canvas.processSelect(group, ctrlEvent) + + expect(group.selected).toBe(false) + expect(nodeA.selected).toBe(true) + expect(nodeB.selected).toBe(true) + }) + }) + + describe('deselect with groupSelectChildren disabled', () => { + it('does not deselect children when deselecting a group', () => { + canvas.groupSelectChildren = true + canvas.select(group) + + canvas.groupSelectChildren = false + canvas.deselect(group) + + expect(group.selected).toBe(false) + expect(nodeA.selected).toBe(true) + expect(nodeB.selected).toBe(true) + }) + }) + + describe('deleteSelected with groupSelectChildren enabled', () => { + beforeEach(() => { + canvas.groupSelectChildren = true + // Attach canvas to DOM so checkPanels() can query parentNode + document.body.appendChild(canvas.canvas) + }) + + it('deletes group and all selected children', () => { + canvas.select(group) + + expect(canvas.selectedItems.size).toBeGreaterThan(1) + + canvas.deleteSelected() + + expect(graph.nodes).not.toContain(nodeA) + expect(graph.nodes).not.toContain(nodeB) + expect(graph.groups).not.toContain(group) + }) + }) +}) diff --git a/src/lib/litegraph/src/LGraphCanvas.ts b/src/lib/litegraph/src/LGraphCanvas.ts index fb9d878cc6..5ecd5bfebc 100644 --- a/src/lib/litegraph/src/LGraphCanvas.ts +++ b/src/lib/litegraph/src/LGraphCanvas.ts @@ -565,6 +565,7 @@ export class LGraphCanvas implements CustomEventDispatcher allow_dragnodes: boolean allow_interaction: boolean multi_select: boolean + groupSelectChildren: boolean allow_searchbox: boolean allow_reconnect_links: boolean align_to_grid: boolean @@ -933,6 +934,7 @@ export class LGraphCanvas implements CustomEventDispatcher this.allow_interaction = true // allow selecting multi nodes without pressing extra keys this.multi_select = false + this.groupSelectChildren = false this.allow_searchbox = true // allows to change a connection with having to redo it again this.allow_reconnect_links = true @@ -4371,7 +4373,17 @@ export class LGraphCanvas implements CustomEventDispatcher if (!modifySelection) this.deselectAll(item) this.select(item) } else if (modifySelection && !sticky) { - this.deselect(item) + // Modifier-click toggles only the clicked item, not its children. + // Cascade on select is a convenience; cascade on deselect would + // remove the user's ability to keep children selected (e.g. for + // deletion) after toggling the group off. + if (item instanceof LGraphGroup && this.groupSelectChildren) { + item.selected = false + this.selectedItems.delete(item) + this.state.selectionChanged = true + } else { + this.deselect(item) + } } else if (!sticky) { this.deselectAll(item) } else { @@ -4396,6 +4408,19 @@ export class LGraphCanvas implements CustomEventDispatcher if (item instanceof LGraphGroup) { item.recomputeInsideNodes() + if (this.groupSelectChildren) { + this.#traverseGroupChildren( + item, + (child) => { + if (!child.selected || !this.selectedItems.has(child)) { + child.selected = true + this.selectedItems.add(child) + this.state.selectionChanged = true + } + }, + (child) => this.select(child) + ) + } return } @@ -4434,6 +4459,22 @@ export class LGraphCanvas implements CustomEventDispatcher item.selected = false this.selectedItems.delete(item) this.state.selectionChanged = true + + if (item instanceof LGraphGroup && this.groupSelectChildren) { + this.#traverseGroupChildren( + item, + (child) => { + if (child.selected || this.selectedItems.has(child)) { + child.selected = false + this.selectedItems.delete(child) + this.state.selectionChanged = true + } + }, + (child) => this.deselect(child) + ) + return + } + if (!(item instanceof LGraphNode)) return // Node-specific handling @@ -4469,6 +4510,29 @@ export class LGraphCanvas implements CustomEventDispatcher } } + /** + * Iterative traversal of a group's descendants. + * Calls {@link groupAction} on nested groups and {@link leafAction} on + * non-group children. Always recurses into nested groups regardless of + * their current selection state. + */ + #traverseGroupChildren( + group: LGraphGroup, + groupAction: (child: LGraphGroup) => void, + leafAction: (child: Positionable) => void + ): void { + const stack: Positionable[] = [...group._children] + while (stack.length > 0) { + const child = stack.pop()! + if (child instanceof LGraphGroup) { + groupAction(child) + for (const nested of child._children) stack.push(nested) + } else { + leafAction(child) + } + } + } + /** @deprecated See {@link LGraphCanvas.processSelect} */ processNodeSelected(item: LGraphNode, e: CanvasPointerEvent): void { this.processSelect( @@ -4601,7 +4665,9 @@ export class LGraphCanvas implements CustomEventDispatcher this.emitBeforeChange() graph.beforeChange() - for (const item of this.selectedItems) { + // Snapshot to prevent mutation during iteration (e.g. group deselect cascade) + const toDelete = [...this.selectedItems] + for (const item of toDelete) { if (item instanceof LGraphNode) { const node = item if (node.block_delete) continue diff --git a/src/platform/settings/composables/useLitegraphSettings.ts b/src/platform/settings/composables/useLitegraphSettings.ts index ffdb58c92b..25319a64fb 100644 --- a/src/platform/settings/composables/useLitegraphSettings.ts +++ b/src/platform/settings/composables/useLitegraphSettings.ts @@ -162,4 +162,12 @@ export const useLitegraphSettings = () => { 'Comfy.EnableWorkflowViewRestore' ) }) + + watchEffect(() => { + const selectChildren = settingStore.get( + 'LiteGraph.Group.SelectChildrenOnClick' + ) + if (canvasStore.canvas) + canvasStore.canvas.groupSelectChildren = selectChildren + }) } diff --git a/src/platform/settings/constants/coreSettings.ts b/src/platform/settings/constants/coreSettings.ts index c5749086f0..137dc26501 100644 --- a/src/platform/settings/constants/coreSettings.ts +++ b/src/platform/settings/constants/coreSettings.ts @@ -1258,5 +1258,15 @@ export const CORE_SETTINGS: SettingParams[] = [ defaultValue: true, experimental: true, versionAdded: '1.40.0' + }, + { + id: 'LiteGraph.Group.SelectChildrenOnClick', + category: ['LiteGraph', 'Group', 'SelectChildrenOnClick'], + name: 'Select group children on click', + tooltip: + 'When enabled, clicking a group selects all nodes and items inside it', + type: 'boolean', + defaultValue: false, + versionAdded: '1.42.0' } ] diff --git a/src/schemas/apiSchema.ts b/src/schemas/apiSchema.ts index fb530f54f5..2d22423d08 100644 --- a/src/schemas/apiSchema.ts +++ b/src/schemas/apiSchema.ts @@ -464,7 +464,8 @@ const zSettings = z.object({ 'Comfy.VersionCompatibility.DisableWarnings': z.boolean(), 'Comfy.RightSidePanel.IsOpen': z.boolean(), 'Comfy.RightSidePanel.ShowErrorsTab': z.boolean(), - 'Comfy.Node.AlwaysShowAdvancedWidgets': z.boolean() + 'Comfy.Node.AlwaysShowAdvancedWidgets': z.boolean(), + 'LiteGraph.Group.SelectChildrenOnClick': z.boolean() }) export type EmbeddingsResponse = z.infer