Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,24 @@ import {
useWorkspaceId,
useWorkspaceViewModel,
} from '@renderer/features/tasks/task-view-context';
import { getFileKind } from '@renderer/lib/editor/fileKind';
import { PreviewSourceToggle } from '@renderer/lib/editor/preview-source-toggle';
import { useDelayedBoolean } from '@renderer/lib/hooks/use-delay-boolean';
import { rpc } from '@renderer/lib/ipc';
import { modelRegistry } from '@renderer/lib/monaco/monaco-model-registry';
import { buildMonacoModelPath } from '@renderer/lib/monaco/monacoModelPath';
import { StickyDiffEditor } from '@renderer/lib/monaco/sticky-diff-editor';
import { MarkdownRenderer } from '@renderer/lib/ui/markdown-renderer';
import { ShowHide } from '@renderer/lib/ui/show-hide';
import { Spinner } from '@renderer/lib/ui/spinner';
import { getLanguageFromPath } from '@renderer/utils/languageUtils';
import { gitRefToString, HEAD_REF, STAGED_REF, type GitObjectRef } from '@shared/core/git/git';
import {
gitRefToString,
HEAD_REF,
STAGED_REF,
type GitObjectRef,
type GitRef,
} from '@shared/core/git/git';
import { getDraftCommentTargetKey, type DraftCommentTarget } from '@shared/lineComments';
import type { ActiveFile } from '@shared/view-state';

Expand All @@ -32,8 +45,15 @@ export const DiffFileRenderer = observer(function DiffFileRenderer({ tab }: Diff
const workspaceId = useWorkspaceId();

switch (tab.renderer.kind) {
case 'text':
case 'text': {
// Markdown files get a rendered-preview toggle (Eye) alongside the source
// diff (Pencil), mirroring the file-tab markdown renderer. Deleted files
// have no "after" content to render, so they stay diff-only.
if (getFileKind(tab.path) === 'markdown' && tab.status !== 'deleted') {
return <MarkdownDiffRenderer tab={tab} />;
}
return <MonacoDiffRenderer tab={tab} />;
}
case 'image': {
const activeFile = tabToActiveFile(tab);
return (
Expand Down Expand Up @@ -105,30 +125,8 @@ const MonacoDiffRenderer = observer(function MonacoDiffRenderer({ tab }: DiffFil
onDeleteComment: handleDeleteComment,
});

const root = `workspace:${workspaceId}`;
const uri = buildMonacoModelPath(root, tab.path);
const language = getLanguageFromPath(tab.path);

const originalUri = (() => {
if (tab.diffGroup === 'disk') {
return modelRegistry.toGitUri(uri, STAGED_REF);
}
if (tab.diffGroup === 'git' || tab.diffGroup === 'pr') {
return modelRegistry.toGitUri(uri, tab.originalRef);
}
return modelRegistry.toGitUri(uri, HEAD_REF);
})();

const modifiedUri = (() => {
if (tab.diffGroup === 'staged') return modelRegistry.toGitUri(uri, STAGED_REF);
if (tab.diffGroup === 'pr') {
return modelRegistry.toGitUri(uri, tab.modifiedRef ?? HEAD_REF);
}
if (tab.diffGroup === 'git') {
return modelRegistry.toGitUri(uri, tab.modifiedRef ?? HEAD_REF);
}
return uri;
})();
const { root, uri, originalUri, modifiedUri } = computeDiffUris(tab, workspaceId);

useEffect(() => {
let disposed = false;
Expand Down Expand Up @@ -233,6 +231,221 @@ const MonacoDiffRenderer = observer(function MonacoDiffRenderer({ tab }: DiffFil
);
});

/**
* Computes the original/modified Monaco model URIs for a diff tab. Shared by the
* Monaco diff editor and the markdown preview so both read identical content.
*/
function computeDiffUris(
tab: DiffTabStore,
workspaceId: string
): { root: string; uri: string; originalUri: string; modifiedUri: string } {
const root = `workspace:${workspaceId}`;
const uri = buildMonacoModelPath(root, tab.path);

const originalUri = (() => {
if (tab.diffGroup === 'disk') return modelRegistry.toGitUri(uri, STAGED_REF);
if (tab.diffGroup === 'git' || tab.diffGroup === 'pr') {
return modelRegistry.toGitUri(uri, tab.originalRef);
}
return modelRegistry.toGitUri(uri, HEAD_REF);
})();

const modifiedUri = (() => {
if (tab.diffGroup === 'staged') return modelRegistry.toGitUri(uri, STAGED_REF);
if (tab.diffGroup === 'pr') return modelRegistry.toGitUri(uri, tab.modifiedRef ?? HEAD_REF);
if (tab.diffGroup === 'git') return modelRegistry.toGitUri(uri, tab.modifiedRef ?? HEAD_REF);
return uri;
})();

return { root, uri, originalUri, modifiedUri };
}

/**
* Renders a markdown diff tab with a toggle between the source diff (Pencil) and
* a rendered markdown preview (Eye), mirroring the file-tab MarkdownEditorRenderer.
*
* The Monaco diff editor is kept mounted via ShowHide while the preview is shown,
* so its models stay registered and the preview can read their content.
*/
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>
);
});
Comment on lines +270 to +286

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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.


/** Where a rendered side's linked images come from, matching its diff source. */
type ImageSource = { kind: 'disk' } | { kind: 'index' } | { kind: 'ref'; ref: GitRef };

/**
* Resolves which source a rendered side's images come from, mirroring the
* original/modified ref selection in computeDiffUris so images stay consistent
* with the text being shown: working tree, index, or a specific git ref.
*/
function imageSourceForSide(tab: DiffTabStore, side: 'original' | 'modified'): ImageSource {
if (side === 'original') {
if (tab.diffGroup === 'disk') return { kind: 'index' };
if (tab.diffGroup === 'git' || tab.diffGroup === 'pr') {
return { kind: 'ref', ref: tab.originalRef };
}
return { kind: 'ref', ref: HEAD_REF };
}
if (tab.diffGroup === 'staged') return { kind: 'index' };
if (tab.diffGroup === 'git' || tab.diffGroup === 'pr') {
return { kind: 'ref', ref: tab.modifiedRef ?? HEAD_REF };
}
return { kind: 'disk' };
}

/** Resolves a relative markdown image against the correct side/source of the diff. */
async function resolveSideImage(
tab: DiffTabStore,
side: 'original' | 'modified',
projectId: string,
workspaceId: string,
fileDir: string,
src: string
): Promise<string | null> {
const imagePath = fileDir ? `${fileDir}/${src}` : src;
const source = imageSourceForSide(tab, side);
if (source.kind === 'disk') {
const res = await rpc.workspace.fs.readImage(projectId, workspaceId, imagePath);
return res.success ? (res.data?.dataUrl ?? null) : null;
}
const res =
source.kind === 'index'
? await rpc.workspace.git.getImageAtIndex(projectId, workspaceId, imagePath)
: await rpc.workspace.git.getImageAtRef(
projectId,
workspaceId,
imagePath,
gitRefToString(source.ref)
);
if (!res.success) return null;
return res.data.result.kind === 'image' ? res.data.result.image.dataUrl : null;
}

/**
* Renders the modified ("after") markdown content as a formatted preview. In
* split mode it shows the original (left) and modified (right) side by side,
* reusing the diff toolbar's unified/split toggle for consistency.
*
* Content is read from the Monaco diff models and kept in sync via
* onDidChangeContent, so the preview tracks model refreshes (e.g. index/disk
* reloads) instead of going stale. Linked images are resolved from the same
* source as the side being rendered.
*/
const MarkdownDiffPreview = observer(function MarkdownDiffPreview({ tab }: DiffFileRendererProps) {
const { projectId } = useTaskViewContext();
const workspaceId = useWorkspaceId();
const diffStyle = useWorkspaceViewModel().diffView?.diffStyle ?? 'unified';
const { originalUri, modifiedUri } = computeDiffUris(tab, workspaceId);

// Model load status drives the loading spinner and triggers the content
// listeners below to (re)attach once a model becomes available.
const originalStatus = modelRegistry.modelStatus.get(originalUri);
const modifiedStatus = modelRegistry.modelStatus.get(modifiedUri);

const [newContent, setNewContent] = useState('');
const [oldContent, setOldContent] = useState('');

// Read content imperatively and keep it in sync via onDidChangeContent rather
// than the file-tab markdown renderer's bufferVersions MobX dependency: the
// registry only bumps bufferVersions for the editable buffer model, never for
// git/index models, so a bufferVersions dependency would go stale when a
// staged/ref/PR diff reloads. onDidChangeContent also fires for those in-place
// setValue() refreshes, so it covers every side.
useEffect(() => {
const model = modelRegistry.getModelByUri(modifiedUri);
if (!model) {
setNewContent('');
return;
}
setNewContent(model.getValue());
const sub = model.onDidChangeContent(() => setNewContent(model.getValue()));
return () => sub.dispose();
}, [modifiedUri, modifiedStatus]);

useEffect(() => {
const model = modelRegistry.getModelByUri(originalUri);
if (!model) {
setOldContent('');
return;
}
setOldContent(model.getValue());
const sub = model.onDidChangeContent(() => setOldContent(model.getValue()));
return () => sub.dispose();
}, [originalUri, originalStatus]);

const fileDir = tab.path.includes('/') ? tab.path.substring(0, tab.path.lastIndexOf('/')) : '';
const resolveModifiedImage = useCallback(
(src: string) => resolveSideImage(tab, 'modified', projectId, workspaceId, fileDir, src),
[tab, projectId, workspaceId, fileDir]
);
const resolveOriginalImage = useCallback(
(src: string) => resolveSideImage(tab, 'original', projectId, workspaceId, fileDir, src),
[tab, projectId, workspaceId, fileDir]
);

const modifiedLoading = !modifiedStatus || modifiedStatus === 'loading';
const originalLoading = !originalStatus || originalStatus === 'loading';
const waiting = diffStyle === 'split' ? modifiedLoading || originalLoading : modifiedLoading;
const showSpinner = useDelayedBoolean(waiting, 200);

if (showSpinner) {
return (
<div className="flex h-full w-full items-center justify-center bg-background-secondary-1">
<Spinner />
</div>
);
}

if (diffStyle === 'split') {
return (
<div className="flex h-full w-full">
<div className="h-full flex-1 overflow-y-auto border-r border-border bg-background-secondary-1">
<MarkdownRenderer
content={oldContent}
variant="full"
className="w-full max-w-3xl px-8 py-8"
resolveImage={resolveOriginalImage}
/>
</div>
<div className="h-full flex-1 overflow-y-auto bg-background-secondary-1">
<MarkdownRenderer
content={newContent}
variant="full"
className="w-full max-w-3xl px-8 py-8"
resolveImage={resolveModifiedImage}
/>
</div>
</div>
);
}

return (
<div className="h-full w-full overflow-y-auto bg-background-secondary-1">
<MarkdownRenderer
content={newContent}
variant="full"
className="w-full max-w-3xl px-8 py-8"
resolveImage={resolveModifiedImage}
/>
</div>
);
});

function refShaOrString(ref: GitObjectRef | undefined): string {
if (!ref) return gitRefToString(HEAD_REF);
return ref.kind === 'commit' ? ref.sha : gitRefToString(ref);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { reaction } from 'mobx';
import { describe, expect, it } from 'vitest';
import type { ActiveFile } from '@shared/view-state';
import { DiffTabStore } from './diff-tab-store';

function makeActiveFile(path: string): ActiveFile {
return {
path,
type: 'disk',
group: 'disk',
originalRef: { kind: 'commit', sha: 'deadbeef' },
};
}

describe('DiffTabStore markdown preview toggle', () => {
it('defaults to the source diff (showRendered = false)', () => {
const tab = new DiffTabStore(makeActiveFile('docs/readme.md'), false);
expect(tab.renderer.kind).toBe('text');
expect(tab.showRendered).toBe(false);
});

it('setShowRendered toggles the rendered-preview state observably', () => {
const tab = new DiffTabStore(makeActiveFile('docs/readme.md'), false);
const seen: boolean[] = [];
const dispose = reaction(
() => tab.showRendered,
(value) => seen.push(value)
);

tab.setShowRendered(true);
tab.setShowRendered(false);
dispose();

expect(seen).toEqual([true, false]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ export class DiffTabStore {
path: string;
isPreview: boolean;
renderer: DiffRendererData;
/**
* For markdown diff tabs: whether to show the rendered markdown preview
* (Eye) instead of the source diff (Pencil). Defaults to the source diff.
* Ignored for non-markdown files.
*/
showRendered = false;
diffGroup: 'disk' | 'staged' | 'git' | 'pr';
originalRef: GitObjectRef;
modifiedRef: GitObjectRef | undefined;
Expand Down Expand Up @@ -57,8 +63,10 @@ export class DiffTabStore {
commitOriginalSha: observable,
commitModifiedSha: observable,
status: observable,
showRendered: observable,
transition: action,
pin: action,
setShowRendered: action,
});
}

Expand Down Expand Up @@ -86,6 +94,10 @@ export class DiffTabStore {
pin(): void {
this.isPreview = false;
}

setShowRendered(showRendered: boolean): void {
this.showRendered = showRendered;
}
}

function resolveDiffRenderer(path: string): DiffRendererData {
Expand Down
Loading