fix: nested group overlap with disconnected nodes in default layout mode

2026-01-14 17:41:00 -06:00
parent 1e6f4ec011
commit 9994b404ee
3 changed files with 359 additions and 22 deletions
@@ -528,7 +528,7 @@ function layoutGroupContents(
  }

  // Process nested groups (bottom-up: deepest first)
-  // Sort by containment depth
+  // In default mode, reposition direct nested groups below member content
  const sortedNested = [...nestedGroups].sort((a, b) => {
    // Count how many other nested groups contain each
    let depthA = 0;
@@ -540,13 +540,52 @@ function layoutGroupContents(
    return depthB - depthA; // Deepest first
  });

-  for (const nestedGroup of sortedNested) {
-    // Get direct members of nested group
+  // Identify direct nested groups (not contained by other nested groups)
+  const directNestedGroups = sortedNested.filter((g) => {
+    for (const other of nestedGroups) {
+      if (other !== g && groupContainsGroup(other, g)) return false;
+    }
+    return true;
+  });
+
+  // Sort direct groups by original position for stable ordering
+  directNestedGroups.sort((a, b) => {
+    const yDiff = a.pos[1] - b.pos[1];
+    return yDiff !== 0 ? yDiff : a.pos[0] - b.pos[0];
+  });
+
+  for (const nestedGroup of directNestedGroups) {
+    // Skip if already processed (by token mode)
+    if (processedGroups.has(nestedGroup)) {
+      // Still include bounds
+      minX = Math.min(minX, nestedGroup.pos[0]);
+      minY = Math.min(minY, nestedGroup.pos[1]);
+      maxX = Math.max(maxX, nestedGroup.pos[0] + nestedGroup.size[0]);
+      maxY = Math.max(maxY, nestedGroup.pos[1] + nestedGroup.size[1]);
+      continue;
+    }
+
+    // Get nested group's members and children
    const nestedChildren = collectNestedGroups(nestedGroup, nestedGroups);
    const nestedMembers = collectDirectMembers(nestedGroup, allNodes, nestedChildren);

-    // Recursively layout nested group
-    const nestedBounds = layoutGroupContents(
+    // In default mode, reposition nested group below current content
+    if (layoutMode.type === "default") {
+      // Position nested group's left edge at startX (same as members) for consistent alignment
+      // This prevents the group from shifting leftward on each layout run
+      const targetX = startX;
+      const targetY = contentEndY + config.verticalGap;
+      const deltaX = targetX - nestedGroup.pos[0];
+      const deltaY = targetY - nestedGroup.pos[1];
+
+      if (deltaX !== 0 || deltaY !== 0) {
+        translateGroupWithMembers(nestedGroup, deltaX, deltaY, nestedMembers, allNodes, nestedGroups);
+        debugLog(`  Repositioned nested group "${nestedGroup.title}" to [${targetX}, ${targetY}]`);
+      }
+    }
+
+    // Recursively layout nested group contents
+    layoutGroupContents(
      nestedGroup,
      nestedMembers,
      nestedChildren,
@@ -555,24 +594,17 @@ function layoutGroupContents(
      processedGroups
    );

-    if (nestedBounds) {
-      // Update bounds to include nested group
-      minX = Math.min(minX, nestedBounds.minX - config.groupPadding);
-      minY = Math.min(minY, nestedBounds.minY - config.groupPadding - GROUP_TITLE_HEIGHT);
-      maxX = Math.max(maxX, nestedBounds.maxX + config.groupPadding);
-      maxY = Math.max(maxY, nestedBounds.maxY + config.groupPadding);
-    } else {
-      // Include nested group's current bounds
-      const nx = nestedGroup.pos[0];
-      const ny = nestedGroup.pos[1];
-      const nw = nestedGroup.size[0];
-      const nh = nestedGroup.size[1];
+    // Resize nested group to fit its organized contents
+    resizeGroupToFitMembers(nestedGroup, nestedMembers, allNodes, config);

-      minX = Math.min(minX, nx);
-      minY = Math.min(minY, ny);
-      maxX = Math.max(maxX, nx + nw);
-      maxY = Math.max(maxY, ny + nh);
-    }
+    // Update contentEndY for next nested group
+    contentEndY = Math.max(contentEndY, nestedGroup.pos[1] + nestedGroup.size[1]);
+
+    // Update bounds to include nested group
+    minX = Math.min(minX, nestedGroup.pos[0]);
+    minY = Math.min(minY, nestedGroup.pos[1]);
+    maxX = Math.max(maxX, nestedGroup.pos[0] + nestedGroup.size[0]);
+    maxY = Math.max(maxY, nestedGroup.pos[1] + nestedGroup.size[1]);
  }

  // Resize group to fit contents
@@ -0,0 +1,188 @@
+{
+  "id": "bbe5e07d-ae25-42f3-b43f-63e9274165a1",
+  "revision": 0,
+  "last_node_id": 6,
+  "last_link_id": 4,
+  "nodes": [
+    {
+      "id": 3,
+      "type": "PreviewImage",
+      "pos": [
+        550,
+        570
+      ],
+      "size": [
+        660,
+        1000
+      ],
+      "flags": {},
+      "order": 2,
+      "mode": 0,
+      "inputs": [
+        {
+          "name": "images",
+          "type": "IMAGE",
+          "link": 3
+        }
+      ],
+      "outputs": [],
+      "title": "Preview Image 1",
+      "properties": {
+        "cnr_id": "comfy-core",
+        "ver": "0.8.2",
+        "Node name for S&R": "PreviewImage"
+      },
+      "widgets_values": []
+    },
+    {
+      "id": 4,
+      "type": "PreviewImage",
+      "pos": [
+        1310,
+        570
+      ],
+      "size": [
+        660,
+        1000
+      ],
+      "flags": {},
+      "order": 3,
+      "mode": 0,
+      "inputs": [
+        {
+          "name": "images",
+          "type": "IMAGE",
+          "link": 4
+        }
+      ],
+      "outputs": [],
+      "title": "Preview Image 2",
+      "properties": {
+        "cnr_id": "comfy-core",
+        "ver": "0.8.2",
+        "Node name for S&R": "PreviewImage"
+      },
+      "widgets_values": []
+    },
+    {
+      "id": 5,
+      "type": "GetNode",
+      "pos": [
+        140,
+        150
+      ],
+      "size": [
+        210,
+        60
+      ],
+      "flags": {},
+      "order": 0,
+      "mode": 0,
+      "inputs": [],
+      "outputs": [
+        {
+          "name": "*",
+          "type": "*",
+          "links": [
+            3
+          ]
+        }
+      ],
+      "title": "GetNode 1",
+      "properties": {},
+      "widgets_values": [
+        ""
+      ]
+    },
+    {
+      "id": 6,
+      "type": "GetNode",
+      "pos": [
+        450,
+        150
+      ],
+      "size": [
+        210,
+        58
+      ],
+      "flags": {},
+      "order": 1,
+      "mode": 0,
+      "inputs": [],
+      "outputs": [
+        {
+          "name": "*",
+          "type": "*",
+          "links": [
+            4
+          ]
+        }
+      ],
+      "title": "GetNode 2",
+      "properties": {},
+      "widgets_values": [
+        ""
+      ]
+    }
+  ],
+  "links": [
+    [
+      3,
+      5,
+      0,
+      3,
+      0,
+      "IMAGE"
+    ],
+    [
+      4,
+      6,
+      0,
+      4,
+      0,
+      "IMAGE"
+    ]
+  ],
+  "groups": [
+    {
+      "id": 1,
+      "title": "Group Wrapper",
+      "bounding": [
+        80,
+        -10,
+        2030,
+        1670
+      ],
+      "color": "#3f789e",
+      "font_size": 22,
+      "flags": {}
+    },
+    {
+      "id": 2,
+      "title": "Inner Group",
+      "bounding": [
+        110,
+        70,
+        580,
+        170
+      ],
+      "color": "#3f789e",
+      "font_size": 22,
+      "flags": {}
+    }
+  ],
+  "config": {},
+  "extra": {
+    "workflowRendererVersion": "LG",
+    "ue_links": [],
+    "ds": {
+      "scale": 0.47082251883518567,
+      "offset": [
+        1126.5268822743458,
+        397.1123544821961
+      ]
+    },
+    "frontendVersion": "1.37.9"
+  },
+  "version": 0.4
+}
@@ -718,6 +718,123 @@ describe("layoutSelectedGroups", () => {
    });
  });

+  describe("nested group + disconnected nodes overlap regression (group-test-simple.json)", () => {
+    it("positions nested groups below disconnected nodes (no token)", () => {
+      const graph = loadFixture("group-test-simple.json");
+      layoutSelectedGroups(graph, new Set([1])); // "Group Wrapper"
+
+      const innerGroup = graph._groups.find((g) => g.id === 2)!;
+      const node3 = graph._nodes.find((n) => n.id === 3)!;
+      const node4 = graph._nodes.find((n) => n.id === 4)!;
+
+      // Inner group should be BELOW the preview images
+      const node3Bottom = node3.pos[1] + node3.size[1];
+      const node4Bottom = node4.pos[1] + node4.size[1];
+      const membersBottom = Math.max(node3Bottom, node4Bottom);
+      expect(innerGroup.pos[1]).toBeGreaterThan(membersBottom);
+    });
+
+    it("maintains no overlaps after default layout", () => {
+      const graph = loadFixture("group-test-simple.json");
+      layoutSelectedGroups(graph, new Set([1]));
+      assertNodesInsideGroups(graph);
+      assertFiniteCoordinates(graph);
+    });
+
+    it("is idempotent with nested groups in default mode", () => {
+      const graph = loadFixture("group-test-simple.json");
+      layoutSelectedGroups(graph, new Set([1]));
+      const positions1 = capturePositions(graph);
+
+      layoutSelectedGroups(graph, new Set([1]));
+      const positions2 = capturePositions(graph);
+
+      // Compare positions with small tolerance
+      for (const [id, pos1] of positions1) {
+        const pos2 = positions2.get(id);
+        if (pos2) {
+          expect(Math.abs(pos1.x - pos2.x)).toBeLessThan(5);
+          expect(Math.abs(pos1.y - pos2.y)).toBeLessThan(5);
+        }
+      }
+    });
+
+    it("multiple sibling nested groups stack vertically", () => {
+      const graph: LGraph = {
+        _nodes: [
+          // Disconnected node in parent
+          {
+            id: 1,
+            type: "Note",
+            title: "Parent Node",
+            pos: [100, 100] as [number, number],
+            size: [200, 100] as [number, number],
+            inputs: [],
+            outputs: [],
+          },
+          // Node in first nested group
+          {
+            id: 2,
+            type: "Note",
+            title: "Nested 1 Node",
+            pos: [100, 300] as [number, number],
+            size: [200, 100] as [number, number],
+            inputs: [],
+            outputs: [],
+          },
+          // Node in second nested group
+          {
+            id: 3,
+            type: "Note",
+            title: "Nested 2 Node",
+            pos: [100, 500] as [number, number],
+            size: [200, 100] as [number, number],
+            inputs: [],
+            outputs: [],
+          },
+        ],
+        _groups: [
+          {
+            id: 1,
+            title: "Parent Group",
+            pos: [50, 50] as [number, number],
+            size: [400, 600] as [number, number],
+          },
+          {
+            id: 2,
+            title: "Nested 1",
+            pos: [80, 250] as [number, number],
+            size: [300, 180] as [number, number],
+          },
+          {
+            id: 3,
+            title: "Nested 2",
+            pos: [80, 450] as [number, number],
+            size: [300, 180] as [number, number],
+          },
+        ],
+        links: new Map(),
+      };
+
+      layoutSelectedGroups(graph, new Set([1]));
+
+      const nested1 = graph._groups.find((g) => g.id === 2)!;
+      const nested2 = graph._groups.find((g) => g.id === 3)!;
+      const parentNode = graph._nodes.find((n) => n.id === 1)!;
+
+      // Both nested groups should be below the parent node
+      const parentNodeBottom = parentNode.pos[1] + parentNode.size[1];
+      expect(nested1.pos[1]).toBeGreaterThan(parentNodeBottom);
+      expect(nested2.pos[1]).toBeGreaterThan(parentNodeBottom);
+
+      // Nested groups should not overlap each other
+      const nested1Bottom = nested1.pos[1] + nested1.size[1];
+      expect(nested2.pos[1]).toBeGreaterThanOrEqual(nested1Bottom);
+
+      assertFiniteCoordinates(graph);
+    });
+  });
+
  describe("nested group position update regression (group-test.json)", () => {
    it("nested group position updates when parent has token layout", () => {
      const graph = loadFixture("group-test.json");