From e2369103beedaefbdbe347c3000b844f7dc0b4ec Mon Sep 17 00:00:00 2001
From: Diego Imbert <diego@windmill.dev>
Date: Sun, 27 Sep 2026 03:32:19 +0200
Subject: [PATCH] fix: address simplified view editing review findings

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 .../graph/simplified/SimplifiedBox.svelte     | 18 ++++++++-------
 .../simplified/SimplifiedFlowGraph.svelte     | 22 ++++++++++++++-----
 .../graph/simplified/SimplifiedNode.svelte    | 16 ++++++++------
 .../graph/simplified/SimplifiedSlot.svelte    | 15 +++++++------
 .../graph/simplified/simplifiedLayout.ts      | 13 +++++++++--
 5 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/frontend/src/lib/components/graph/simplified/SimplifiedBox.svelte b/frontend/src/lib/components/graph/simplified/SimplifiedBox.svelte
index 24d641ee78..5956071d33 100644
--- a/frontend/src/lib/components/graph/simplified/SimplifiedBox.svelte
+++ b/frontend/src/lib/components/graph/simplified/SimplifiedBox.svelte
@@ -2,6 +2,7 @@
 	import { twMerge } from 'tailwind-merge'
 	import { GitBranchPlus } from 'lucide-svelte'
 	import FlowModuleIcon from '$lib/components/flows/FlowModuleIcon.svelte'
+	import { Button } from '$lib/components/common'
 	import StepContextMenu from './StepContextMenu.svelte'
 	import { getSimplifiedEditContext, useStepMenu } from './simplifiedContext'
 	import { getFlowRunStatusContext } from '../flowRunStatus.svelte'
@@ -96,17 +97,18 @@
 					<span class="font-mono text-hint truncate min-w-0">{header.detail}</span>
 				{/if}
 				{#if canAddBranch}
-					<button
-						type="button"
+					<Button
+						variant="subtle"
+						unifiedSize="2xs"
+						iconOnly
+						startIcon={{ icon: GitBranchPlus }}
 						title="Add branch"
-						class="nodrag shrink-0 rounded p-0.5 text-secondary hover:text-primary hover:bg-surface-hover"
-						onclick={(e) => {
-							e.stopPropagation()
+						btnClasses="nodrag shrink-0"
+						onClick={(e) => {
+							e?.stopPropagation()
 							edit?.eventHandlers?.newBranch(id)
 						}}
-					>
-						<GitBranchPlus size={12} />
-					</button>
+					/>
 				{/if}
 			</div>
 		</StepContextMenu>
diff --git a/frontend/src/lib/components/graph/simplified/SimplifiedFlowGraph.svelte b/frontend/src/lib/components/graph/simplified/SimplifiedFlowGraph.svelte
index 576de3270c..6a4b8b0f19 100644
--- a/frontend/src/lib/components/graph/simplified/SimplifiedFlowGraph.svelte
+++ b/frontend/src/lib/components/graph/simplified/SimplifiedFlowGraph.svelte
@@ -9,7 +9,7 @@
 	import '@xyflow/svelte/dist/base.css'
 	import type { FlowModule } from '$lib/gen'
 	import type { Snippet } from 'svelte'
-	import { layoutSimplifiedFlow } from './simplifiedLayout'
+	import { layoutSimplifiedFlow, type SimplifiedBoxData } from './simplifiedLayout'
 	import { setSimplifiedEditContext } from './simplifiedContext'
 	import type { FlowStructureNode } from '../flowStructure'
 	import type { GraphEventHandlers } from '../graphBuilder.svelte'
@@ -183,10 +183,22 @@
 				zoomOnDoubleClick={false}
 				deleteKey={null}
 				onnodeclick={({ node, event }) => {
-					// Adding to a multi-selection is xyflow's, synced by SelectionTool
-					if (event.shiftKey) return
-					// Boxes stand for their loop or branch step; placeholders are not selectable
-					if (node.type === 'simplifiedBox' || node.selectable) onSelect?.(node.id)
+					// A loop or branch box stands for its step, keyed by the module id; a user group's
+					// box (`group:<key>`) is no step, and its header handles its own clicks.
+					const isStepBox =
+						node.type === 'simplifiedBox' && (node.data as SimplifiedBoxData).kind !== 'group'
+					if (event.shiftKey) {
+						// Steps are added by xyflow and synced by SelectionTool. Boxes stay unselectable
+						// there, so a rectangle over a loop's steps does not grab the loop, and toggle here.
+						if (isStepBox && selectionOverlay && selectionManager) {
+							const ids = selectionManager.selectedIds
+							selectionManager.selectByIds(
+								ids.includes(node.id) ? ids.filter((i) => i !== node.id) : [...ids, node.id]
+							)
+						}
+						return
+					}
+					if (isStepBox || node.selectable) onSelect?.(node.id)
 				}}
 				onpaneclick={() => selectionManager?.clearSelection()}
 				{proOptions}
diff --git a/frontend/src/lib/components/graph/simplified/SimplifiedNode.svelte b/frontend/src/lib/components/graph/simplified/SimplifiedNode.svelte
index f00d067032..ec742ca931 100644
--- a/frontend/src/lib/components/graph/simplified/SimplifiedNode.svelte
+++ b/frontend/src/lib/components/graph/simplified/SimplifiedNode.svelte
@@ -3,6 +3,7 @@
 	import { twMerge } from 'tailwind-merge'
 	import { AlertTriangle, ArrowRightToLine, Filter, Flag, Trash2, X } from 'lucide-svelte'
 	import FlowModuleIcon from '$lib/components/flows/FlowModuleIcon.svelte'
+	import { Button } from '$lib/components/common'
 	import type { ContextMenuItem } from '$lib/components/common/contextmenu/ContextMenu.svelte'
 	import StepContextMenu from './StepContextMenu.svelte'
 	import { getNodeColorClasses } from '../util'
@@ -79,14 +80,15 @@
 		>
 			<span class="truncate">{data.title}</span>
 			{#if deleteBranch}
-				<button
-					type="button"
+				<Button
+					variant="subtle"
+					unifiedSize="2xs"
+					iconOnly
+					startIcon={{ icon: X }}
 					title="Delete branch"
-					class="nodrag shrink-0 opacity-0 group-hover/caption:opacity-100 text-secondary hover:text-red-500"
-					onclick={deleteBranch}
-				>
-					<X size={11} />
-				</button>
+					btnClasses="nodrag shrink-0 opacity-0 group-hover/caption:opacity-100 hover:text-red-500"
+					onClick={deleteBranch}
+				/>
 			{/if}
 		</div>
 	</StepContextMenu>
diff --git a/frontend/src/lib/components/graph/simplified/SimplifiedSlot.svelte b/frontend/src/lib/components/graph/simplified/SimplifiedSlot.svelte
index abf59ed68b..93cb3891f5 100644
--- a/frontend/src/lib/components/graph/simplified/SimplifiedSlot.svelte
+++ b/frontend/src/lib/components/graph/simplified/SimplifiedSlot.svelte
@@ -1,5 +1,6 @@
 <script lang="ts">
 	import { CircleDot } from 'lucide-svelte'
+	import { Button } from '$lib/components/common'
 	import InsertModulePopover from '$lib/components/flows/map/InsertModulePopover.svelte'
 	import InsertModuleButton from '$lib/components/flows/map/InsertModuleButton.svelte'
 	import { getGraphContext } from '../graphContext'
@@ -28,14 +29,14 @@
 <div class="nodrag nopan w-full h-full flex items-center justify-center">
 	{#if moving}
 		{#if !pasteBlocked}
-			<button
+			<Button
+				variant="default"
+				unifiedSize="2xs"
+				iconOnly
+				startIcon={{ icon: CircleDot }}
 				title="Paste module"
-				type="button"
-				class="w-5 h-5 flex items-center justify-center rounded-md bg-surface-secondary text-primary hover:text-accent"
-				onclick={() => edit?.eventHandlers?.insert(position)}
-			>
-				<CircleDot size={12} />
-			</button>
+				onClick={() => edit?.eventHandlers?.insert(position)}
+			/>
 		{/if}
 	{:else}
 		<InsertModulePopover
diff --git a/frontend/src/lib/components/graph/simplified/simplifiedLayout.ts b/frontend/src/lib/components/graph/simplified/simplifiedLayout.ts
index a53ab9e618..fc4b93389a 100644
--- a/frontend/src/lib/components/graph/simplified/simplifiedLayout.ts
+++ b/frontend/src/lib/components/graph/simplified/simplifiedLayout.ts
@@ -28,7 +28,8 @@ export const SIMPLIFIED = {
 	trunkGap: 88,
 	/** Column gap holding a junction, with a trunk on each side of it */
 	junctionGap: 120,
-	collapsedGroupWidth: 240,
+	/** The full graph's card width, which the step-icon row inside it is sized for */
+	collapsedGroupWidth: 275,
 	/** Extra box side padding and column gap in the editor, where "+" buttons sit there */
 	editRing: 12,
 	editGap: 32
@@ -603,7 +604,15 @@ function layoutNode(node: FlowStructureNode, layer: number, ancestors: string[],
 		const group = { key: node.id, value: node.group }
 		const noteHeight = ctx.groupNoteHeights[node.id] ?? 0
 		if (ctx.collapsedGroups.has(node.id)) {
-			const modules = (node.moduleIds ?? []).flatMap((id) => ctx.modules.get(id) ?? [])
+			// Its top-level steps, as the full graph's card takes them: containers carry their nested
+			// steps, and a nested group's steps stand in for it.
+			const topLevel = (items: FlowStructureNode[]): FlowModule[] =>
+				items.flatMap((c) =>
+					c.kind === 'group'
+						? topLevel(c.branches[0]?.children ?? [])
+						: (ctx.modules.get(c.id) ?? [])
+				)
+			const modules = topLevel(node.branches[0]?.children ?? [])
 			const data: SimplifiedNodeData = {
 				kind: 'collapsedGroup',
 				title: node.group.summary || 'Group',