From 4c111f80ff0d3ff455f9f6c244645e5d53f5ca65 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Wed, 11 Mar 2026 20:43:00 +0100 Subject: [PATCH] feat: add horizontal and bottom padding to nodeExtraSpace for groups Extend nodeExtraSpace from {top, bottom} to {top, bottom, left, right} so the layout engine allocates wider columns for nodes with group decorations. Groups get 16px side and bottom padding matching GroupOverlay. Bottom handle of the bottommost group node is offset down via groupBottomOffset on edges. Co-Authored-By: Claude Opus 4.6 --- .../lib/components/graph/FlowGraphV2.svelte | 71 ++++++++++++++----- .../lib/components/graph/compoundLayout.ts | 24 ++++--- .../graph/renderers/edges/BaseEdge.svelte | 10 +-- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/frontend/src/lib/components/graph/FlowGraphV2.svelte b/frontend/src/lib/components/graph/FlowGraphV2.svelte index 667cc3c347..78c2a98156 100644 --- a/frontend/src/lib/components/graph/FlowGraphV2.svelte +++ b/frontend/src/lib/components/graph/FlowGraphV2.svelte @@ -362,8 +362,8 @@ */ function computeNodeExtraSpace( graphNodes: NodeDep[] - ): Map | undefined { - const extraSpace = new Map() + ): Map | undefined { + const extraSpace = new Map() // 1. Assets if ($showAssets) { @@ -373,8 +373,9 @@ const hasRead = assets.some(assetDisplaysAsInputInFlowGraph) const hasWrite = assets.some(assetDisplaysAsOutputInFlowGraph) if (hasRead || hasWrite) { - const prev = extraSpace.get(node.id) ?? { top: 0, bottom: 0 } + const prev = extraSpace.get(node.id) ?? { top: 0, bottom: 0, left: 0, right: 0 } extraSpace.set(node.id, { + ...prev, top: prev.top + (hasRead ? NODE_WITH_READ_ASSET_Y_OFFSET : 0), bottom: prev.bottom + (hasWrite ? NODE_WITH_WRITE_ASSET_Y_OFFSET : 0) }) @@ -393,15 +394,15 @@ // Execution mode: tools below const totalRows = Math.ceil(agentActions.length / MAX_TOOLS_PER_ROW) const space = AI_TOOL_BASE_OFFSET + AI_TOOL_ROW_OFFSET * totalRows + BELOW_ADDITIONAL_OFFSET - const prev = extraSpace.get(node.id) ?? { top: 0, bottom: 0 } - extraSpace.set(node.id, { top: prev.top, bottom: prev.bottom + space }) + const prev = extraSpace.get(node.id) ?? { top: 0, bottom: 0, left: 0, right: 0 } + extraSpace.set(node.id, { ...prev, bottom: prev.bottom + space }) } else { // Edit mode: tools above const tools = mod.value.tools ?? [] const totalRows = Math.ceil(tools.length / MAX_TOOLS_PER_ROW) + (insertable ? 1 : 0) const space = AI_TOOL_BASE_OFFSET + AI_TOOL_ROW_OFFSET * totalRows - const prev = extraSpace.get(node.id) ?? { top: 0, bottom: 0 } - extraSpace.set(node.id, { top: prev.top + space, bottom: prev.bottom }) + const prev = extraSpace.get(node.id) ?? { top: 0, bottom: 0, left: 0, right: 0 } + extraSpace.set(node.id, { ...prev, top: prev.top + space }) } } @@ -418,10 +419,10 @@ if (topmostNodeId) { const textHeight = noteTextHeights[groupNote.id] || 60 const spacing = textHeight + 16 // padding - const prev = extraSpace.get(topmostNodeId) ?? { top: 0, bottom: 0 } + const prev = extraSpace.get(topmostNodeId) ?? { top: 0, bottom: 0, left: 0, right: 0 } extraSpace.set(topmostNodeId, { - top: Math.max(prev.top, spacing + prev.top), - bottom: prev.bottom + ...prev, + top: Math.max(prev.top, spacing + prev.top) }) } } @@ -456,11 +457,28 @@ const spacing = isCollapsed ? GROUP_HEADER_HEIGHT + noteHeight : GROUP_HEADER_HEIGHT + noteHeight + GROUP_TOP_MARGIN - const prev = extraSpace.get(topmostNodeId) ?? { top: 0, bottom: 0 } + const groupPadding = 16 + const prev = extraSpace.get(topmostNodeId) ?? { top: 0, bottom: 0, left: 0, right: 0 } extraSpace.set(topmostNodeId, { top: prev.top + spacing, - bottom: prev.bottom + bottom: prev.bottom, + left: Math.max(prev.left, groupPadding), + right: Math.max(prev.right, groupPadding) }) + + // Bottom padding for uncollapsed groups on the bottommost node + if (!isCollapsed) { + const bottommostNodeId = [...sortedNodes].reverse().find((node) => + group.module_ids.includes(node.id) + )?.id + if (bottommostNodeId) { + const prevBottom = extraSpace.get(bottommostNodeId) ?? { top: 0, bottom: 0, left: 0, right: 0 } + extraSpace.set(bottommostNodeId, { + ...prevBottom, + bottom: Math.max(prevBottom.bottom, groupPadding) + }) + } + } } } } @@ -468,7 +486,7 @@ return extraSpace.size > 0 ? extraSpace : undefined } - function layoutNodes(nodes: NodeDep[], nodeExtraSpace?: Map): (NodeDep & NodePos)[] { + function layoutNodes(nodes: NodeDep[], nodeExtraSpace?: Map): (NodeDep & NodePos)[] { let lastResult = lastNodes?.[2] if (lastResult && deepEqual(nodes, lastNodes?.[0]) && deepEqual(nodeExtraSpace, lastNodes?.[1])) { console.debug('layoutNodes', 'same nodes') @@ -780,9 +798,10 @@ ) : undefined - // Compute group header offsets for edges targeting topmost nodes in groups + // Compute group header/bottom offsets for edges targeting/sourcing group boundary nodes const groups = groupEditorContext?.groupEditor.getGroups() ?? [] const groupHeaderOffsets: Record = {} + const groupBottomOffsets: Record = {} if (groups.length > 0 && nodeExtraSpace) { // Use nodeExtraSpace directly - nodes with group headers already have the space // The offset for edges equals the group header + note portion of the topPadding @@ -811,17 +830,33 @@ : GROUP_HEADER_HEIGHT + noteHeight + GROUP_TOP_MARGIN groupHeaderOffsets[topNodeId] = Math.max(groupHeaderOffsets[topNodeId] ?? 0, offset) } + + // Bottom offset for uncollapsed groups + if (!isCollapsed) { + const bottomNodeId = [...sortedNodes].reverse().find((node) => + group.module_ids.includes(node.id) + )?.id + if (bottomNodeId) { + const groupPadding = 16 + groupBottomOffsets[bottomNodeId] = Math.max(groupBottomOffsets[bottomNodeId] ?? 0, groupPadding) + } + } } } // update nodes nodes = [...finalNodes, ...(noteNodesResult?.noteNodes ?? [])] - // Patch edges with group header offsets + // Patch edges with group header/bottom offsets const graphEdges = graph.edges.map((e) => { - const offset = groupHeaderOffsets[e.target] - if (offset) { - return { ...e, data: { ...e.data, groupHeaderOffset: offset } } + const headerOffset = groupHeaderOffsets[e.target] + const bottomOffset = groupBottomOffsets[e.source] + if (headerOffset || bottomOffset) { + return { ...e, data: { + ...e.data, + ...(headerOffset ? { groupHeaderOffset: headerOffset } : {}), + ...(bottomOffset ? { groupBottomOffset: bottomOffset } : {}) + } } } return e }) diff --git a/frontend/src/lib/components/graph/compoundLayout.ts b/frontend/src/lib/components/graph/compoundLayout.ts index a1c66df892..9c0afaca5d 100644 --- a/frontend/src/lib/components/graph/compoundLayout.ts +++ b/frontend/src/lib/components/graph/compoundLayout.ts @@ -238,15 +238,15 @@ function runSugiyama( function buildNodeSizes( nodeIds: string[], constants: LayoutConstants, - nodeExtraSpace?: Map + nodeExtraSpace?: Map ): Map | undefined { if (!nodeExtraSpace || nodeExtraSpace.size === 0) return undefined const sizes = new Map() for (const id of nodeIds) { const extra = nodeExtraSpace.get(id) - if (extra && (extra.top > 0 || extra.bottom > 0)) { + if (extra && (extra.top > 0 || extra.bottom > 0 || extra.left > 0 || extra.right > 0)) { sizes.set(id, { - width: constants.nodeWidth, + width: constants.nodeWidth + extra.left + extra.right, height: constants.nodeHeight + extra.top + extra.bottom }) } @@ -263,7 +263,7 @@ function layoutLevel( childrenMap: Map, containerDescendants: Map, depth: number = 0, - nodeExtraSpace?: Map + nodeExtraSpace?: Map ): LayoutResult { const positions = new Map() const nodeIdSet = new Set(nodeIds) @@ -482,7 +482,10 @@ function layoutLevel( // Position the head node at the top-center of the wrapper // Apply extra top padding so decorations above the head node have room const headExtra = nodeExtraSpace?.get(headId) - positions.set(headId, { x: wrapperPos.x, y: wrapperPos.y + (headExtra?.top ?? 0) }) + positions.set(headId, { + x: wrapperPos.x, + y: wrapperPos.y + (headExtra?.top ?? 0) + }) if (isBranch) { // Reuse cached branchWidths and totalWidth @@ -539,8 +542,8 @@ function layoutLevel( let maxY = -Infinity for (const [nid, pos] of positions) { const extra = nodeExtraSpace?.get(nid) - minX = Math.min(minX, pos.x - constants.nodeWidth / 2) - maxX = Math.max(maxX, pos.x + constants.nodeWidth / 2) + minX = Math.min(minX, pos.x - constants.nodeWidth / 2 - (extra?.left ?? 0)) + maxX = Math.max(maxX, pos.x + constants.nodeWidth / 2 + (extra?.right ?? 0)) // Account for top decoration space above the node minY = Math.min(minY, pos.y - (extra?.top ?? 0)) maxY = Math.max(maxY, pos.y + constants.nodeHeight + (extra?.bottom ?? 0)) @@ -572,14 +575,15 @@ function layoutLevel( * Takes the flat list of nodes and edges from graphBuilder and produces * absolute positions that account for compound structure (branches, loops). * - * nodeExtraSpace: per-node top/bottom padding that should be allocated in layout. + * nodeExtraSpace: per-node top/bottom/left/right padding that should be allocated in layout. * After layout, each node's y is shifted down by its top padding so decorations - * (assets, AI tools, group headers) have room above. + * (assets, AI tools, group headers) have room above. Left/right padding widens the + * column allocated to the node so neighbors are pushed further away. */ export function compoundLayout( nodes: { id: string; parentIds?: string[] }[], constants?: Partial, - nodeExtraSpace?: Map + nodeExtraSpace?: Map ): LayoutResult { const c: LayoutConstants = { nodeWidth: constants?.nodeWidth ?? NODE.width, diff --git a/frontend/src/lib/components/graph/renderers/edges/BaseEdge.svelte b/frontend/src/lib/components/graph/renderers/edges/BaseEdge.svelte index f0c90e1998..e51d37f218 100644 --- a/frontend/src/lib/components/graph/renderers/edges/BaseEdge.svelte +++ b/frontend/src/lib/components/graph/renderers/edges/BaseEdge.svelte @@ -43,15 +43,17 @@ suspendStatus?: Record shouldOffsetInsertBtnDueToAssetNode?: boolean groupHeaderOffset?: number + groupBottomOffset?: number } } = $props() + let adjustedSourceY = $derived(sourceY + (data?.groupBottomOffset ?? 0)) let adjustedTargetY = $derived(targetY - (data?.groupHeaderOffset ?? 0)) let [edgePath] = $derived( getBezierPath({ sourceX, - sourceY: adjustedTargetY - sourceY > 100 ? adjustedTargetY - 100 : sourceY, + sourceY: adjustedTargetY - adjustedSourceY > 100 ? adjustedTargetY - 100 : adjustedSourceY, sourcePosition, targetX, targetY: adjustedTargetY, @@ -61,8 +63,8 @@ ) let completeEdge = $derived( - adjustedTargetY - sourceY > 100 - ? `${edgePath} ${getStraightLinePath({ sourceX, sourceY, targetY: adjustedTargetY })}` + adjustedTargetY - adjustedSourceY > 100 + ? `${edgePath} ${getStraightLinePath({ sourceX, sourceY: adjustedSourceY, targetY: adjustedTargetY })}` : edgePath ) @@ -78,7 +80,7 @@ ) let centerY = $derived( - sourceY + + adjustedSourceY + 32 + (data.shouldOffsetInsertBtnDueToAssetNode && $showAssets ? NODE_WITH_WRITE_ASSET_Y_OFFSET : 0) )