From 9b7626f6ff9ceb86cc2c2543018615f047e24b69 Mon Sep 17 00:00:00 2001 From: hsiegeln <37154749+hsiegeln@users.noreply.github.com> Date: Fri, 27 Mar 2026 16:33:24 +0100 Subject: [PATCH] fix: diagram rendering improvements - Recursive compound rendering: CompoundNode checks if children are themselves compound types (WHEN inside CHOICE) and renders them recursively. Added EIP_WHEN, EIP_OTHERWISE, DO_CATCH, DO_FINALLY to frontend COMPOUND_TYPES. - Edge z-ordering: edges are distributed to their containing compound and rendered after the background rect, so they're not hidden behind compound containers. - Error section sizing: normalize error handler node coordinates to start at (0,0), compute red tint background height from actual content with symmetric padding for vertical centering. - Toolbar as HTML overlay: moved from SVG foreignObject to absolute- positioned HTML div so it stays fixed size at any zoom level. Uses design system tokens for consistent styling. - Zoom: replaced viewBox approach with CSS transform on content group. Default zoom is 100% anchored top-left. Fit-to-view still available via button. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ProcessDiagram/CompoundNode.tsx | 102 +++++++++++++----- .../ProcessDiagram/ErrorSection.tsx | 101 ++++++++++------- .../components/ProcessDiagram/NodeToolbar.tsx | 86 +++++---------- .../ProcessDiagram/ProcessDiagram.module.css | 36 +++++++ .../ProcessDiagram/ProcessDiagram.tsx | 93 +++++++++++----- .../components/ProcessDiagram/node-colors.ts | 4 +- .../ProcessDiagram/useDiagramData.ts | 40 +++++-- .../components/ProcessDiagram/useZoomPan.ts | 40 +++---- 8 files changed, 326 insertions(+), 176 deletions(-) diff --git a/ui/src/components/ProcessDiagram/CompoundNode.tsx b/ui/src/components/ProcessDiagram/CompoundNode.tsx index 63ea6ca9..42200f2e 100644 --- a/ui/src/components/ProcessDiagram/CompoundNode.tsx +++ b/ui/src/components/ProcessDiagram/CompoundNode.tsx @@ -1,13 +1,19 @@ -import type { DiagramNode as DiagramNodeType } from '../../api/queries/diagrams'; +import type { DiagramNode as DiagramNodeType, DiagramEdge as DiagramEdgeType } from '../../api/queries/diagrams'; import type { NodeConfig } from './types'; -import { colorForType } from './node-colors'; +import { colorForType, isCompoundType } from './node-colors'; import { DiagramNode } from './DiagramNode'; +import { DiagramEdge } from './DiagramEdge'; const HEADER_HEIGHT = 22; const CORNER_RADIUS = 4; interface CompoundNodeProps { node: DiagramNodeType; + /** All edges for this section — compound filters to its own internal edges */ + edges: DiagramEdgeType[]; + /** Absolute offset of the nearest compound ancestor (for coordinate adjustment) */ + parentX?: number; + parentY?: number; selectedNodeId?: string; hoveredNodeId: string | null; nodeConfigs?: Map; @@ -17,17 +23,28 @@ interface CompoundNodeProps { } export function CompoundNode({ - node, selectedNodeId, hoveredNodeId, nodeConfigs, + node, edges, parentX = 0, parentY = 0, + selectedNodeId, hoveredNodeId, nodeConfigs, onNodeClick, onNodeEnter, onNodeLeave, }: CompoundNodeProps) { - const x = node.x ?? 0; - const y = node.y ?? 0; + const x = (node.x ?? 0) - parentX; + const y = (node.y ?? 0) - parentY; + const absX = node.x ?? 0; + const absY = node.y ?? 0; const w = node.width ?? 200; const h = node.height ?? 100; const color = colorForType(node.type); const typeName = node.type?.replace(/^EIP_/, '').replace(/_/g, ' ') ?? ''; const label = node.label ? `${typeName}: ${node.label}` : typeName; + // Collect all descendant node IDs to filter edges that belong inside this compound + const descendantIds = new Set(); + collectIds(node.children ?? [], descendantIds); + + const internalEdges = edges.filter( + e => descendantIds.has(e.sourceId) && descendantIds.has(e.targetId), + ); + return ( {/* Container body */} @@ -58,25 +75,62 @@ export function CompoundNode({ {label} - {/* Children nodes (positioned relative to compound) */} - {node.children?.map(child => ( - child.id && onNodeClick(child.id)} - onMouseEnter={() => child.id && onNodeEnter(child.id)} - onMouseLeave={onNodeLeave} - /> - ))} + {/* Internal edges (rendered after background, before children) */} + + {internalEdges.map((edge, i) => ( + [p[0] - absX, p[1] - absY]), + }} + /> + ))} + + + {/* Children — recurse into compound children, render leaves as DiagramNode */} + {node.children?.map(child => { + if (isCompoundType(child.type) && child.children && child.children.length > 0) { + return ( + + ); + } + return ( + child.id && onNodeClick(child.id)} + onMouseEnter={() => child.id && onNodeEnter(child.id)} + onMouseLeave={onNodeLeave} + /> + ); + })} ); } + +function collectIds(nodes: DiagramNodeType[], set: Set) { + for (const n of nodes) { + if (n.id) set.add(n.id); + if (n.children) collectIds(n.children, set); + } +} diff --git a/ui/src/components/ProcessDiagram/ErrorSection.tsx b/ui/src/components/ProcessDiagram/ErrorSection.tsx index 580da0b5..fe3bfdf1 100644 --- a/ui/src/components/ProcessDiagram/ErrorSection.tsx +++ b/ui/src/components/ProcessDiagram/ErrorSection.tsx @@ -1,10 +1,15 @@ +import { useMemo } from 'react'; import type { DiagramSection } from './types'; import type { NodeConfig } from './types'; +import type { DiagramNode as DiagramNodeType } from '../../api/queries/diagrams'; import { DiagramEdge } from './DiagramEdge'; import { DiagramNode } from './DiagramNode'; import { CompoundNode } from './CompoundNode'; import { isCompoundType } from './node-colors'; +const CONTENT_PADDING_Y = 20; +const CONTENT_PADDING_LEFT = 12; + interface ErrorSectionProps { section: DiagramSection; totalWidth: number; @@ -20,8 +25,29 @@ export function ErrorSection({ section, totalWidth, selectedNodeId, hoveredNodeId, nodeConfigs, onNodeClick, onNodeEnter, onNodeLeave, }: ErrorSectionProps) { + const boxHeight = useMemo(() => { + let maxY = 0; + for (const n of section.nodes) { + const bottom = (n.y ?? 0) + (n.height ?? 40); + if (bottom > maxY) maxY = bottom; + if (n.children) { + for (const c of n.children) { + const cb = (c.y ?? 0) + (c.height ?? 40); + if (cb > maxY) maxY = cb; + } + } + } + // Content height + top padding + bottom padding (to vertically center) + return maxY + CONTENT_PADDING_Y * 2; + }, [section.nodes]); + return ( + {/* Section label */} + + {section.label} + + {/* Divider line */} - {/* Section label */} - - {section.label} - - - {/* Subtle red tint background */} + {/* Subtle red tint background — sized to actual content */} - {/* Edges */} - - {section.edges.map((edge, i) => ( - - ))} - + {/* Content group with margin from top-left */} + + {/* Edges */} + + {section.edges.map((edge, i) => ( + + ))} + - {/* Nodes */} - - {section.nodes.map(node => { - if (isCompoundType(node.type) && node.children && node.children.length > 0) { + {/* Nodes */} + + {section.nodes.map(node => { + if (isCompoundType(node.type) && node.children && node.children.length > 0) { + return ( + + ); + } return ( - node.id && onNodeClick(node.id)} + onMouseEnter={() => node.id && onNodeEnter(node.id)} + onMouseLeave={onNodeLeave} /> ); - } - return ( - node.id && onNodeClick(node.id)} - onMouseEnter={() => node.id && onNodeEnter(node.id)} - onMouseLeave={onNodeLeave} - /> - ); - })} + })} + ); diff --git a/ui/src/components/ProcessDiagram/NodeToolbar.tsx b/ui/src/components/ProcessDiagram/NodeToolbar.tsx index ccfb2651..6dc81df3 100644 --- a/ui/src/components/ProcessDiagram/NodeToolbar.tsx +++ b/ui/src/components/ProcessDiagram/NodeToolbar.tsx @@ -1,82 +1,50 @@ import { useCallback, useRef, useState } from 'react'; import type { NodeAction } from './types'; +import styles from './ProcessDiagram.module.css'; -const TOOLBAR_HEIGHT = 28; -const TOOLBAR_WIDTH = 140; const HIDE_DELAY = 150; interface NodeToolbarProps { nodeId: string; - nodeX: number; - nodeY: number; - nodeWidth: number; + /** Screen-space position (already transformed by zoom/pan) */ + screenX: number; + screenY: number; onAction: (nodeId: string, action: NodeAction) => void; onMouseEnter: () => void; onMouseLeave: () => void; } -const ACTIONS: { label: string; icon: string; action: NodeAction; title: string }[] = [ - { label: 'Inspect', icon: '\uD83D\uDD0D', action: 'inspect', title: 'Inspect node' }, - { label: 'Trace', icon: 'T', action: 'toggle-trace', title: 'Toggle tracing' }, - { label: 'Tap', icon: '\u270E', action: 'configure-tap', title: 'Configure tap' }, - { label: 'More', icon: '\u22EF', action: 'copy-id', title: 'Copy processor ID' }, +const ACTIONS: { icon: string; action: NodeAction; title: string }[] = [ + { icon: '\uD83D\uDD0D', action: 'inspect', title: 'Inspect' }, + { icon: 'T', action: 'toggle-trace', title: 'Toggle tracing' }, + { icon: '\u270E', action: 'configure-tap', title: 'Configure tap' }, + { icon: '\u22EF', action: 'copy-id', title: 'Copy ID' }, ]; export function NodeToolbar({ - nodeId, nodeX, nodeY, nodeWidth, onAction, onMouseEnter, onMouseLeave, + nodeId, screenX, screenY, onAction, onMouseEnter, onMouseLeave, }: NodeToolbarProps) { - const x = nodeX + (nodeWidth - TOOLBAR_WIDTH) / 2; - const y = nodeY - TOOLBAR_HEIGHT - 6; - return ( - -
- {ACTIONS.map(a => ( - - ))} -
- + {ACTIONS.map(a => ( + + ))} + ); } diff --git a/ui/src/components/ProcessDiagram/ProcessDiagram.module.css b/ui/src/components/ProcessDiagram/ProcessDiagram.module.css index 9211045a..bdff3294 100644 --- a/ui/src/components/ProcessDiagram/ProcessDiagram.module.css +++ b/ui/src/components/ProcessDiagram/ProcessDiagram.module.css @@ -77,3 +77,39 @@ text-align: center; font-variant-numeric: tabular-nums; } + +.nodeToolbar { + position: absolute; + display: flex; + align-items: center; + gap: 2px; + padding: 3px 4px; + background: var(--bg-surface, #FFFFFF); + border: 1px solid var(--border, #E4DFD8); + border-radius: var(--radius-sm, 5px); + box-shadow: var(--shadow-lg, 0 4px 16px rgba(44, 37, 32, 0.10)); + transform: translate(-50%, -100%); + margin-top: -6px; + z-index: 10; + pointer-events: auto; +} + +.nodeToolbarBtn { + display: flex; + align-items: center; + justify-content: center; + width: 26px; + height: 26px; + border: none; + background: transparent; + color: var(--text-secondary, #5C5347); + font-size: 12px; + cursor: pointer; + border-radius: var(--radius-sm, 5px); + padding: 0; +} + +.nodeToolbarBtn:hover { + background: var(--bg-hover, #F5F0EA); + color: var(--text-primary, #1A1612); +} diff --git a/ui/src/components/ProcessDiagram/ProcessDiagram.tsx b/ui/src/components/ProcessDiagram/ProcessDiagram.tsx index bfec2b0b..0d22911b 100644 --- a/ui/src/components/ProcessDiagram/ProcessDiagram.tsx +++ b/ui/src/components/ProcessDiagram/ProcessDiagram.tsx @@ -33,10 +33,10 @@ export function ProcessDiagram({ const contentWidth = totalWidth + PADDING * 2; const contentHeight = totalHeight + PADDING * 2; - // Fit to view on first data load + // Reset to 100% at top-left on first data load useEffect(() => { if (totalWidth > 0 && totalHeight > 0) { - zoom.fitToView(contentWidth, contentHeight); + zoom.resetView(); } }, [totalWidth, totalHeight]); // eslint-disable-line react-hooks/exhaustive-deps @@ -98,7 +98,6 @@ export function ProcessDiagram({ > - - {/* Main section edges */} + + {/* Main section top-level edges (not inside compounds) */} - {mainSection.edges.map((edge, i) => ( + {mainSection.edges.filter(e => topLevelEdge(e, mainSection.nodes)).map((edge, i) => ( ))} @@ -136,6 +135,7 @@ export function ProcessDiagram({ - {/* Toolbar for hovered node */} - {toolbar.hoveredNodeId && onNodeAction && (() => { - const hNode = findNodeById(sections, toolbar.hoveredNodeId!); - if (!hNode) return null; - return ( - - ); - })()} + {/* Toolbar rendered as HTML overlay below */} {/* Error handler sections */} {errorSections.map((section, i) => ( @@ -194,6 +179,27 @@ export function ProcessDiagram({ + {/* Node toolbar — HTML overlay, fixed size regardless of zoom */} + {toolbar.hoveredNodeId && onNodeAction && (() => { + const hNode = findNodeById(sections, toolbar.hoveredNodeId!); + if (!hNode) return null; + // Convert SVG coordinates to screen-space using zoom transform + const nodeCenter = (hNode.x ?? 0) + (hNode.width ?? 160) / 2; + const nodeTop = hNode.y ?? 0; + const screenX = nodeCenter * zoom.state.scale + zoom.state.translateX; + const screenY = nodeTop * zoom.state.scale + zoom.state.translateY; + return ( + + ); + })()} + c.id === nodeId); - if (child) return child; + const found = findInChildren(node.children, nodeId); + if (found) return found; } } } return undefined; } + +function findInChildren( + nodes: import('../../api/queries/diagrams').DiagramNode[], + nodeId: string, +): import('../../api/queries/diagrams').DiagramNode | undefined { + for (const n of nodes) { + if (n.id === nodeId) return n; + if (n.children) { + const found = findInChildren(n.children, nodeId); + if (found) return found; + } + } + return undefined; +} + +/** Returns true if the edge connects two top-level nodes (not inside any compound). */ +function topLevelEdge( + edge: import('../../api/queries/diagrams').DiagramEdge, + nodes: import('../../api/queries/diagrams').DiagramNode[], +): boolean { + // Collect all IDs that are children of compound nodes (at any depth) + const compoundChildIds = new Set(); + for (const n of nodes) { + if (n.children && n.children.length > 0) { + collectDescendantIds(n.children, compoundChildIds); + } + } + return !compoundChildIds.has(edge.sourceId) && !compoundChildIds.has(edge.targetId); +} + +function collectDescendantIds( + nodes: import('../../api/queries/diagrams').DiagramNode[], + set: Set, +) { + for (const n of nodes) { + if (n.id) set.add(n.id); + if (n.children) collectDescendantIds(n.children, set); + } +} diff --git a/ui/src/components/ProcessDiagram/node-colors.ts b/ui/src/components/ProcessDiagram/node-colors.ts index 2a521b76..bebbd028 100644 --- a/ui/src/components/ProcessDiagram/node-colors.ts +++ b/ui/src/components/ProcessDiagram/node-colors.ts @@ -56,7 +56,9 @@ const TYPE_MAP: Record = { }; const COMPOUND_TYPES = new Set([ - 'EIP_CHOICE', 'EIP_SPLIT', 'TRY_CATCH', 'DO_TRY', + 'EIP_CHOICE', 'EIP_WHEN', 'EIP_OTHERWISE', + 'EIP_SPLIT', 'TRY_CATCH', + 'DO_TRY', 'DO_CATCH', 'DO_FINALLY', 'EIP_LOOP', 'EIP_MULTICAST', 'EIP_AGGREGATE', 'ON_EXCEPTION', 'ERROR_HANDLER', ]); diff --git a/ui/src/components/ProcessDiagram/useDiagramData.ts b/ui/src/components/ProcessDiagram/useDiagramData.ts index ca0547a7..0abe2721 100644 --- a/ui/src/components/ProcessDiagram/useDiagramData.ts +++ b/ui/src/components/ProcessDiagram/useDiagramData.ts @@ -56,27 +56,43 @@ export function useDiagramData( ]; let currentY = mainBounds.maxY + SECTION_GAP; + let maxWidth = mainBounds.maxX; for (const es of errorSections) { + const errorBounds = computeBounds(es.nodes); + const offX = errorBounds.minX; + const offY = errorBounds.minY; + + // Normalize node coordinates relative to the section's own origin + const shiftedNodes = shiftNodes(es.nodes, offX, offY); + const errorNodeIds = new Set(); collectNodeIds(es.nodes, errorNodeIds); - const errorEdges = allEdges.filter( - e => errorNodeIds.has(e.sourceId) && errorNodeIds.has(e.targetId), - ); + + // Shift edge points too + const errorEdges = allEdges + .filter(e => errorNodeIds.has(e.sourceId) && errorNodeIds.has(e.targetId)) + .map(e => ({ + ...e, + points: e.points.map(p => [p[0] - offX, p[1] - offY]), + })); + + const sectionHeight = errorBounds.maxY - errorBounds.minY; + const sectionWidth = errorBounds.maxX - errorBounds.minX; sections.push({ label: es.label, - nodes: es.nodes, + nodes: shiftedNodes, edges: errorEdges, offsetY: currentY, variant: 'error', }); - const errorBounds = computeBounds(es.nodes); - currentY += (errorBounds.maxY - errorBounds.minY) + SECTION_GAP; + currentY += sectionHeight + SECTION_GAP; + if (sectionWidth > maxWidth) maxWidth = sectionWidth; } - const totalWidth = layout.width ?? mainBounds.maxX; + const totalWidth = Math.max(layout.width ?? 0, mainBounds.maxX, maxWidth); const totalHeight = currentY; return { sections, totalWidth, totalHeight }; @@ -85,6 +101,16 @@ export function useDiagramData( return { ...result, isLoading, error }; } +/** Shift all node coordinates by subtracting an offset, recursively. */ +function shiftNodes(nodes: DiagramNode[], offX: number, offY: number): DiagramNode[] { + return nodes.map(n => ({ + ...n, + x: (n.x ?? 0) - offX, + y: (n.y ?? 0) - offY, + children: n.children ? shiftNodes(n.children, offX, offY) : undefined, + })); +} + function collectNodeIds(nodes: DiagramNode[], set: Set) { for (const n of nodes) { if (n.id) set.add(n.id); diff --git a/ui/src/components/ProcessDiagram/useZoomPan.ts b/ui/src/components/ProcessDiagram/useZoomPan.ts index ddc3aba7..ecef94dc 100644 --- a/ui/src/components/ProcessDiagram/useZoomPan.ts +++ b/ui/src/components/ProcessDiagram/useZoomPan.ts @@ -9,7 +9,7 @@ interface ZoomPanState { const MIN_SCALE = 0.25; const MAX_SCALE = 4.0; const ZOOM_STEP = 0.15; -const FIT_PADDING = 40; +const FIT_PADDING = 20; export function useZoomPan() { const [state, setState] = useState({ @@ -23,16 +23,8 @@ export function useZoomPan() { const clampScale = (s: number) => Math.min(MAX_SCALE, Math.max(MIN_SCALE, s)); - const viewBox = useCallback( - (contentWidth: number, contentHeight: number) => { - const vw = contentWidth / state.scale; - const vh = contentHeight / state.scale; - const vx = -state.translateX / state.scale; - const vy = -state.translateY / state.scale; - return `${vx} ${vy} ${vw} ${vh}`; - }, - [state], - ); + /** Returns the CSS transform string for the content element. */ + const transform = `translate(${state.translateX}px, ${state.translateY}px) scale(${state.scale})`; const onWheel = useCallback( (e: React.WheelEvent) => { @@ -40,20 +32,18 @@ export function useZoomPan() { const direction = e.deltaY < 0 ? 1 : -1; const factor = 1 + direction * ZOOM_STEP; - // Capture rect and cursor position before entering setState updater, - // because React clears e.currentTarget after the event handler returns. const rect = (e.currentTarget as SVGSVGElement).getBoundingClientRect(); - const clientX = e.clientX; - const clientY = e.clientY; + const cursorX = e.clientX - rect.left; + const cursorY = e.clientY - rect.top; setState(prev => { const newScale = clampScale(prev.scale * factor); - const cursorX = clientX - rect.left; - const cursorY = clientY - rect.top; const scaleRatio = newScale / prev.scale; - const newTx = cursorX - scaleRatio * (cursorX - prev.translateX); - const newTy = cursorY - scaleRatio * (cursorY - prev.translateY); - return { scale: newScale, translateX: newTx, translateY: newTy }; + return { + scale: newScale, + translateX: cursorX - scaleRatio * (cursorX - prev.translateX), + translateY: cursorY - scaleRatio * (cursorY - prev.translateY), + }; }); }, [], @@ -61,7 +51,6 @@ export function useZoomPan() { const onPointerDown = useCallback( (e: React.PointerEvent) => { - // Only pan on background click (not on nodes) if ((e.target as Element).closest('[data-node-id]')) return; isPanning.current = true; panStart.current = { x: e.clientX - state.translateX, y: e.clientY - state.translateY }; @@ -122,16 +111,20 @@ export function useZoomPan() { }); }, []); + const resetView = useCallback(() => { + setState({ scale: 1, translateX: FIT_PADDING, translateY: FIT_PADDING }); + }, []); + const fitToView = useCallback( (contentWidth: number, contentHeight: number) => { const container = containerRef.current; if (!container) return; const cw = container.clientWidth - FIT_PADDING * 2; const ch = container.clientHeight - FIT_PADDING * 2; + if (contentWidth <= 0 || contentHeight <= 0) return; const scaleX = cw / contentWidth; const scaleY = ch / contentHeight; const newScale = clampScale(Math.min(scaleX, scaleY)); - // Anchor to top-left with padding setState({ scale: newScale, translateX: FIT_PADDING, translateY: FIT_PADDING }); }, [], @@ -161,7 +154,8 @@ export function useZoomPan() { return { state, containerRef, - viewBox, + transform, + resetView, onWheel, onPointerDown, onPointerMove,