-
Notifications
You must be signed in to change notification settings - Fork 0
feat(frontend): mermaid diagrams + preview card auto-close fix #434
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
08f47f8
01e46b8
f4aebb4
eb12ac1
b754ab4
4596532
4c633a6
946f7ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import { useEffect, useId, useRef, useState } from 'react'; | ||
| import mermaid from 'mermaid'; | ||
| import { CopyButton } from './CopyButton'; | ||
|
|
||
| let mermaidInitialized = false; | ||
|
Owner
Author
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. 🔵 unsafe_assumptions: The module-level
Owner
Author
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. 🔵 unsafe_assumptions: The module-level
Owner
Author
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. 🔵 style: Module-level
Owner
Author
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. 🔵 unsafe_assumptions: Module-level
Owner
Author
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. 🔵 unsafe_assumptions: The module-level
Owner
Author
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. 🔵 unsafe_assumptions: The module-level |
||
|
|
||
| function ensureMermaidInit() { | ||
| if (mermaidInitialized) return; | ||
| mermaidInitialized = true; | ||
| mermaid.initialize({ | ||
|
Owner
Author
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. 🟡 unsafe_assumptions: Mermaid's |
||
| startOnLoad: false, | ||
| theme: 'dark', | ||
| themeVariables: { | ||
| darkMode: true, | ||
| background: '#1e1e2e', | ||
| primaryColor: '#7c3aed', | ||
| primaryTextColor: '#e2e8f0', | ||
| primaryBorderColor: '#6366f1', | ||
| lineColor: '#94a3b8', | ||
| secondaryColor: '#374151', | ||
| tertiaryColor: '#1f2937', | ||
| noteBkgColor: '#374151', | ||
| noteTextColor: '#e2e8f0', | ||
| fontFamily: 'inherit', | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| interface MermaidBlockProps { | ||
| code: string; | ||
| } | ||
|
|
||
| export function MermaidBlock({ code }: MermaidBlockProps) { | ||
| const instanceId = useId(); | ||
| const containerRef = useRef<HTMLDivElement>(null); | ||
|
Owner
Author
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. 🔵 style: |
||
| const [error, setError] = useState<string | null>(null); | ||
| const [svg, setSvg] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| ensureMermaidInit(); | ||
| let cancelled = false; | ||
| // useId() returns a stable, unique string per component instance | ||
| const id = `mermaid-${instanceId.replace(/:/g, '')}`; | ||
|
|
||
| async function render() { | ||
| try { | ||
| const { svg: rendered } = await mermaid.render(id, code); | ||
| if (!cancelled) { | ||
| setSvg(rendered); | ||
| setError(null); | ||
| } | ||
| } catch { | ||
| if (!cancelled) { | ||
|
Owner
Author
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. 🔵 unsafe_assumptions: The DOM cleanup |
||
| setError('Invalid diagram'); | ||
| setSvg(null); | ||
| } | ||
| // Clean up any leftover element mermaid may have created | ||
| document.getElementById(`d${id}`)?.remove(); | ||
|
Owner
Author
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. 🔵 bugs: The DOM cleanup
Owner
Author
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. 🟡 bugs: The cleanup |
||
| } | ||
| } | ||
|
|
||
| render(); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [code]); | ||
|
Owner
Author
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. 🔵 bugs: |
||
|
|
||
| if (error) { | ||
| // Fall back to plain code block | ||
| return ( | ||
| <div className="code-block-wrapper"> | ||
| <pre> | ||
| <code>{code}</code> | ||
| </pre> | ||
| <CopyButton text={code} className="code-block-copy" label="Copy code" /> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (!svg) return null; | ||
|
Owner
Author
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. 🔵 style: When |
||
|
|
||
| return ( | ||
| <div className="mermaid-block"> | ||
| <div | ||
| className="mermaid-block-svg" | ||
| ref={containerRef} | ||
| dangerouslySetInnerHTML={{ __html: svg }} | ||
| /> | ||
| <CopyButton text={code} className="code-block-copy" label="Copy source" /> | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import React, { useState, useEffect, useRef } from 'react'; | ||
| import React, { useState, useEffect, useRef, useMemo } from 'react'; | ||
| import ReactMarkdown, { defaultUrlTransform } from 'react-markdown'; | ||
| import remarkGfm from 'remark-gfm'; | ||
| import rehypeHighlight from 'rehype-highlight'; | ||
|
|
@@ -11,6 +11,7 @@ import { ShareButton } from './ShareButton'; | |
| import { ReadAloudButton } from './ReadAloudButton'; | ||
| import { extractText } from '../lib/extractText'; | ||
| import { MarkdownPreviewCard } from './MarkdownPreviewCard'; | ||
| import { MermaidBlock } from './MermaidBlock'; | ||
|
|
||
| const COLLAPSE_HEIGHT = 300; | ||
|
|
||
|
|
@@ -101,6 +102,79 @@ export function TextBubble({ content, streaming = false, timestamp, readAloud }: | |
|
|
||
| const showCollapsed = isLong && collapsed && !streaming; | ||
|
|
||
| // Memoize components so react-markdown preserves component instances | ||
| // (e.g. MarkdownPreviewCard expanded state) across parent re-renders. | ||
| const mdComponents = useMemo( | ||
| () => ({ | ||
| table: ({ children, ...props }: React.ComponentProps<'table'>) => ( | ||
| <div className="table-scroll-wrapper"> | ||
| <table {...props}>{children}</table> | ||
| </div> | ||
| ), | ||
| pre: ({ children, ...props }: React.ComponentProps<'pre'>) => { | ||
|
Owner
Author
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. 🔵 style: The mermaid detection logic (extracting the first child, checking className against
Owner
Author
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. 🔵 style: The mermaid-aware ). Consider extracting the shared mermaid detection + fallback pattern, or having MessageBubble compose on top of markdown-config's pre.
|
||
| // Detect mermaid code blocks and render as diagrams | ||
| const child = React.Children.toArray(children)[0]; | ||
| if (React.isValidElement(child)) { | ||
| const className = (child.props as Record<string, unknown>)?.className; | ||
| if (typeof className === 'string' && /language-mermaid/.test(className)) { | ||
| const text = extractText(children); | ||
| return <MermaidBlock code={text} />; | ||
| } | ||
| } | ||
| const text = extractText(children); | ||
| return ( | ||
| <div className="code-block-wrapper"> | ||
| <pre {...props}>{children}</pre> | ||
| <CopyButton text={text} className="code-block-copy" label="Copy code" /> | ||
| </div> | ||
| ); | ||
| }, | ||
| p: ({ children }: React.ComponentProps<'p'>) => { | ||
| const childArray = React.Children.toArray(children); | ||
| if (childArray.length === 1 && React.isValidElement(childArray[0])) { | ||
| const el = childArray[0] as React.ReactElement<Record<string, unknown>>; | ||
| const href = el.props?.href as string | undefined; | ||
| if (href?.startsWith(FILE_SCHEME)) { | ||
| const filePath = decodeURIComponent(href.slice(FILE_SCHEME.length)); | ||
| if (/\.mdx?$/i.test(filePath)) { | ||
| return <MarkdownPreviewCard filePath={filePath} />; | ||
| } | ||
| } | ||
| } | ||
| return <p>{children}</p>; | ||
| }, | ||
| a: ({ href, children }: React.ComponentProps<'a'>) => { | ||
| if (href?.startsWith(FILE_SCHEME)) { | ||
| const filePath = decodeURIComponent(href.slice(FILE_SCHEME.length)); | ||
| return ( | ||
| <span className="file-path-group"> | ||
| <a | ||
| href="#" | ||
| className="file-path-link" | ||
| data-file-path={filePath} | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| navigate( | ||
| `/files?path=${encodeURIComponent(filePath)}&from=${encodeURIComponent(currentPath)}`, | ||
| ); | ||
| }} | ||
| > | ||
| {children} | ||
| </a> | ||
| <ShareButton filePath={filePath} className="file-path-share" /> | ||
| </span> | ||
| ); | ||
| } | ||
| return ( | ||
| <a href={href} target="_blank" rel="noopener noreferrer"> | ||
| {children} | ||
| </a> | ||
| ); | ||
| }, | ||
| }), | ||
| [navigate, currentPath], | ||
|
Owner
Author
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. 🟡 unsafe_assumptions: useMemo deps include
Owner
Author
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. 🔵 regressions: The useMemo deps include |
||
| ); | ||
|
|
||
| return ( | ||
| <div | ||
| className={`msg-bubble msg-bubble--assistant${streaming ? ' msg-bubble--streaming' : ''}${showCollapsed ? ' msg-bubble--collapsed' : ''}`} | ||
|
|
@@ -110,69 +184,7 @@ export function TextBubble({ content, streaming = false, timestamp, readAloud }: | |
| remarkPlugins={[remarkGfm]} | ||
| rehypePlugins={[rehypeHighlight]} | ||
| urlTransform={(url) => (url.startsWith(FILE_SCHEME) ? url : defaultUrlTransform(url))} | ||
| components={{ | ||
| table: ({ children, ...props }) => ( | ||
| <div className="table-scroll-wrapper"> | ||
| <table {...props}>{children}</table> | ||
| </div> | ||
| ), | ||
| pre: ({ children, ...props }) => { | ||
| const text = extractText(children); | ||
| return ( | ||
| <div className="code-block-wrapper"> | ||
| <pre {...props}>{children}</pre> | ||
| <CopyButton text={text} className="code-block-copy" label="Copy code" /> | ||
| </div> | ||
| ); | ||
| }, | ||
| // When a paragraph contains a single file-path link to a .md/.mdx | ||
| // file, promote it to an inline preview card instead of a plain link. | ||
| // In ReactMarkdown v10, children are unrendered component instances — | ||
| // the `a` handler hasn't run yet — so we check `href` (the prop | ||
| // ReactMarkdown passes) rather than rendered DOM attributes. | ||
| p: ({ children }) => { | ||
| const childArray = React.Children.toArray(children); | ||
| if (childArray.length === 1 && React.isValidElement(childArray[0])) { | ||
| const el = childArray[0] as React.ReactElement<Record<string, unknown>>; | ||
| const href = el.props?.href as string | undefined; | ||
| if (href?.startsWith(FILE_SCHEME)) { | ||
| const filePath = decodeURIComponent(href.slice(FILE_SCHEME.length)); | ||
| if (/\.mdx?$/i.test(filePath)) { | ||
| return <MarkdownPreviewCard filePath={filePath} />; | ||
| } | ||
| } | ||
| } | ||
| return <p>{children}</p>; | ||
| }, | ||
| a: ({ href, children }) => { | ||
| if (href?.startsWith(FILE_SCHEME)) { | ||
| const filePath = decodeURIComponent(href.slice(FILE_SCHEME.length)); | ||
| return ( | ||
| <span className="file-path-group"> | ||
| <a | ||
| href="#" | ||
| className="file-path-link" | ||
| data-file-path={filePath} | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| navigate( | ||
| `/files?path=${encodeURIComponent(filePath)}&from=${encodeURIComponent(currentPath)}`, | ||
| ); | ||
| }} | ||
| > | ||
| {children} | ||
| </a> | ||
| <ShareButton filePath={filePath} className="file-path-share" /> | ||
| </span> | ||
| ); | ||
| } | ||
| return ( | ||
| <a href={href} target="_blank" rel="noopener noreferrer"> | ||
| {children} | ||
| </a> | ||
| ); | ||
| }, | ||
| }} | ||
| components={mdComponents} | ||
| > | ||
| {processed} | ||
| </ReactMarkdown> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { createElement } from 'react'; | ||
| import { markdownComponents } from '../markdown-config'; | ||
|
|
||
| describe('markdownComponents.pre', () => { | ||
| const pre = markdownComponents.pre!; | ||
|
|
||
| it('renders MermaidBlock for language-mermaid code blocks', () => { | ||
| const codeEl = createElement('code', { className: 'language-mermaid' }, 'graph TD; A-->B;'); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const result = (pre as any)({ children: codeEl }); | ||
|
Owner
Author
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. 🔵 style: Three |
||
| expect(result.type).not.toBe('pre'); | ||
| expect(result.props.code).toBe('graph TD; A-->B;'); | ||
| }); | ||
|
|
||
| it('renders normal pre for non-mermaid code blocks', () => { | ||
| const codeEl = createElement('code', { className: 'language-python' }, 'print("hi")'); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const result = (pre as any)({ children: codeEl }); | ||
| expect(result.type).toBe('pre'); | ||
| }); | ||
|
|
||
| it('renders normal pre when code has no className', () => { | ||
| const codeEl = createElement('code', null, 'plain text'); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const result = (pre as any)({ children: codeEl }); | ||
| expect(result.type).toBe('pre'); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,12 +4,16 @@ import rehypeRaw from 'rehype-raw'; | |
| import rehypeSanitize, { defaultSchema } from 'rehype-sanitize'; | ||
| import type { Components } from 'react-markdown'; | ||
| import type { PluggableList } from 'unified'; | ||
| import { MermaidBlock } from '../components/MermaidBlock'; | ||
| import { extractText } from './extractText'; | ||
|
|
||
| const sanitizeSchema = { | ||
| ...defaultSchema, | ||
| attributes: { | ||
| ...defaultSchema.attributes, | ||
| img: [...(defaultSchema.attributes?.img ?? []), 'width', 'height'], | ||
| // Only allow language-* classes (set by rehype-highlight) — not arbitrary classNames | ||
|
Owner
Author
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. 🔵 style: Comment says 'Only allow language-* classes (set by rehype-highlight)' but rehype-highlight is not used in this rendering path (MarkdownPreviewCard/FileViewer use rehypeRaw + rehypeSanitize, not rehypeHighlight). The classes come from the markdown parser's fenced code block syntax. Misleading comment could confuse future readers. |
||
| code: [...(defaultSchema.attributes?.code ?? []), ['className', /^language-/]], | ||
| }, | ||
| }; | ||
|
|
||
|
|
@@ -22,4 +26,15 @@ export const markdownComponents: Components = { | |
| <table {...props}>{children}</table> | ||
| </div> | ||
| ), | ||
| pre: ({ children, ...props }) => { | ||
|
Owner
Author
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. 🔵 missing_tests: The new
Owner
Author
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. 🟡 regressions: The
Owner
Author
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. 🔵 regressions: The
Owner
Author
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. 🔵 style: The |
||
| const child = React.Children.toArray(children)[0]; | ||
| if (React.isValidElement(child)) { | ||
| const className = (child.props as Record<string, unknown>)?.className; | ||
| if (typeof className === 'string' && /language-mermaid/.test(className)) { | ||
| const text = extractText(children); | ||
| return <MermaidBlock code={text} />; | ||
| } | ||
| } | ||
| return <pre {...props}>{children}</pre>; | ||
| }, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2123,6 +2123,29 @@ textarea:focus { | |
| } | ||
| } | ||
|
|
||
| /* ===== Mermaid Diagrams ===== */ | ||
|
Owner
Author
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. 🟡 bugs: The CopyButton on rendered mermaid diagrams will be permanently invisible on desktop. The hover rule |
||
|
|
||
| .mermaid-block { | ||
| position: relative; | ||
| margin: 0.5em 0; | ||
| border-radius: 6px; | ||
| background: var(--code-bg); | ||
| border: 1px solid var(--border); | ||
| overflow-x: auto; | ||
| -webkit-overflow-scrolling: touch; | ||
| } | ||
|
|
||
| .mermaid-block-svg { | ||
| padding: 0.75rem; | ||
| display: flex; | ||
| justify-content: center; | ||
| } | ||
|
|
||
| .mermaid-block-svg svg { | ||
| max-width: 100%; | ||
| height: auto; | ||
| } | ||
|
|
||
| /* ===== Collapsible Messages ===== */ | ||
|
|
||
| .msg-bubble--collapsed .msg-bubble-markdown { | ||
|
|
||
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.
🟡 style: Static
import mermaid from 'mermaid'pulls mermaid (plus d3, cytoscape, katex, roughjs, dompurify, etc.) into the main bundle. Since MermaidBlock is imported by MessageBubble.tsx (a core chat component), all these dependencies load on initial page load. For a mobile-first app this is a significant hit. Consider dynamically importing mermaid inside the useEffect (const { default: mermaid } = await import('mermaid')) so Vite can code-split it into a separate chunk loaded only when a mermaid diagram is actually encountered.[fixable]