mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 00:01:49 +00:00
fix: compute group bounding box using container descendants from layout
Thread a shared containerDescendants map through layoutLevel recursion instead of re-running detectGroups in a separate pass. This ensures group overlays properly encompass container modules (branchall, branchone, forloop, whileloop) in width by including their synthetic nodes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
2f794c45ec
commit
e73c7ed305
@@ -349,6 +349,7 @@
|
||||
}
|
||||
type NodePos = { position: { x: number; y: number } }
|
||||
let lastNodes: [NodeDep[], (NodeDep & NodePos)[]] | undefined = undefined
|
||||
let currentContainerDescendants: Map<string, string[]> = new Map()
|
||||
|
||||
function layoutNodes(nodes: NodeDep[]): (NodeDep & NodePos)[] {
|
||||
let lastResult = lastNodes?.[1]
|
||||
@@ -366,7 +367,7 @@
|
||||
}
|
||||
|
||||
// Run recursive compound layout
|
||||
const { positions, bbox } = compoundLayout(nodes, {
|
||||
const { positions, bbox, containerDescendants: layoutContainerDescendants } = compoundLayout(nodes, {
|
||||
nodeWidth: NODE.width,
|
||||
nodeHeight: NODE.height,
|
||||
gapH: NODE.gap.horizontal,
|
||||
@@ -383,6 +384,7 @@
|
||||
}
|
||||
}))
|
||||
|
||||
currentContainerDescendants = layoutContainerDescendants ?? new Map()
|
||||
lastNodes = [nodes, newNodes]
|
||||
return newNodes
|
||||
}
|
||||
@@ -1111,6 +1113,7 @@
|
||||
allNodes={nodesWithOffset as (Node & { type: string })[]}
|
||||
{editMode}
|
||||
{showNotes}
|
||||
containerDescendants={currentContainerDescendants}
|
||||
/>
|
||||
|
||||
<!-- SelectionTool for handling selection changes and filtering -->
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
<script lang="ts">
|
||||
import { ViewportPortal, type Node } from '@xyflow/svelte'
|
||||
import { calculateNodesBoundsWithOffset } from './util'
|
||||
import { calculateNodesBoundsWithOffset, expandWithContainerDescendants } from './util'
|
||||
import { getGroupEditorContext, GROUP_HEADER_HEIGHT, GROUP_TOP_MARGIN, type FlowGroup } from './groupEditor.svelte'
|
||||
import { NoteColor, NOTE_COLORS } from './noteColors'
|
||||
import GroupActionBar from './GroupActionBar.svelte'
|
||||
@@ -13,9 +13,10 @@
|
||||
allNodes: (Node & { type: string })[]
|
||||
editMode: boolean
|
||||
showNotes: boolean
|
||||
containerDescendants: Map<string, string[]>
|
||||
}
|
||||
|
||||
let { hoveredNodeId, allNodes, editMode, showNotes }: Props = $props()
|
||||
let { hoveredNodeId, allNodes, editMode, showNotes, containerDescendants }: Props = $props()
|
||||
|
||||
const groupEditorContext = getGroupEditorContext()
|
||||
|
||||
@@ -66,40 +67,52 @@
|
||||
return showNotes ? (noteHeights[groupId] ?? 0) : 0
|
||||
}
|
||||
|
||||
// Compute bounds for each group (no header card when expanded)
|
||||
function computeGroupBounds(group: FlowGroup) {
|
||||
if (group.module_ids.length === 0) return null
|
||||
const { minX, minY, maxX, maxY } = calculateNodesBoundsWithOffset(group.module_ids, allNodes)
|
||||
const padding = 16
|
||||
const noteHeight = getGroupNoteHeight(group.id)
|
||||
const topPadding = GROUP_HEADER_HEIGHT + noteHeight + GROUP_TOP_MARGIN
|
||||
const halfHeader = GROUP_HEADER_HEIGHT / 2
|
||||
return {
|
||||
x: minX - padding,
|
||||
y: minY - topPadding + halfHeader,
|
||||
width: maxX - minX + 2 * padding,
|
||||
height: maxY - minY + topPadding - halfHeader + padding,
|
||||
headerY: minY - topPadding
|
||||
// Pre-compute bounds for all groups reactively (tracks allNodes measured changes)
|
||||
let groupBoundsMap = $derived.by(() => {
|
||||
const map: Record<string, { x: number; y: number; width: number; height: number; headerY: number } | null> = {}
|
||||
for (const group of allGroups) {
|
||||
if (group.module_ids.length === 0) {
|
||||
map[group.id] = null
|
||||
continue
|
||||
}
|
||||
if (groupEditorContext?.groupEditor.isRuntimeCollapsed(group.id)) {
|
||||
// Collapsed group bounds
|
||||
const nodeId = `collapsed-group:${group.id}`
|
||||
const node = allNodes.find((n) => n.id === nodeId)
|
||||
if (!node) {
|
||||
map[group.id] = null
|
||||
continue
|
||||
}
|
||||
const width = node.measured?.width ?? 275
|
||||
const nodeHeight = node.measured?.height ?? 34
|
||||
const noteHeight = getGroupNoteHeight(group.id)
|
||||
const headerTotal = GROUP_HEADER_HEIGHT + noteHeight
|
||||
map[group.id] = {
|
||||
x: node.position.x,
|
||||
y: node.position.y - noteHeight,
|
||||
width,
|
||||
height: nodeHeight + noteHeight,
|
||||
headerY: node.position.y - headerTotal
|
||||
}
|
||||
} else {
|
||||
// Expanded group bounds — expand module_ids to include container descendants
|
||||
const expandedIds = expandWithContainerDescendants(group.module_ids, containerDescendants)
|
||||
const { minX, minY, maxX, maxY } = calculateNodesBoundsWithOffset(expandedIds, allNodes)
|
||||
const padding = 16
|
||||
const noteHeight = getGroupNoteHeight(group.id)
|
||||
const topPadding = GROUP_HEADER_HEIGHT + noteHeight + GROUP_TOP_MARGIN
|
||||
const halfHeader = GROUP_HEADER_HEIGHT / 2
|
||||
map[group.id] = {
|
||||
x: minX - padding,
|
||||
y: minY - topPadding + halfHeader,
|
||||
width: maxX - minX + 2 * padding,
|
||||
height: maxY - minY + topPadding - halfHeader + padding,
|
||||
headerY: minY - topPadding
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Compute bounds for a collapsed group (tight around the single collapsed node)
|
||||
function computeCollapsedGroupBounds(group: FlowGroup) {
|
||||
const nodeId = `collapsed-group:${group.id}`
|
||||
const node = allNodes.find((n) => n.id === nodeId)
|
||||
if (!node) return null
|
||||
const width = node.measured?.width ?? 275
|
||||
const nodeHeight = node.measured?.height ?? 34
|
||||
const noteHeight = getGroupNoteHeight(group.id)
|
||||
const headerTotal = GROUP_HEADER_HEIGHT + noteHeight
|
||||
return {
|
||||
x: node.position.x,
|
||||
y: node.position.y - noteHeight,
|
||||
width,
|
||||
height: nodeHeight + noteHeight,
|
||||
headerY: node.position.y - headerTotal
|
||||
}
|
||||
}
|
||||
return map
|
||||
})
|
||||
|
||||
function getOutlineColorClass(color?: string, hovered?: boolean): string {
|
||||
const config = NOTE_COLORS[(color as NoteColor) ?? NoteColor.BLUE] ?? NOTE_COLORS[NoteColor.BLUE]
|
||||
@@ -152,7 +165,7 @@
|
||||
|
||||
{#each allGroups as group (group.id)}
|
||||
{#if groupEditorContext?.groupEditor.isRuntimeCollapsed(group.id)}
|
||||
{@const bounds = computeCollapsedGroupBounds(group)}
|
||||
{@const bounds = groupBoundsMap[group.id]}
|
||||
{#if bounds}
|
||||
<!-- Collapsed: bounding box background + border on sides/bottom only (behind nodes) -->
|
||||
<ViewportPortal target="back">
|
||||
@@ -220,7 +233,7 @@
|
||||
</ViewportPortal>
|
||||
{/if}
|
||||
{:else}
|
||||
{@const bounds = computeGroupBounds(group)}
|
||||
{@const bounds = groupBoundsMap[group.id]}
|
||||
{#if bounds}
|
||||
<!-- Uncollapsed: bounding box background + outline (behind nodes) -->
|
||||
<ViewportPortal target="back">
|
||||
|
||||
@@ -27,6 +27,7 @@ type LayoutResult = {
|
||||
positions: Map<string, { x: number; y: number }>
|
||||
bbox: { width: number; height: number }
|
||||
contentMinX: number
|
||||
containerDescendants?: Map<string, string[]>
|
||||
}
|
||||
|
||||
const LOOP_INDENT = 25
|
||||
@@ -237,6 +238,7 @@ function layoutLevel(
|
||||
allNodes: Map<string, LayoutNode>,
|
||||
constants: LayoutConstants,
|
||||
childrenMap: Map<string, string[]>,
|
||||
containerDescendants: Map<string, string[]>,
|
||||
depth: number = 0
|
||||
): LayoutResult {
|
||||
const positions = new Map<string, { x: number; y: number }>()
|
||||
@@ -296,6 +298,15 @@ function layoutLevel(
|
||||
}
|
||||
}
|
||||
|
||||
// Populate containerDescendants for each top-level container group
|
||||
for (const group of topLevelGroups) {
|
||||
const owned: string[] = [group.endId]
|
||||
for (const branch of group.branches) {
|
||||
owned.push(branch.labelId, ...branch.innerIds)
|
||||
}
|
||||
containerDescendants.set(group.headId, owned)
|
||||
}
|
||||
|
||||
// Step 2-3: Recursively lay out each group and compute wrapper sizes
|
||||
type GroupLayout = {
|
||||
group: CompoundGroup
|
||||
@@ -322,7 +333,14 @@ function layoutLevel(
|
||||
const branchNodeIds = [branch.labelId, ...branch.innerIds]
|
||||
|
||||
// Find sub-groups within this branch
|
||||
const result = layoutLevel(branchNodeIds, allNodes, constants, childrenMap, depth + 1)
|
||||
const result = layoutLevel(
|
||||
branchNodeIds,
|
||||
allNodes,
|
||||
constants,
|
||||
childrenMap,
|
||||
containerDescendants,
|
||||
depth + 1
|
||||
)
|
||||
|
||||
branchLayouts.push({
|
||||
labelId: branch.labelId,
|
||||
@@ -528,7 +546,8 @@ export function compoundLayout(
|
||||
}
|
||||
|
||||
const nodeIds = nodes.map((n) => n.id)
|
||||
const result = layoutLevel(nodeIds, allNodes, c, childrenMap)
|
||||
const containerDescendants = new Map<string, string[]>()
|
||||
const result = layoutLevel(nodeIds, allNodes, c, childrenMap, containerDescendants)
|
||||
|
||||
// Shift positions so minX=0 (left-aligned).
|
||||
// FlowGraphV2 centers with: xCenter = viewport/2 - bbox.width/2
|
||||
@@ -551,5 +570,5 @@ export function compoundLayout(
|
||||
)
|
||||
}
|
||||
|
||||
return result
|
||||
return { ...result, containerDescendants }
|
||||
}
|
||||
|
||||
@@ -227,6 +227,7 @@ export function calculateNodesBoundsWithOffset(
|
||||
id: string
|
||||
position: { x: number; y: number }
|
||||
type: string
|
||||
measured?: { width?: number; height?: number }
|
||||
}>
|
||||
): {
|
||||
minX: number
|
||||
@@ -239,11 +240,13 @@ export function calculateNodesBoundsWithOffset(
|
||||
|
||||
return nodesToCalculate.reduce(
|
||||
(acc, node) => {
|
||||
const w = node.measured?.width ?? NODE.width
|
||||
const h = node.measured?.height ?? NODE.height
|
||||
return {
|
||||
minX: Math.min(acc.minX, node.position.x),
|
||||
minY: Math.min(acc.minY, node.position.y),
|
||||
maxX: Math.max(acc.maxX, node.position.x + NODE.width),
|
||||
maxY: Math.max(acc.maxY, node.position.y + NODE.height)
|
||||
maxX: Math.max(acc.maxX, node.position.x + w),
|
||||
maxY: Math.max(acc.maxY, node.position.y + h)
|
||||
}
|
||||
},
|
||||
{
|
||||
@@ -255,6 +258,33 @@ export function calculateNodesBoundsWithOffset(
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Expand a list of node IDs to include all structurally-owned descendants of containers.
|
||||
* Uses the containerDescendants map (container head → owned node IDs) built by compoundLayout,
|
||||
* which correctly bounds traversal at container end nodes (unlike the flow-topology childrenMap
|
||||
* which would leak past container boundaries into downstream nodes).
|
||||
*/
|
||||
export function expandWithContainerDescendants(
|
||||
nodeIds: string[],
|
||||
containerDescendants: Map<string, string[]>
|
||||
): string[] {
|
||||
const result = new Set<string>(nodeIds)
|
||||
const queue = [...nodeIds]
|
||||
let qi = 0
|
||||
while (qi < queue.length) {
|
||||
const current = queue[qi++]
|
||||
const owned = containerDescendants.get(current)
|
||||
if (!owned) continue
|
||||
for (const id of owned) {
|
||||
if (!result.has(id)) {
|
||||
result.add(id)
|
||||
queue.push(id)
|
||||
}
|
||||
}
|
||||
}
|
||||
return Array.from(result)
|
||||
}
|
||||
|
||||
/**
|
||||
* Find all nodes related to the given node IDs, including expanded subflow nodes
|
||||
* @param targetNodeIds - Array of node IDs to find related nodes for
|
||||
@@ -267,17 +297,19 @@ function getAllRelatedSubflowNodes(
|
||||
id: string
|
||||
position: { x: number; y: number }
|
||||
type: string
|
||||
measured?: { width?: number; height?: number }
|
||||
}>
|
||||
): Array<{
|
||||
id: string
|
||||
position: { x: number; y: number }
|
||||
measured?: { width?: number; height?: number }
|
||||
}> {
|
||||
const relatedNodeIds = new Set<string>()
|
||||
|
||||
// Add original target nodes
|
||||
targetNodeIds.forEach((id) => relatedNodeIds.add(id))
|
||||
|
||||
// For each target node, check if it's a subflow and find expanded nodes
|
||||
// For each target node, check if it's a subflow or container and find child nodes
|
||||
targetNodeIds.forEach((nodeId) => {
|
||||
// Find nodes like "subflow:{nodeId}:*"
|
||||
const subflowNodes = allNodes.filter(
|
||||
|
||||
Reference in New Issue
Block a user