Repository navigation
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR adds Code / Split / Chart mode toggling to both standalone
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| src/renderer/src/components/editor/MermaidViewer.tsx | Complete rewrite of MermaidViewer to add Code/Split/Chart mode toggling, live draft editing, read-only enforcement, and file-switch state sync. Uses the render-time state update pattern for file-path and echo-protection tracking. latestContentRef is only updated asynchronously via effect (see previous threads), so Ctrl+S on the same keypress as the last edit may save stale content — this is acknowledged in previous review threads. Otherwise well-structured. |
| src/renderer/src/components/editor/ZoomableDiagramSurface.tsx | New shared zoomable canvas with pointer-capture panning, anchored wheel/keyboard zoom, ResizeObserver surface sizing, and MutationObserver SVG intrinsic-size tracking. Address-of-element split (surfaceRef + surfaceElement state) correctly scopes wheel and resize effects. Previous thread concerns (children in MutationObserver deps, handleWheel stale closure) are addressed in this version. Viewport scroll position is not explicitly reset when resetKey changes, but in practice the parent always remounts via key=activeFile.id before resetKey can fire for a different file. |
| src/renderer/src/components/editor/RichMarkdownMermaidViews.tsx | New component that adds Code/Split/Chart toggle to inline markdown Mermaid fences. NodeViewContent is CSS-hidden (not removed from DOM) in chart mode to keep Tiptap's editor DOM intact. Debounce hook called without resetKey (correct since block identity is tied to component lifetime, not a prop). |
| src/renderer/src/components/editor/use-debounced-mermaid-diagram-content.ts | Clean debounce hook with two special cases: immediate flush when resetKey changes (file switch), and immediate clear when content becomes empty. Ref-based resetKey tracking avoids stale-closure issues. Empty-content path returns without a cleanup function, which is safe because React already runs the previous effect's cleanup before the new body executes. |
| src/renderer/src/components/editor/anchored-surface-zoom.ts | Pure math library for zoom fit/clamp/anchor scroll-offset calculations, extracted from the image viewer. Shared by both image viewer and diagram surface. Meticulously correct edge-case handling (zero currentZoom guard, deltaMode normalization, clamped exponential wheel factor). |
| src/renderer/src/components/editor/anchored-surface-dom-zoom.ts | DOM adapter over anchored-surface-zoom: reads current scroll position, flushSync-forces the React zoom state update, then adjusts scroll to preserve the anchor point. Used by both image viewer (via applyAnchoredImageViewerZoomChange delegation) and diagram surface. |
| src/renderer/src/components/editor/diagram-surface-zoom.ts | Diagram-specific constants and helpers (bounds, step, pan scroll math, keyboard intent mapping). getDiagramSurfaceKeyboardZoomIntent correctly distinguishes darwin/win32/linux modifier keys and rejects alt-modified combinations. Well tested. |
| src/renderer/src/components/editor/mermaid-text-diagram-view-modes.ts | Shared mode enum and option generator for Code/Split/Chart. Prevents mode-string duplication across MermaidViewer and RichMarkdownMermaidViews. Simple and correct. |
| src/renderer/src/components/editor/EditorEditFileSurface.tsx | Small change to pass readOnly, onContentChange, and onSave to MermaidViewer. The triple-ternary guard (activeFile.readOnly === true ? undefined : handler) correctly suppresses editing callbacks for read-only files. |
| src/renderer/src/components/editor/RichMarkdownCodeBlock.tsx | Replaces the inline MermaidBlock preview with RichMarkdownMermaidViews when isMermaid. NodeViewContent is now delegated to the child component, which is valid Tiptap usage. Container class rich-markdown-mermaid-code-block provides the inline-size container query context for the split layout. |
| src/renderer/src/components/editor/MermaidViewer.test.tsx | New tests covering: live rerender in split mode, code-mode source editability, auto-switch from chart on first keystroke in empty file, draft preservation on echo, external reload acceptance, read-only immutability and save-shortcut suppression, and file-switch diagram flush. Good coverage of the critical state-sync edge cases. |
Sequence Diagram
sequenceDiagram
participant User
participant MV as MermaidViewer
participant DM as useDebouncedMermaid
participant ZDS as ZoomableDiagramSurface
participant MB as MermaidBlock
User->>MV: Open .mmd file
MV->>MV: "useState(content), mode='chart'"
MV->>DM: useDebouncedMermaidDiagramContent(trimmed, filePath)
DM-->>MV: renderContent (initial)
User->>MV: Type in textarea (handleSourceChange)
MV->>MV: "setDraftContent(next), lastEmittedRef=next"
MV->>MV: onContentChange(next)
MV->>MV: if empty→chart auto-switches to split
MV->>DM: content changes (trimmed)
DM->>DM: setTimeout(250ms)
DM-->>MV: debouncedContent (stale during typing)
Note over DM: Timer fires after 250ms idle
DM->>MV: renderContent updated
MV->>ZDS: "diagramKey=renderContent"
ZDS->>MB: "content=renderContent"
MB-->>ZDS: SVG rendered
ZDS->>ZDS: MutationObserver → updateDiagramSize
ZDS->>ZDS: computeLayoutSize, fitScale×zoom
User->>ZDS: Ctrl+wheel / keyboard ± / toolbar buttons
ZDS->>ZDS: applyAnchoredSurfaceZoomChange (flushSync)
ZDS->>ZDS: adjust scrollLeft/scrollTop for anchor
User->>ZDS: Drag to pan
ZDS->>ZDS: setPointerCapture, track panDragRef
ZDS->>ZDS: getDraggedDiagramSurfaceScrollPosition
Note over MV,ZDS: File switch (key=activeFile.id remounts)
MV->>MV: "remount, fresh state, mode='chart'"
MV->>DM: "resetKey=new filePath → immediate flush"
MV->>ZDS: "resetKey=filePath → setZoom(1)"
Reviews (5): Last reviewed commit: "Distinguish echoes from external content..." | Re-trigger Greptile
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
Priority: Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 17 files. (1 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly identifies the main change: adding editable Mermaid diagram views. It is concise and specific. Description check The description is complete and aligned with the template. It includes the user impact, technical changes, rationale, issue reference, visual proof, testing results, AI disclosure, review notes, compa… Full details: Description check
-
Explanation
The description is complete and aligned with the template. It includes the user impact, technical changes, rationale, issue reference, visual proof, testing results, AI disclosure, review notes, compatibility considerations, and checklist status. Minor gaps remain: the linked issue is referenced as “Related:
#22372” instead of using the template’s “Fixes #” format with a direct issue link, and the visual proof provides an after image but no before image.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 17 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
src/renderer/src/components/editor/ZoomableDiagramSurface.tsx (1)
193-210: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider dropping
childrenfrom the deps to avoid observer churn.
childrenis a fresh element object on every parent render, so this effect tears down and recreates theMutationObservereach time even though the DOM subtree is unchanged. The observer (plusdiagramKey) already covers content replacement.♻️ Suggested change
- }, [children, diagramKey, updateDiagramSize]) + }, [diagramKey, updateDiagramSize])src/renderer/src/components/editor/MermaidViewer.tsx (2)
18-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMode type and toggle options are duplicated across both Mermaid views. Both files declare an identical
MermaidTextDiagramModeunion and an identical three-entry option list with lazytranslategetters, differing only in the i18n key prefix; the root cause is the absence of a shared module for the view-mode contract.
src/renderer/src/components/editor/MermaidViewer.tsx#L18-L39: import the mode type and options from a new shared module (e.g.mermaid-diagram-view-modes.ts) instead of redeclaring them.src/renderer/src/components/editor/RichMarkdownMermaidViews.tsx#L9-L30: import the same shared definitions, passing its own i18n key prefix if the keys must stay distinct.
133-142: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUndebounced source text drives
mermaid.render()on every keystroke in both views. Each component passes the live editor text straight toMermaidBlock(and asdiagramKey), so every character queues a full parse/render through the serialized mermaid queue.
src/renderer/src/components/editor/MermaidViewer.tsx#L133-L142: debouncetrimmedContent(~200-300ms) before handing it todiagramKey/MermaidBlock, keeping the textarea itself controlled bydraftContent.src/renderer/src/components/editor/RichMarkdownMermaidViews.tsx#L73-L85: apply the same debounce tocontent(node.textContent) before rendering the diagram.src/renderer/src/assets/markdown-preview.css (1)
842-849: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStage padding duplicates
DIAGRAM_SURFACE_PADDING.
padding: 24pxhere must stay in sync withDIAGRAM_SURFACE_PADDING = 24indiagram-surface-zoom.ts, which drives the fit-scale math. A shared custom property (e.g.--diagram-surface-padding) or a comment cross-referencing the constant would prevent silent drift.src/renderer/src/components/editor/MermaidViewer.test.tsx (1)
58-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
readOnlypath.The new
readOnlyprop (and the suppressed save shortcut) is the behaviorEditorContentnow relies on for read-only tabs, but no test exercises it. A case asserting the textarea isreadOnlyand thatonSaveis never invoked on the save keybinding would lock that in.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 67a36d73-e7f5-454c-971f-4cae1d242e7a
📥 Commits
Reviewing files that changed from the base of the PR and between 857f285 and 6677fb0d4c3260332a1f4b1435022aaee701c19a.
📒 Files selected for processing (15)
src/renderer/src/assets/markdown-preview.csssrc/renderer/src/assets/rich-markdown-editor.csssrc/renderer/src/components/editor/EditorContent.tsxsrc/renderer/src/components/editor/MermaidViewer.test.tsxsrc/renderer/src/components/editor/MermaidViewer.tsxsrc/renderer/src/components/editor/RichMarkdownCodeBlock.tsxsrc/renderer/src/components/editor/RichMarkdownMermaidViews.tsxsrc/renderer/src/components/editor/ZoomableDiagramSurface.tsxsrc/renderer/src/components/editor/anchored-surface-dom-zoom.tssrc/renderer/src/components/editor/anchored-surface-zoom.tssrc/renderer/src/components/editor/diagram-surface-zoom.test.tssrc/renderer/src/components/editor/diagram-surface-zoom.tssrc/renderer/src/components/editor/image-viewer-dom-zoom.tssrc/renderer/src/components/editor/image-viewer-zoom.test.tssrc/renderer/src/components/editor/image-viewer-zoom.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/editor/MermaidViewer.tsx (1)
33-60: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDiagram briefly shows the previous file's content after switching files.
renderContent(line 43) is debounced uniformly for everytrimmedContentchange, including the one triggered by the file-switch sync effect (lines 53-60). SinceresetKey={filePath}onZoomableDiagramSurfaceresets zoom immediately butdiagramKey={renderContent}lags behind by up toMERMAID_RENDER_DEBOUNCE_MS(250ms), switching between two Mermaid files momentarily re-zooms onto the previous file's diagram before it snaps to the new one. This isn't covered by the current test suite (only same-file "echo" and same-file edits are tested).Suggested fix: make the debounce hook file-switch-aware so it flushes immediately when the reset key changes, mirroring the
resetKeypattern already used byZoomableDiagramSurface.🐛 Proposed fix: bypass debounce on file switch
- const renderContent = useDebouncedMermaidDiagramContent(trimmedContent) + const renderContent = useDebouncedMermaidDiagramContent(trimmedContent, filePath)And in
use-debounced-mermaid-diagram-content.ts:export function useDebouncedMermaidDiagramContent(content: string, resetKey?: unknown): string { const [debouncedContent, setDebouncedContent] = useState(content) const previousResetKeyRef = useRef(resetKey) useEffect(() => { if (resetKey !== previousResetKeyRef.current) { previousResetKeyRef.current = resetKey setDebouncedContent(content) return } if (content.length === 0) { setDebouncedContent('') return } const timer = window.setTimeout(() => { setDebouncedContent(content) }, MERMAID_RENDER_DEBOUNCE_MS) return () => window.clearTimeout(timer) }, [content, resetKey]) return debouncedContent }Want me to open an issue and/or add a regression test that switches
filePathwith differing content and asserts the diagram updates without a stale intermediate render?Also applies to: 43-43, 127-129
🧹 Nitpick comments (1)
src/renderer/src/assets/markdown-preview.css (1)
807-810: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
24pxinstead of a design token.As per coding guidelines, UI work under
src/renderer/src/**/*.{css,scss,tsx,ts}must use tokens fromsrc/renderer/src/assets/main.cssrather than inventing new design values.--diagram-surface-padding: 24pxintroduces a standalone value; consider sourcing it from an existing spacing token if one covers this size.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c9c81416-a659-4886-b807-404072ba498c
📥 Commits
Reviewing files that changed from the base of the PR and between 6677fb0d4c3260332a1f4b1435022aaee701c19a and 14b17851e4b29105abbb37c6d08cbb4958a8f3f3.
📒 Files selected for processing (7)
src/renderer/src/assets/markdown-preview.csssrc/renderer/src/components/editor/MermaidViewer.test.tsxsrc/renderer/src/components/editor/MermaidViewer.tsxsrc/renderer/src/components/editor/RichMarkdownMermaidViews.tsxsrc/renderer/src/components/editor/ZoomableDiagramSurface.tsxsrc/renderer/src/components/editor/mermaid-text-diagram-view-modes.tssrc/renderer/src/components/editor/use-debounced-mermaid-diagram-content.ts
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 956f78aa-5788-4422-b0c7-63cd00d1bf7f
📥 Commits
Reviewing files that changed from the base of the PR and between 14b17851e4b29105abbb37c6d08cbb4958a8f3f3 and cfdfe7381d34fb70c3c8013ffa98d8e2cfdce757.
📒 Files selected for processing (3)
src/renderer/src/components/editor/MermaidViewer.test.tsxsrc/renderer/src/components/editor/MermaidViewer.tsxsrc/renderer/src/components/editor/use-debounced-mermaid-diagram-content.ts
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fcafb179-fee9-4fd7-9a1a-b49ba2d94a71
📥 Commits
Reviewing files that changed from the base of the PR and between cfdfe7381d34fb70c3c8013ffa98d8e2cfdce757 and 53fae905a6f2a26aa67cc172308d912273e81af9.
📒 Files selected for processing (2)
src/renderer/src/components/editor/MermaidViewer.test.tsxsrc/renderer/src/components/editor/MermaidViewer.tsx
|
Hi @brennanb2025, gentle follow-up on this one. Happy to address any feedback or make changes if anything's blocking. Thanks for taking a look! |
53fae90 to
a873440
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@Pr1p We'll soon roll out a plugin system and this can be adapted to be a plugin. Will ping you when it's ready. |
a873440 to
1cfe11e
Compare
|
@coderabbitai review\n\nAddressed the latest draft-sync and immediate-save findings in 7357fb809e, with focused regression coverage. The PR description now follows the repository template and records the validation limits transparently. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
6bd4b675-3972-4c79-a0f6-659209830ef9
📥 Commits
Reviewing files that changed from the base of the PR and between a873440a91625f0e56fd0eaa29f9b3a1d3985649 and 7357fb809e4edd02d7348b0509149544c9579260.
📒 Files selected for processing (10)
src/renderer/src/assets/markdown-preview.csssrc/renderer/src/components/editor/EditorEditFileSurface.tsxsrc/renderer/src/components/editor/MermaidViewer.test.tsxsrc/renderer/src/components/editor/MermaidViewer.tsxsrc/renderer/src/components/editor/RichMarkdownCodeBlock.tsxsrc/renderer/src/components/editor/RichMarkdownMermaidViews.tsxsrc/renderer/src/components/editor/ZoomableDiagramSurface.tsxsrc/renderer/src/components/editor/mermaid-text-diagram-view-modes.tssrc/renderer/src/components/editor/use-scroll-top-cache-restore.test.tsxsrc/renderer/src/components/editor/use-scroll-top-cache-restore.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/src/assets/markdown-preview.css
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
|
Hey! Check #24189 |
|
@AmethystLiang Following up on your note from September ("we'll soon roll out a plugin system and this can be adapted to be a plugin — will ping you when it's ready"). I checked the current plugin manifest schema (v0 contribution points: panels, commands, events, languagePacks, keybindings, vmRecipes, agents) and didn't see a custom viewer/editor contribution point yet — which I assume is what this PR would need to become a plugin. Two quick questions so I can plan around it:
Happy to wait and adapt this into a plugin once the surface exists — just looking for an anchor so it doesn't sit in limbo. Thanks! |
1ad4e3c to
0cbbe44
Compare
0cbbe44 to
eb7290a
Compare
|
Closing — upstream now covers the half of this that mattered, and the rest is a direction the maintainers already answered differently. What this PR asked for was three view modes for Mermaid files and fences (Code / Split / Chart) plus a zoomable, pannable diagram surface. What main has today already renders The remaining piece — editing source in the same pane next to the live diagram, with debounced re-render and drag/keyboard zoom — is a real design choice rather than a bug fix, and it has since been made in the other direction: the editor keeps the diagram surface read-only and gives chat-side diagrams their own pan/zoom viewer (#27715, #30154). Re-proposing the split-edit layout now would mean re-litigating that, over a diff that is three months stale across 19 files, including If the maintainers want the split editing flow after all, that is worth a fresh design discussion — the zoom primitives in |
|
The plugin system's https://github.com/Pr1p/orca-mermaid-editor It carries the same shape this PR proposed — chart / split / source views with live rendering, debounced edits, conflict-safe saves — implemented against the Thanks @AmethystLiang for the September pointer that this would land as a plugin — closing this PR in favor of the plugin, which anyone can install today. |
ELI5
Mermaid files can now be viewed as code, a live code-and-diagram split, or a diagram. When editing is allowed, changing the text updates the diagram without losing the latest draft; users can zoom and pan complex diagrams.
What Changed
.mmd/.mermaidfiles and Mermaid fences in rich Markdown.Before, Mermaid content was only rendered in a limited static view. After, users can inspect and edit source alongside an interactive diagram. The shared surface reuses the existing image-viewer zoom behavior instead of creating a second implementation.
Why
Large diagrams are hard to inspect in a static preview, and editing source without a live view slows down iteration. Reusing one surface keeps zoom behavior consistent and reduces future maintenance. Draft acknowledgement is safer than accepting every same-file prop update, because asynchronous echoes can arrive after a newer edit.
Linked Issue
Related: #22372. This PR also adds editable Mermaid source and inline Markdown views, so it does not close that broader feature request by itself.
Visual Proof
Before: no Code / Split / Chart controls or interactive zoomable Mermaid surface.
After:
Testing
I manually tested these changes locally. Not run: no Electron UI launch was needed for this focused refresh; any future launch will use the project's background-only policy.
Automated tests added/updated.
vitest run --config config/vitest.config.ts MermaidViewer.test.tsx use-scroll-top-cache-restore.test.tsx diagram-surface-zoom.test.ts image-viewer-zoom.test.ts— 31 passed.tsc --noEmit -p config/tsconfig.tc.web.json --composite false— passed.check-changed-code-quality.mjs— standard, type-aware, React, and design-system checks reported 0 new findings. Its final ast-grep scan could not start locally because@ast-grep/cli/postinstall.jsis absent from this dependency installation.git diff --check— passed.Full repository lint, build, and CI checks remain for CI.
AI Disclosure
Used Codex (GPT-5) for implementation and test authoring; the final diff and test results were reviewed before push.
Review
Reviewed save shortcut timing, out-of-order parent echoes, read-only behavior, file-switch rendering, shared zoom interactions, text selection, and the newer pan/zoom dialog's wheel-delta dependency. The change adds no IPC, network, command execution, or new path handling. Keyboard zoom uses existing platform-aware helpers; the renderer-only implementation remains applicable to local, SSH, and folder workspaces.
Agent skill upstream boundary
Notes
The feature is renderer-local and uses existing sanitized Mermaid rendering. A future plugin system may choose a different integration point; this PR keeps the current editor behavior self-contained.
Checklist