feat(diff): rendered markdown preview toggle in the diff viewer#2431
feat(diff): rendered markdown preview toggle in the diff viewer#2431george-vii wants to merge 2 commits into
Conversation
Markdown files opened from the Changes/diff sidebar now get the same
Eye/Pencil toggle as the file editor, in the same position, reusing the
shared PreviewSourceToggle component:
- Pencil (default): the Monaco source diff — unchanged behavior
- Eye: the modified ("after") content rendered via MarkdownRenderer
- In split mode the preview shows the original (left) and modified
(right) rendered side by side, reusing the diff toolbar's
unified/split toggle
Deleted markdown files stay diff-only (no "after" content to render).
Extracts a shared computeDiffUris() helper so the Monaco diff editor and
the markdown preview read identical original/modified model URIs, and
adds optional previewLabel/sourceLabel props to PreviewSourceToggle
(defaults preserved).
Greptile SummaryThis PR adds an Eye/Pencil toggle to the diff viewer for Markdown files, mirroring the toggle already present in the file editor — letting reviewers flip between the raw source diff and the rendered Markdown without leaving the Changes panel. Deleted Markdown files are correctly excluded (no "after" content to render), and split mode shows original ∥ modified rendered side-by-side.
Confidence Score: 4/5Safe to merge; the change is well-contained and follows established patterns in the codebase. The implementation is solid — MobX wiring is correct, model URI sharing avoids duplication, and the ShowHide mount-preservation pattern mirrors what the file editor already does. Two non-blocking concerns exist: the preview renders blank content with no loading indicator while git/PR models are still being fetched, and StickyDiffEditor has no explicit layout() call for when its ShowHide parent transitions back to visible, relying entirely on Monaco's ResizeObserver. Both are worth a quick manual verification but neither blocks correctness. diff-file-renderer.tsx — verify the Monaco editor re-draws correctly after toggling back from preview to source, and consider adding a loading state for the rendered preview.
|
| Filename | Overview |
|---|---|
| apps/emdash-desktop/src/renderer/features/tasks/diff-view/main-panel/diff-file-renderer.tsx | Adds MarkdownDiffRenderer and MarkdownDiffPreview components; extracts computeDiffUris helper; two P2 concerns: blank preview while models load and no explicit Monaco re-layout after ShowHide reveal. |
| apps/emdash-desktop/src/renderer/features/tasks/tabs/diff-tab-store.ts | Adds observable showRendered field and setShowRendered action; correctly wired into MobX makeObservable. |
| apps/emdash-desktop/src/renderer/features/tasks/tabs/diff-tab-store.test.ts | New unit test covering default state and MobX reaction for showRendered; clean and correct. |
| apps/emdash-desktop/src/renderer/lib/editor/preview-source-toggle.tsx | Adds optional previewLabel/sourceLabel props with defaults preserved; backwards-compatible change. |
Sequence Diagram
sequenceDiagram
participant User
participant MarkdownDiffRenderer
participant ShowHide
participant MonacoDiffRenderer
participant StickyDiffEditor
participant MarkdownDiffPreview
participant ModelRegistry
User->>MarkdownDiffRenderer: open .md diff tab
MarkdownDiffRenderer->>ShowHide: "visible=true (Pencil mode)"
ShowHide->>MonacoDiffRenderer: mount (display: contents)
MonacoDiffRenderer->>ModelRegistry: registerModel(originalUri, modifiedUri)
MonacoDiffRenderer->>StickyDiffEditor: render diff editor
User->>MarkdownDiffRenderer: click Eye toggle
MarkdownDiffRenderer->>ShowHide: "visible=false (hide Monaco)"
Note over ShowHide,StickyDiffEditor: Monaco stays mounted, models remain registered
MarkdownDiffRenderer->>MarkdownDiffPreview: mount
MarkdownDiffPreview->>ModelRegistry: getModelByUri(originalUri) → oldContent
MarkdownDiffPreview->>ModelRegistry: getModelByUri(modifiedUri) → newContent
MarkdownDiffPreview->>User: render MarkdownRenderer(newContent)
User->>MarkdownDiffRenderer: click Pencil toggle
MarkdownDiffRenderer->>ShowHide: "visible=true (reveal Monaco)"
MarkdownDiffRenderer->>MarkdownDiffPreview: unmount
ShowHide->>MonacoDiffRenderer: display: contents → Monaco re-layouts via ResizeObserver
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/emdash-desktop/src/renderer/features/tasks/diff-view/main-panel/diff-file-renderer.tsx:293-298
**Empty preview while models are still loading**
When the user switches to the Eye view before the underlying Monaco models have finished loading (common for `git`/`pr` diffs that need a network round-trip), `getModelByUri` returns `null` and both `newContent`/`oldContent` resolve to `''`. The reactive subscriptions on `_origStatus`/`_modStatus` will eventually re-render once the models reach `'ready'`, but in the meantime the user sees a blank white panel with no indication that content is pending. The existing `MarkdownEditorRenderer` avoids this problem because the buffer model is pre-loaded before the renderer mounts; the diff preview has no such guarantee.
### Issue 2 of 2
apps/emdash-desktop/src/renderer/features/tasks/diff-view/main-panel/diff-file-renderer.tsx:262-278
**`MonacoDiffRenderer` layout not explicitly refreshed after `ShowHide` reveal**
`ShowHide` hides Monaco using `display: none` and reveals it via `display: contents`. `StickyDiffEditor` calls `editor.layout()` only inside the autorun that fires when models are first attached. If the user switches back to the source diff without the URIs changing — the common case — that autorun will not re-run and `editor.layout()` is never called. Monaco's internal `ResizeObserver` typically handles this, but if the ResizeObserver callback lags the diff editor can momentarily render at zero height until the user resizes the window. The existing `FileRenderer`/`MonacoFileRenderer` pairing uses the same `ShowHide` pattern and works in practice, so this may be fine — but it is a new consumer of the pattern for `StickyDiffEditor` specifically and worth a quick manual check.
Reviews (1): Last reviewed commit: "feat(diff): add rendered markdown previe..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const _origStatus = modelRegistry.modelStatus.get(originalUri); | ||
| const _modStatus = modelRegistry.modelStatus.get(modifiedUri); | ||
| const _bufferVersion = modelRegistry.bufferVersions.get(uri); | ||
|
|
||
| const newContent = modelRegistry.getModelByUri(modifiedUri)?.getValue() ?? ''; | ||
| const oldContent = modelRegistry.getModelByUri(originalUri)?.getValue() ?? ''; |
There was a problem hiding this comment.
When the user switches to the Eye view before the underlying Monaco models have finished loading (common for git/pr diffs that need a network round-trip), getModelByUri returns null and both newContent/oldContent resolve to ''. The reactive subscriptions on _origStatus/_modStatus will eventually re-render once the models reach 'ready', but in the meantime the user sees a blank white panel with no indication that content is pending. The existing MarkdownEditorRenderer avoids this problem because the buffer model is pre-loaded before the renderer mounts; the diff preview has no such guarantee.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/emdash-desktop/src/renderer/features/tasks/diff-view/main-panel/diff-file-renderer.tsx
Line: 293-298
Comment:
**Empty preview while models are still loading**
When the user switches to the Eye view before the underlying Monaco models have finished loading (common for `git`/`pr` diffs that need a network round-trip), `getModelByUri` returns `null` and both `newContent`/`oldContent` resolve to `''`. The reactive subscriptions on `_origStatus`/`_modStatus` will eventually re-render once the models reach `'ready'`, but in the meantime the user sees a blank white panel with no indication that content is pending. The existing `MarkdownEditorRenderer` avoids this problem because the buffer model is pre-loaded before the renderer mounts; the diff preview has no such guarantee.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const MarkdownDiffRenderer = observer(function MarkdownDiffRenderer({ | ||
| tab, | ||
| }: DiffFileRendererProps) { | ||
| return ( | ||
| <div className="relative h-full w-full overflow-hidden"> | ||
| <ShowHide visible={!tab.showRendered}> | ||
| <MonacoDiffRenderer tab={tab} /> | ||
| </ShowHide> | ||
| {tab.showRendered && <MarkdownDiffPreview tab={tab} />} | ||
| <PreviewSourceToggle | ||
| activeMode={tab.showRendered ? 'preview' : 'source'} | ||
| onSwitch={(mode) => tab.setShowRendered(mode === 'preview')} | ||
| sourceLabel="View diff" | ||
| /> | ||
| </div> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
MonacoDiffRenderer layout not explicitly refreshed after ShowHide reveal
ShowHide hides Monaco using display: none and reveals it via display: contents. StickyDiffEditor calls editor.layout() only inside the autorun that fires when models are first attached. If the user switches back to the source diff without the URIs changing — the common case — that autorun will not re-run and editor.layout() is never called. Monaco's internal ResizeObserver typically handles this, but if the ResizeObserver callback lags the diff editor can momentarily render at zero height until the user resizes the window. The existing FileRenderer/MonacoFileRenderer pairing uses the same ShowHide pattern and works in practice, so this may be fine — but it is a new consumer of the pattern for StickyDiffEditor specifically and worth a quick manual check.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/emdash-desktop/src/renderer/features/tasks/diff-view/main-panel/diff-file-renderer.tsx
Line: 262-278
Comment:
**`MonacoDiffRenderer` layout not explicitly refreshed after `ShowHide` reveal**
`ShowHide` hides Monaco using `display: none` and reveals it via `display: contents`. `StickyDiffEditor` calls `editor.layout()` only inside the autorun that fires when models are first attached. If the user switches back to the source diff without the URIs changing — the common case — that autorun will not re-run and `editor.layout()` is never called. Monaco's internal `ResizeObserver` typically handles this, but if the ResizeObserver callback lags the diff editor can momentarily render at zero height until the user resizes the window. The existing `FileRenderer`/`MonacoFileRenderer` pairing uses the same `ShowHide` pattern and works in practice, so this may be fine — but it is a new consumer of the pattern for `StickyDiffEditor` specifically and worth a quick manual check.
How can I resolve this? If you propose a fix, please make it concise.
|
Thanks for contributing this, the direction fits nicely with the existing markdown/file preview work. One correctness concern before merge: the rendered markdown preview needs to stay tied to the same source as the diff. Right now the preview reads markdown from the Monaco diff models, but linked images are loaded from the current workspace filesystem. For staged/git/PR diffs that can show images from the wrong state, or miss images that exist in the index/ref being previewed. It would be better to resolve images from the same side/source being rendered: filesystem for working tree, index for staged, and getImageAtRef for commit/PR refs. Also, please make sure the preview re-renders when the underlying diff models are invalidated or refreshed. The current observable dependencies look like they may miss disk model content changes, so the source diff could update while the rendered preview remains stale. The UI placement and reuse of existing renderer/toggle pieces looks good; I think these two data-source/reactivity fixes would make it much safer. |
Addresses review feedback on the markdown diff preview: - Resolve linked images from the same source as the rendered side (working tree, index, or ref) via readImage / getImageAtIndex / getImageAtRef, instead of always reading the working tree. Previously staged/git/PR previews could show images from the wrong state. - Read content from the diff models and keep it in sync via onDidChangeContent, so the preview tracks index/disk model reloads instead of going stale when the source diff updates. - Show a delayed loading spinner while the diff models are still loading instead of a blank panel.
|
Hey @arnestrickmann, no worries at all — and thanks for the thorough review! Both points are addressed: 1. Per-side image source. Linked images now resolve from the same source as the side being rendered, mirroring the original/modified ref selection used for the diff text:
So staged/git/PR previews read images from the matching ref instead of the working tree. One caveat worth flagging: on SSH workspaces the git-side reads return 2. Reactivity / staleness. The preview now reads content from the diff models and stays in sync via Also folded in a small delayed loading spinner (per the Greptile note) so the preview shows a spinner instead of a blank pane while models are still fetching. |
2 participants
What
Adds an Eye/Pencil toggle to markdown files opened from the Changes/diff sidebar —
the same toggle (and position) the file editor already uses for markdown — letting
you flip between the source diff and the rendered markdown.
MarkdownRendererdiff toolbar's existing unified/split toggle
Deleted markdown files stay diff-only (no "after" content to render).
Why
The rendered markdown view was only reachable from the file explorer, not when
reviewing changes — so reviewing a
.mddiff meant reading raw source.How
PreviewSourceToggle,MarkdownRenderer, and theShowHidemount-preservation pattern (keeps Monaco's diff models registered so the preview
can read their content).
computeDiffUris()helper as the single source of truth for theoriginal/modified model URIs (shared by the Monaco diff editor and the preview).
previewLabel/sourceLabelprops toPreviewSourceToggle(defaults preserved).
Screenshots
Pencil (default) — source diff:
Eye — rendered preview:
Eye + Split — original ∥ modified, rendered:
Testing
pnpm run typecheck,lint,format:check, andtestall pass.