-
-
Notifications
You must be signed in to change notification settings - Fork 22
feat(canvas): render real mermaid/flowchart diagrams (slice 4) #1411
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.
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { describe, it, expect, vi } from "vitest"; | ||
|
|
||
| vi.mock("@excalidraw/mermaid-to-excalidraw", () => ({ | ||
| parseMermaidToExcalidraw: vi.fn(async (src: string) => { | ||
| if (src.includes("BOOM")) throw new Error("invalid mermaid"); | ||
| return { elements: [{ type: "rectangle", x: 5, y: 7, width: 10, height: 10 }], files: {} }; | ||
| }), | ||
| })); | ||
| vi.mock("@excalidraw/excalidraw", () => ({ | ||
| convertToExcalidrawElements: (els: unknown[]) => els, | ||
| })); | ||
|
|
||
| import { mermaidToExcalidraw } from "../canvas/mermaid-to-elements"; | ||
|
|
||
| describe("mermaidToExcalidraw", () => { | ||
| it("converts the diagram and translates it by the element offset", async () => { | ||
| const out = await mermaidToExcalidraw("graph TD\n A-->B", 100, 200); | ||
| expect(out).toHaveLength(1); | ||
| expect(out[0]).toMatchObject({ type: "rectangle", x: 105, y: 207 }); | ||
| }); | ||
|
|
||
| it("returns an empty array for a blank source", async () => { | ||
| expect(await mermaidToExcalidraw(" ", 0, 0)).toEqual([]); | ||
| }); | ||
|
|
||
| it("returns an empty array when the mermaid source is invalid", async () => { | ||
| expect(await mermaidToExcalidraw("BOOM", 0, 0)).toEqual([]); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,43 +1,108 @@ | ||
| import { useMemo, useState, useEffect } from "react"; | ||
| import { useMemo, useState, useEffect, useRef } from "react"; | ||
| import type { ComponentProps } from "react"; | ||
| import { Excalidraw, convertToExcalidrawElements } from "@excalidraw/excalidraw"; | ||
| import "@excalidraw/excalidraw/index.css"; | ||
| import { CanvasElement } from "./canvas-api"; | ||
| import { elementsToSkeletons } from "./element-to-excalidraw"; | ||
| import { elementsToSkeletons, elementToSkeleton } from "./element-to-excalidraw"; | ||
| import { mermaidToExcalidraw, type ExcalidrawElements } from "./mermaid-to-elements"; | ||
|
|
||
| // Read-only Excalidraw view over the canonical CanvasElement scene. Slice 2 of | ||
| // the tldraw -> Excalidraw migration: the board is built alongside the existing | ||
| // tldraw board and not yet wired into CanvasView. Write-back interactions, | ||
| // diagram rendering, and the swap that retires tldraw are later slices. | ||
| // Read-only Excalidraw view over the canonical CanvasElement scene (tldraw -> | ||
| // Excalidraw migration). Most kinds map synchronously; mermaid/flowchart kinds | ||
| // render their real diagram via mermaid-to-excalidraw, falling back to the | ||
| // placeholder rectangle while the (async) conversion runs or if it fails. The | ||
| // board is not yet wired into CanvasView; write-back interactions and the swap | ||
| // that retires tldraw are later slices. | ||
|
|
||
| const DIAGRAM_KINDS = new Set(["mermaid", "flowchart"]); | ||
|
|
||
| type ExcalidrawAPI = Parameters< | ||
| NonNullable<ComponentProps<typeof Excalidraw>["excalidrawAPI"]> | ||
| >[0]; | ||
|
|
||
| type SkeletonInput = Parameters<typeof convertToExcalidrawElements>[0]; | ||
|
|
||
| export interface ExcalidrawBoardProps { | ||
| elements: CanvasElement[]; | ||
| theme?: "light" | "dark"; | ||
| } | ||
|
|
||
| function live(elements: CanvasElement[]): CanvasElement[] { | ||
| return elements | ||
| .filter((el) => el.deleted_at == null) | ||
| .slice() | ||
| .sort((a, b) => (a.z_index || 0) - (b.z_index || 0)); | ||
| } | ||
|
|
||
| export function ExcalidrawBoard({ elements, theme = "light" }: ExcalidrawBoardProps) { | ||
| const [api, setApi] = useState<ExcalidrawAPI | null>(null); | ||
| // Converted diagram elements keyed by the CanvasElement id; absent until the | ||
| // async mermaid conversion resolves. | ||
| const [diagrams, setDiagrams] = useState<Record<string, ExcalidrawElements>>({}); | ||
|
|
||
| const sceneElements = useMemo( | ||
| // Re-run conversion only when a diagram element's id/source/position changes. | ||
| const diagramKey = useMemo( | ||
| () => | ||
| convertToExcalidrawElements( | ||
| elementsToSkeletons(elements) as unknown as Parameters< | ||
| typeof convertToExcalidrawElements | ||
| >[0], | ||
| JSON.stringify( | ||
| live(elements) | ||
| .filter((el) => DIAGRAM_KINDS.has(el.kind)) | ||
| .map((el) => [el.id, el.x, el.y, (el.payload || {}).source]), | ||
| ), | ||
| [elements], | ||
| ); | ||
|
|
||
| // Excalidraw opens at scroll origin, so a scene whose elements sit away from | ||
| // (0,0) renders off-screen. Fit the viewport to the content once the API is | ||
| // ready and whenever the scene changes. | ||
| useEffect(() => { | ||
| if (api && sceneElements.length > 0) { | ||
| let cancelled = false; | ||
| const diagramEls = live(elements).filter((el) => DIAGRAM_KINDS.has(el.kind)); | ||
| if (diagramEls.length === 0) { | ||
| setDiagrams({}); | ||
| return; | ||
| } | ||
| (async () => { | ||
| // Convert diagrams in parallel: each parse is independent, so the total | ||
| // wait is the slowest single diagram, not the sum. | ||
| const converted = await Promise.all( | ||
| diagramEls.map(async (el) => { | ||
| const src = String((el.payload || {}).source ?? ""); | ||
| return [el.id, await mermaidToExcalidraw(src, el.x, el.y)] as const; | ||
| }), | ||
| ); | ||
| if (!cancelled) setDiagrams(Object.fromEntries(converted)); | ||
| })(); | ||
|
Comment on lines
+60
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Quality: Sequential awaits make multi-diagram conversion slowThe conversion loop awaits Was this helpful? React with 👍 / 👎 |
||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| // diagramKey captures the inputs that affect conversion. | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [diagramKey]); | ||
|
|
||
| const sceneElements = useMemo(() => { | ||
| const els = live(elements); | ||
| const regular = els.filter((el) => !DIAGRAM_KINDS.has(el.kind)); | ||
| const regularScene = convertToExcalidrawElements( | ||
| elementsToSkeletons(regular) as unknown as SkeletonInput, | ||
| ); | ||
| const diagramScene = els | ||
| .filter((el) => DIAGRAM_KINDS.has(el.kind)) | ||
| .flatMap((el) => { | ||
| const ready = diagrams[el.id]; | ||
| if (ready && ready.length > 0) return ready; | ||
| // Fallback: the placeholder rectangle from the base mapping. | ||
| return convertToExcalidrawElements([elementToSkeleton(el)] as unknown as SkeletonInput); | ||
| }); | ||
| return [...regularScene, ...diagramScene]; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Z-ordering bug — regular elements always render below all diagram elements.
Merge the two arrays and re-sort by Reply with |
||
| }, [elements, diagrams]); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // Excalidraw reads initialData once at mount; push later scenes (async | ||
| // diagrams) through the imperative API. Fit the viewport to the content only | ||
| // on the first non-empty scene -- refitting on every update would yank a | ||
| // user's pan/zoom back once interactions land. | ||
| const hasFit = useRef(false); | ||
| useEffect(() => { | ||
| if (!api) return; | ||
| api.updateScene({ elements: sceneElements }); | ||
| if (!hasFit.current && sceneElements.length > 0) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: Fit-once can lock the viewport to the placeholder extent On mount, Consider fitting on the first non-empty scene after diagrams have resolved (e.g. gate on Reply with |
||
| api.scrollToContent(sceneElements, { fitToContent: true, animate: false }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: The effect re-runs whenever Reply with |
||
| hasFit.current = true; | ||
| } | ||
| }, [api, sceneElements]); | ||
|
Comment on lines
+98
to
105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Performance: Viewport refits on every scene update, resetting pan/zoomThe effect calls Consider running Was this helpful? React with 👍 / 👎 |
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { parseMermaidToExcalidraw } from "@excalidraw/mermaid-to-excalidraw"; | ||
| import { convertToExcalidrawElements } from "@excalidraw/excalidraw"; | ||
|
|
||
| // Convert a mermaid/flowchart source string into positioned Excalidraw elements | ||
| // for the canvas board. parseMermaidToExcalidraw yields skeleton elements laid | ||
| // out from the diagram's own origin; convertToExcalidrawElements fills them in, | ||
| // then we translate the whole diagram to the CanvasElement's (x, y). Returns an | ||
| // empty array on invalid mermaid so a bad source degrades to nothing (the board | ||
| // keeps the placeholder) rather than throwing. | ||
|
|
||
| export type ExcalidrawElements = ReturnType<typeof convertToExcalidrawElements>; | ||
|
|
||
| export async function mermaidToExcalidraw( | ||
| source: string, | ||
| offsetX: number, | ||
| offsetY: number, | ||
| ): Promise<ExcalidrawElements> { | ||
| const src = (source || "").trim(); | ||
| if (!src) return []; | ||
| try { | ||
| const { elements } = await parseMermaidToExcalidraw(src); | ||
| const converted = convertToExcalidrawElements(elements); | ||
| // Translate the diagram as a whole; bindings between nodes are relative, so | ||
| // shifting every element by the same offset preserves them. | ||
| return converted.map((el) => ({ ...el, x: el.x + offsetX, y: el.y + offsetY })); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Translating only The comment claims "shifting every element by the same offset preserves them" because bindings are relative. That is true for Reply with |
||
| } catch (err) { | ||
| // Degrading to the placeholder is the intended UX, but a swallowed parse | ||
| // error gives no signal in dev; log it so a malformed source is debuggable. | ||
| console.warn("mermaid conversion failed; showing placeholder", err); | ||
| return []; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WARNING:
diagramKeyinvalidates on every unrelated element change, re-converting all diagrams.The
useMemofordiagramKeylistselementsas its only dependency.elementsis a fresh array reference whenever the scene changes (e.g., editing a note's text payload, dragging a sticky), so the memo recomputesJSON.stringify(...)and returns a new string even when the diagram-relevant inputs (id,x,y,source) are unchanged. A newdiagramKeytriggers the conversion effect, so adding/removing/editing any non-diagram element re-parses every mermaid source on the board.Either drop the memoization and depend on a derived signature (e.g., a useMemo that returns the diagram list itself with
elementsas the dep), or hash only the relevant fields with a stable key.Reply with
@kilocode-bot fix itto have Kilo Code address this issue.