Skip to content

feat(ui): add read-only workflow visualization with auto-layout#1114

Open
geoffjay wants to merge 3 commits intoissue-1101from
issue-1102
Open

feat(ui): add read-only workflow visualization with auto-layout#1114
geoffjay wants to merge 3 commits intoissue-1101from
issue-1102

Conversation

@geoffjay
Copy link
Copy Markdown
Owner

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

@geoffjay geoffjay added the review-agent Used to invoke a review by an agent tracking this label label Apr 14, 2026
Copy link
Copy Markdown
Owner Author

@geoffjay geoffjay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 at groupStart * nodeSpacing, no trigger placed. Correct. ✓
  • Duplicate-edge deduplication: !list.includes(edge.source) guard. ✓
  • other node 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, zoomOnScroll unconditional). ✓
  • readonly-badge with pointer-events-none so it doesn't intercept canvas clicks. ✓
  • data-readonly={readOnly} attribute — allows CSS targeting and test assertions. ✓

WorkflowDetail.tsx — view toggle

  • useEffect only runs when view === "graph" — no wasted computation in default details view. ✓
  • autoLayout called once per view switch, result stored in state (static read-only canvas). ✓
  • localStorage write wrapped in try/catch (private browsing / quota exceeded). ✓
  • No-op handlers for onNodesChange/onEdgesChange/onConnect — never triggered because readOnly disables all mutations. ✓

Tests

  • 5 new readOnly mode tests in WorkflowCanvas.test.tsx — badge presence/absence, badge text, data-readonly attribute. ✓
  • 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. ✓
  • useReactFlow stub added to WorkflowBuilder.test.tsx prevents FitViewOnLoad from crashing tests. ✓

Non-blocking suggestions

  1. FitViewOnLoad dep 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 when readOnly changes. In practice readOnly is 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].

  2. minimapNodeColor mixes CSS vars and hardcoded hex:
    trigger uses var(--th-accent) (theme-aware) but agent uses "#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.

  3. localStorage cast 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, view receives 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";
  4. onNodesChange/onEdgesChange/onConnect required in read-only usage:
    Making these props optional in WorkflowCanvasProps (defaulting to no-ops internally) would clean up the call site in WorkflowDetail. Minor API ergonomics — follow-up.

@geoffjay geoffjay added merge-queue Approved by reviewer, queued for merge by conductor and removed review-agent Used to invoke a review by an agent tracking this label labels Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-queue Approved by reviewer, queued for merge by conductor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant