feat(ui): add read-only workflow visualization with auto-layout#1114
feat(ui): add read-only workflow visualization with auto-layout#1114geoffjay wants to merge 3 commits intoissue-1101from
Conversation
|
This change is part of the following stack:
Change managed by git-spice. |
geoffjay
left a comment
There was a problem hiding this comment.
Review: feat(ui): add read-only workflow visualization with auto-layout
LGTM — cannot self-approve, posting as comment.
Stack note
This branch (issue-1102) is stacked on PR #1113 (issue-1101: "feat(ui): add workflow builder page with routing and persistence"), which also carries review-agent and has not yet been reviewed. Ideally #1113 is reviewed and merged first. The code in this PR is sound on its own, but (needs push) shown by git-spice was stale — origin/issue-1102 is fully current with both commits pushed.
What was verified
layout.ts — autoLayout algorithm
- Constants cross-checked against all test assertions:
TRIGGER_X=80,AGENT_X=380 (80+300),nodeSpacing=120. Every expected pixel value in the test suite is arithmetically correct. ✓ - Agent-centering formula:
agentCenterSlot = groupStart + (groupSize - 1) / 2— correct midpoint of the trigger group. ✓ - Gap slot advance:
nextSlot = groupStart + groupSize + 1— one blank row between groups. ✓ - Isolated agent (no triggers):
groupSize = Math.max(0,1) = 1, agent placed atgroupStart * nodeSpacing, no trigger placed. Correct. ✓ - Duplicate-edge deduplication:
!list.includes(edge.source)guard. ✓ othernode passthrough: preserved with original positions. ✓- LR and TB implementations are structurally symmetric. ✓
WorkflowCanvas.tsx — readOnly prop
- All five interaction flags correctly gated:
nodesDraggable={!readOnly},nodesConnectable={!readOnly},deleteKeyCode={readOnly ? null : "Delete"},snapToGrid={!readOnly}. ✓ - Zoom/pan always active (
panOnDrag,zoomOnScrollunconditional). ✓ readonly-badgewithpointer-events-noneso it doesn't intercept canvas clicks. ✓data-readonly={readOnly}attribute — allows CSS targeting and test assertions. ✓
WorkflowDetail.tsx — view toggle
useEffectonly runs whenview === "graph"— no wasted computation in default details view. ✓autoLayoutcalled once per view switch, result stored in state (static read-only canvas). ✓localStoragewrite wrapped in try/catch (private browsing / quota exceeded). ✓- No-op handlers for
onNodesChange/onEdgesChange/onConnect— never triggered becausereadOnlydisables all mutations. ✓
Tests
- 5 new
readOnly modetests inWorkflowCanvas.test.tsx— badge presence/absence, badge text,data-readonlyattribute. ✓ - 13 layout tests covering: empty graph, single pair, centering, multiple groups, TB direction, shared agent (3 triggers), duplicate edges. All assertions match the implementation constants. ✓
useReactFlowstub added toWorkflowBuilder.test.tsxpreventsFitViewOnLoadfrom crashing tests. ✓
Non-blocking suggestions
-
FitViewOnLoaddep array comment is inaccurate (WorkflowCanvas.tsx~line 202):
The comment says "We only want this to run once on mount" but the dep is[readOnly], meaning it also fires whenreadOnlychanges. In practicereadOnlyis a static prop so behavior is correct, but the comment is misleading. Either use[]with the disable comment, or drop the comment and keep[readOnly]. -
minimapNodeColormixes CSS vars and hardcoded hex:
triggerusesvar(--th-accent)(theme-aware) butagentuses"#22c55e"(hardcoded green-500). In a dark theme this may look inconsistent. Consider a CSS custom property or Tailwind token so both colours follow the active theme. -
localStoragecast without runtime validation (WorkflowDetail.tsx~line 807):
localStorage.getItem(VIEW_PREF_KEY) as "details" | "graph"is a TypeScript cast, not a runtime check. If the key contains an unexpected string,viewreceives a value that matches neither branch and the page body renders blank. A one-liner guard would make this safe:const stored = localStorage.getItem(VIEW_PREF_KEY); return stored === "graph" ? "graph" : "details";
-
onNodesChange/onEdgesChange/onConnectrequired in read-only usage:
Making these props optional inWorkflowCanvasProps(defaulting to no-ops internally) would clean up the call site inWorkflowDetail. Minor API ergonomics — follow-up.
Adds read-only canvas mode to WorkflowCanvas (disables drag/connect/delete, shows 'View mode' badge, color-codes minimap by node type), implements a dependency-free LR/TB auto-layout algorithm, and adds a graph/details toggle to WorkflowDetail with localStorage preference persistence.
Closes #1102