Skip to content

fix/simplify-graph-viewer #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thomashebrard wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix/simplify-graph-viewer #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ coverage/
.DS_Store
.vscode/
.idea/
/temp/
/temp/
.gstack/
330 changes: 226 additions & 104 deletions README.md
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 10 additions & 1 deletion src/graph/graphConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import type { GraphConfig } from "./types";

export const DEFAULT_GRAPH_CONFIG: GraphConfig = {
direction: "TB",
direction: "LR",
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P2: Changing the exported default direction to LR silently changes GraphViewer's public default layout for every consumer that does not pass direction. Keep TB here unless this breaking change is being rolled out intentionally with matching docs/release updates.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/graph/graphConfig.ts, line 4:

<comment>Changing the exported default direction to `LR` silently changes `GraphViewer`'s public default layout for every consumer that does not pass `direction`. Keep `TB` here unless this breaking change is being rolled out intentionally with matching docs/release updates.</comment>

<file context>
@@ -1,14 +1,15 @@
 
 export const DEFAULT_GRAPH_CONFIG: GraphConfig = {
-  direction: "TB",
+  direction: "LR",
   showControllers: false,
   nodesep: 50,
</file context>
Suggested change
direction: "LR",
direction: "TB",
Fix with Cubic

showControllers: false,
nodesep: 50,
ranksep: 30,
edgeType: "bezier",
initialZoom: null,
panToTop: true,
paletteColors: {
// Graph node/edge colors
"--color-pipe": "#ff6b6b",
"--color-pipe-bg": "rgba(224,108,117,0.18)",
"--color-pipe-text": "#ffffff",
Expand All @@ -27,6 +28,14 @@ export const DEFAULT_GRAPH_CONFIG: GraphConfig = {
"--color-error-bg": "rgba(255,85,85,0.15)",
"--color-accent": "#8BE9FD",
"--color-warning": "#FFB86C",
// Base theme vars used by graph-core.css
"--color-bg": "#1e1e1e",
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P1: These new palette keys are too generic for a component that writes its theme variables onto document.documentElement; mounting the graph can now override the host app's global --color-bg, --font-*, and --shadow-* tokens.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/graph/graphConfig.ts, line 32:

<comment>These new palette keys are too generic for a component that writes its theme variables onto `document.documentElement`; mounting the graph can now override the host app's global `--color-bg`, `--font-*`, and `--shadow-*` tokens.</comment>

<file context>
@@ -27,6 +28,14 @@ export const DEFAULT_GRAPH_CONFIG: GraphConfig = {
     "--color-accent": "#8BE9FD",
     "--color-warning": "#FFB86C",
+    // Base theme vars used by graph-core.css
+    "--color-bg": "#1e1e1e",
+    "--color-bg-dots": "#334155",
+    "--color-text-muted": "#94a3b8",
</file context>
Fix with Cubic

"--color-bg-dots": "#334155",
"--color-text-muted": "#94a3b8",
"--color-controller-text": "#94a3b8",
"--font-sans": '"Inter", -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif',
"--font-mono": '"JetBrains Mono", "Monaco", "Menlo", monospace',
"--shadow-lg": "0 8px 24px rgba(0, 0, 0, 0.5)",
},
};

Expand Down
27 changes: 22 additions & 5 deletions src/graph/react/GraphViewer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ import { toAppNodes, toAppEdges } from "./rfTypes";
import { buildGraph } from "../graphBuilders";
import { getLayoutedElements, ensureControllerSpacing } from "../graphLayout";
import { applyControllers } from "../graphControllers";
import { DEFAULT_GRAPH_CONFIG } from "../graphConfig";
import { hydrateLabels } from "./renderLabel";
import { controllerNodeTypes } from "./ControllerGroupNode";

export interface GraphViewerProps {
graphspec: GraphSpec | null;
config: GraphConfig;
direction: GraphDirection;
showControllers: boolean;
config?: GraphConfig;
direction?: GraphDirection;
showControllers?: boolean;
onNavigateToPipe?: (pipeCode: string) => void;
onReactFlowInit?: (instance: AppRFInstance) => void;
}
Expand All @@ -42,8 +43,24 @@ function cloneCachedNodes(nodes: GraphNode[]): GraphNode[] {
}

export function GraphViewer(props: GraphViewerProps) {
const { graphspec, config, direction, showControllers, onNavigateToPipe, onReactFlowInit } =
props;
const {
graphspec,
config = DEFAULT_GRAPH_CONFIG,
direction = config.direction ?? DEFAULT_GRAPH_CONFIG.direction ?? "TB",
showControllers = config.showControllers ?? DEFAULT_GRAPH_CONFIG.showControllers ?? false,
onNavigateToPipe,
onReactFlowInit,
} = props;

// Apply palette CSS vars on mount (so consumers don't have to)
React.useEffect(() => {
const palette = config.paletteColors ?? DEFAULT_GRAPH_CONFIG.paletteColors;
if (palette) {
for (const [cssVar, value] of Object.entries(palette)) {
document.documentElement.style.setProperty(cssVar, value);
}
}
Comment on lines +56 to +62
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P2: Restore root CSS variables in this effect; otherwise one GraphViewer instance can leave a stale palette applied globally after unmount or palette changes.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/graph/react/GraphViewer.tsx, line 56:

<comment>Restore root CSS variables in this effect; otherwise one GraphViewer instance can leave a stale palette applied globally after unmount or palette changes.</comment>

<file context>
@@ -42,8 +43,24 @@ function cloneCachedNodes(nodes: GraphNode[]): GraphNode[] {
+  } = props;
+
+  // Apply palette CSS vars on mount (so consumers don't have to)
+  React.useEffect(() => {
+    const palette = config.paletteColors ?? DEFAULT_GRAPH_CONFIG.paletteColors;
+    if (palette) {
</file context>
Suggested change
React.useEffect(() => {
const palette = config.paletteColors ?? DEFAULT_GRAPH_CONFIG.paletteColors;
if (palette) {
for (const [cssVar, value] of Object.entries(palette)) {
document.documentElement.style.setProperty(cssVar, value);
}
}
React.useEffect(() => {
const rootStyle = document.documentElement.style;
const palette = config.paletteColors ?? DEFAULT_GRAPH_CONFIG.paletteColors;
if (!palette) return;
const previousValues = Object.fromEntries(
Object.keys(palette).map((cssVar) => [cssVar, rootStyle.getPropertyValue(cssVar)]),
);
for (const [cssVar, value] of Object.entries(palette)) {
rootStyle.setProperty(cssVar, value);
}
return () => {
for (const [cssVar, value] of Object.entries(previousValues)) {
if (value) {
rootStyle.setProperty(cssVar, value);
} else {
rootStyle.removeProperty(cssVar);
}
}
};
}, [config.paletteColors]);
Fix with Cubic

}, [config.paletteColors]);

const [nodes, setNodes, onNodesChange] = useNodesState<AppNode>([]);
const [edges, setEdges, onEdgesChange] = useEdgesState<AppEdge>([]);
Expand Down
4 changes: 2 additions & 2 deletions src/graph/react/graph-core.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.react-flow-container {
width: 100%;
height: 100%;
position: absolute;
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P2: Making the exported GraphViewer root position: absolute is a breaking layout change. Existing callers that mount it in a normal container will now need a positioned parent, or the graph will size/anchor against the wrong containing block.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/graph/react/graph-core.css, line 2:

<comment>Making the exported GraphViewer root `position: absolute` is a breaking layout change. Existing callers that mount it in a normal container will now need a positioned parent, or the graph will size/anchor against the wrong containing block.</comment>

<file context>
@@ -1,6 +1,6 @@
 .react-flow-container {
-  width: 100%;
-  height: 100%;
+  position: absolute;
+  inset: 0;
   background: var(--color-bg);
</file context>
Fix with Cubic

inset: 0;
background: var(--color-bg);
}
.react-flow__node {
Expand Down
5 changes: 5 additions & 0 deletions src/graph/react/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
"use client";
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P2: This client directive is too broad for the graph/react barrel. It makes every export from this entrypoint client-only, including the helper/type-converter APIs re-exported below.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/graph/react/index.ts, line 1:

<comment>This client directive is too broad for the `graph/react` barrel. It makes every export from this entrypoint client-only, including the helper/type-converter APIs re-exported below.</comment>

<file context>
@@ -1,3 +1,8 @@
+"use client";
+
+import "@xyflow/react/dist/style.css";
</file context>
Fix with Cubic


import "@xyflow/react/dist/style.css";
import "./graph-core.css";

export { GraphViewer } from "./GraphViewer";
export type { GraphViewerProps } from "./GraphViewer";
export type { AppNode, AppEdge, AppRFInstance } from "./rfTypes";
Expand Down
3 changes: 3 additions & 0 deletions tsup.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ export default defineConfig({
"react",
"react-dom",
"@xyflow/react",
/graph-core\.css$/,
],
// Keep graph-core.css as a standalone export for consumers that need it separately.
// GraphViewer.tsx also imports it directly, so it gets bundled into the React entry's CSS.
onSuccess: async () => {
mkdirSync("dist/graph/react", { recursive: true });
cpSync("src/graph/react/graph-core.css", "dist/graph/react/graph-core.css");
Expand Down