Skip to content

Commit 522a54c

Browse files
os-zhuangclaude
andauthored
feat(studio): make the flow-canvas error banner clickable (#1976)
The inline structural-error banner (ADR-0044 cycle surfacing) was display-only, duplicating the newer Problems-panel data via a separate `validationErrors` string[]. Drive it from the unified `problems` list instead and make each row with a concrete target a button: clicking selects + pans-to-reveal the offending node/edge (reusing the Problems panel's reveal handler), so the always-visible banner is actionable without opening the panel. The `validationErrors` prop + its duplicate validateFlowDraft-message derivation are removed; the red edge/node highlighting stays. Refs #1958 (ADR-0044 inline cycle surfacing). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 0b9c96c commit 522a54c

4 files changed

Lines changed: 105 additions & 34 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
---
4+
5+
feat(studio): make the flow-canvas error banner clickable
6+
7+
The inline structural-error banner (ADR-0044 cycle surfacing) is now driven by
8+
the unified `problems` list, and each row with a concrete target is clickable —
9+
clicking it selects and pans-to-reveal the offending node/edge (the same reveal
10+
the Problems panel performs). So the always-visible banner is actionable without
11+
opening the panel. Drops the now-redundant `validationErrors` string prop: the
12+
banner, the Problems panel, and the on-canvas badges all share one source.

packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.test.tsx

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

3-
import { describe, it, expect, afterEach } from 'vitest';
4-
import { render, screen, cleanup } from '@testing-library/react';
3+
import { describe, it, expect, afterEach, vi } from 'vitest';
4+
import { render, screen, cleanup, fireEvent } from '@testing-library/react';
55
import { FlowCanvas } from './FlowCanvas';
6+
import type { FlowProblem } from './flow-problems';
67

78
afterEach(cleanup);
89

910
/**
1011
* ADR-0044: an un-declared cycle is surfaced INLINE on the canvas — the
1112
* offending edges/nodes are painted red (data-invalid) and an error banner
12-
* shows the message — so the author needn't open the Debug panel.
13+
* shows the message. Each banner row with a concrete target is clickable and
14+
* reveals (selects + pans to) the offending element — the same reveal the
15+
* Problems panel does — so the always-visible banner is actionable.
1316
*/
1417
describe('FlowCanvas — inline cycle/error surfacing', () => {
1518
const nodes = [
@@ -20,8 +23,15 @@ describe('FlowCanvas — inline cycle/error surfacing', () => {
2023
{ source: 'a', target: 'w', label: 'revise' },
2124
{ source: 'w', target: 'a', label: 'resubmit' }, // unmarked cycle
2225
];
26+
const cycleProblem: FlowProblem = {
27+
id: 'cyc1',
28+
level: 'error',
29+
message: 'Cycle detected (a → w → a). Mark the closing edge as a back-edge.',
30+
target: { kind: 'edge', edgeKey: 'w->a#1', source: 'w', target: 'a' },
31+
source: 'structural',
32+
};
2333

24-
it('renders the error banner and marks invalid edges + nodes', () => {
34+
it('renders the error banner (from problems) and marks invalid edges + nodes', () => {
2535
const { container } = render(
2636
<FlowCanvas
2737
nodes={nodes}
@@ -32,15 +42,47 @@ describe('FlowCanvas — inline cycle/error surfacing', () => {
3242
onSelect={() => {}}
3343
invalidNodeIds={['a', 'w']}
3444
invalidEdges={new Set(['a->w', 'w->a'])}
35-
validationErrors={['Cycle detected (a → w → a). Mark the closing edge as a back-edge.']}
45+
problems={[cycleProblem]}
3646
/>,
3747
);
38-
// Banner message is visible on the canvas.
3948
expect(screen.getByText(/Cycle detected/)).toBeInTheDocument();
4049
// Two cycle edges + two cycle nodes carry the data-invalid marker.
4150
expect(container.querySelectorAll('[data-invalid="true"]').length).toBeGreaterThanOrEqual(4);
4251
});
4352

53+
it('reveals the offending element when a banner row is clicked', () => {
54+
const onRevealProblem = vi.fn();
55+
render(
56+
<FlowCanvas
57+
nodes={nodes}
58+
edges={edges}
59+
editable={false}
60+
designMode={false}
61+
selectedId={null}
62+
onSelect={() => {}}
63+
problems={[cycleProblem]}
64+
onRevealProblem={onRevealProblem}
65+
/>,
66+
);
67+
fireEvent.click(screen.getByText(/Cycle detected/));
68+
expect(onRevealProblem).toHaveBeenCalledWith(cycleProblem);
69+
});
70+
71+
it('only counts error-level problems in the banner (warnings are not shown)', () => {
72+
render(
73+
<FlowCanvas
74+
nodes={nodes}
75+
edges={edges}
76+
editable={false}
77+
designMode={false}
78+
selectedId={null}
79+
onSelect={() => {}}
80+
problems={[{ id: 'w1', level: 'warning', message: 'Just a heads-up', target: { kind: 'flow' }, source: 'structural' }]}
81+
/>,
82+
);
83+
expect(screen.queryByText(/Just a heads-up/)).toBeNull();
84+
});
85+
4486
it('shows no banner and no invalid markers for a clean flow', () => {
4587
const { container } = render(
4688
<FlowCanvas

packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,12 @@ export interface FlowCanvasProps {
8888
invalidNodeIds?: string[];
8989
/** Structural-validation: edges (keyed `${source}->${target}`) to paint red. */
9090
invalidEdges?: ReadonlySet<string>;
91-
/** Structural-validation error messages shown in an inline canvas banner. */
92-
validationErrors?: string[];
91+
/**
92+
* Select + reveal a problem when its inline-banner row is clicked — wired to
93+
* the same handler the Problems panel uses, so the always-visible banner is
94+
* actionable without opening the panel.
95+
*/
96+
onRevealProblem?: (problem: FlowProblem) => void;
9397
/**
9498
* Unified validation issues (structural + server) rendered as per-element
9599
* badges; the Problems panel shares the same list.
@@ -120,7 +124,7 @@ export function FlowCanvas({
120124
traversedEdgeIds,
121125
invalidNodeIds,
122126
invalidEdges,
123-
validationErrors,
127+
onRevealProblem,
124128
problems,
125129
revealSignal,
126130
onSelect,
@@ -160,6 +164,10 @@ export function FlowCanvas({
160164
[problems],
161165
);
162166

167+
// Error-level problems shown in the always-visible inline banner — driven by
168+
// the same `problems` list as the panel/badges so the three stay in lock-step.
169+
const bannerErrors = React.useMemo(() => (problems ?? []).filter((p) => p.level === 'error'), [problems]);
170+
163171
const positionOf = React.useCallback(
164172
(id: string): Point => {
165173
if (dragPos && dragPos.id === id) return { x: dragPos.x, y: dragPos.y };
@@ -450,21 +458,34 @@ export function FlowCanvas({
450458
return (
451459
<div className="relative h-full min-h-[320px] w-full overflow-hidden">
452460
{/* Inline structural-validation banner (ADR-0044 cycle surfacing): shows
453-
errors directly on the canvas so the author needn't open Debug. */}
454-
{validationErrors && validationErrors.length > 0 && (
461+
errors directly on the canvas so the author needn't open Debug. Each row
462+
with a concrete target is clickable — it selects + pans to the offending
463+
node/edge (the same reveal the Problems panel does). */}
464+
{bannerErrors.length > 0 && (
455465
<div className="absolute left-2 top-2 z-30 max-w-[min(60%,420px)] space-y-1">
456-
{validationErrors.slice(0, 3).map((msg, i) => (
457-
<div
458-
key={i}
459-
role="alert"
460-
className="flex items-start gap-1.5 rounded-lg border border-destructive/40 bg-destructive/10 px-2.5 py-1.5 text-[11px] leading-snug text-destructive shadow-sm backdrop-blur-sm"
461-
>
462-
<AlertTriangle className="mt-0.5 h-3.5 w-3.5 shrink-0" />
463-
<span>{msg}</span>
464-
</div>
465-
))}
466-
{validationErrors.length > 3 && (
467-
<div className="px-2.5 text-[10px] text-destructive/80">+{validationErrors.length - 3} more…</div>
466+
{bannerErrors.slice(0, 3).map((p) => {
467+
const clickable = !!onRevealProblem && p.target.kind !== 'flow';
468+
return (
469+
<button
470+
key={p.id}
471+
type="button"
472+
role="alert"
473+
disabled={!clickable}
474+
onPointerDown={(e) => e.stopPropagation()}
475+
onClick={clickable ? (e) => { e.stopPropagation(); onRevealProblem!(p); } : undefined}
476+
title={clickable ? 'Reveal on canvas' : undefined}
477+
className={cn(
478+
'flex w-full items-start gap-1.5 rounded-lg border border-destructive/40 bg-destructive/10 px-2.5 py-1.5 text-left text-[11px] leading-snug text-destructive shadow-sm backdrop-blur-sm transition-colors',
479+
clickable && 'cursor-pointer hover:border-destructive/60 hover:bg-destructive/20',
480+
)}
481+
>
482+
<AlertTriangle className="mt-0.5 h-3.5 w-3.5 shrink-0" />
483+
<span>{p.message}</span>
484+
</button>
485+
);
486+
})}
487+
{bannerErrors.length > 3 && (
488+
<div className="px-2.5 text-[10px] text-destructive/80">+{bannerErrors.length - 3} more…</div>
468489
)}
469490
</div>
470491
)}

packages/app-shell/src/views/metadata-admin/previews/FlowPreview.tsx

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,11 @@ export function FlowPreview({ draft, editing, selection, onSelectionChange, onPa
9595
traversedEdgeIds: string[];
9696
} | null>(null);
9797

98-
// Continuous structural validation surfaced INLINE on the canvas (ADR-0044):
99-
// an un-declared cycle (and other structural errors) paints the offending
100-
// edges/nodes red and shows a banner — so the author sees it without opening
101-
// the Debug panel. Same `validateFlowDraft` the simulator preflight uses.
102-
const { invalidNodeIds, invalidEdges, validationErrors } = React.useMemo(() => {
98+
// Continuous structural validation paints the offending edges/nodes red on the
99+
// canvas (ADR-0044) — so an un-declared cycle is visible without opening Debug.
100+
// Same `validateFlowDraft` the simulator preflight uses; the inline banner and
101+
// per-element badges are driven by the unified `problems` list below.
102+
const { invalidNodeIds, invalidEdges } = React.useMemo(() => {
103103
const v = validateFlowDraft(nodes as unknown as SimNode[], edges as unknown as SimEdge[]);
104104
const nodeSet = new Set<string>();
105105
const edgeSet = new Set<string>();
@@ -113,11 +113,7 @@ export function FlowPreview({ draft, editing, selection, onSelectionChange, onPa
113113
}
114114
}
115115
}
116-
return {
117-
invalidNodeIds: [...nodeSet],
118-
invalidEdges: edgeSet,
119-
validationErrors: v.errors.map((diag) => diag.message),
120-
};
116+
return { invalidNodeIds: [...nodeSet], invalidEdges: edgeSet };
121117
}, [nodes, edges]);
122118

123119
// Unified problem list (structural + server `_diagnostics`) shared by the
@@ -298,7 +294,7 @@ export function FlowPreview({ draft, editing, selection, onSelectionChange, onPa
298294
traversedEdgeIds={runHL?.traversedEdgeIds}
299295
invalidNodeIds={invalidNodeIds}
300296
invalidEdges={invalidEdges}
301-
validationErrors={validationErrors}
297+
onRevealProblem={handleSelectProblem}
302298
problems={problems}
303299
revealSignal={reveal}
304300
onSelect={(n) =>

0 commit comments

Comments
 (0)