Unify markdown to a single renderer.

Author: cpojer

core/App.css

@@ -1623,141 +1623,53 @@ html[data-codiff-platform='darwin'] .sidebar {
 
 .codiff-markdown-preview:not(.editable) {
   color: var(--text);
-  font: 14px/1.55 var(--font-sans);
-  overflow-wrap: anywhere;
-  padding: 18px 24px 26px;
   white-space: normal;
-  word-break: normal;
-}
-
-.codiff-markdown-preview:not(.editable) > :first-child {
-  margin-top: 0;
-}
-
-.codiff-markdown-preview:not(.editable) > :last-child {
-  margin-bottom: 0;
-}
-
-.codiff-markdown-preview:not(.editable) h1,
-.codiff-markdown-preview:not(.editable) h2,
-.codiff-markdown-preview:not(.editable) h3,
-.codiff-markdown-preview:not(.editable) h4,
-.codiff-markdown-preview:not(.editable) h5,
-.codiff-markdown-preview:not(.editable) h6 {
-  color: var(--text);
-  font-weight: 700;
-  line-height: 1.2;
-  margin: 22px 0 10px;
-}
-
-.codiff-markdown-preview:not(.editable) h1 {
-  border-bottom: 1px solid var(--file-border);
-  font-size: 24px;
-  padding-bottom: 8px;
 }
 
-.codiff-markdown-preview:not(.editable) h2 {
-  border-bottom: 1px solid var(--file-border);
-  font-size: 20px;
-  padding-bottom: 6px;
-}
-
-.codiff-markdown-preview:not(.editable) h3 {
-  font-size: 17px;
-}
-
-.codiff-markdown-preview:not(.editable) p,
-.codiff-markdown-preview:not(.editable) blockquote,
-.codiff-markdown-preview:not(.editable) ol,
-.codiff-markdown-preview:not(.editable) ul,
-.codiff-markdown-preview:not(.editable) pre {
-  margin: 0 0 14px;
-}
-
-.codiff-markdown-preview:not(.editable) .codiff-markdown-added {
-  border-radius: 0 6px 6px 0;
-  position: relative;
+.codiff-markdown-preview-editor {
+  --codiff-readonly-markdown-accent: var(--tree-selection-focus);
+  --mdx-editor-accent: var(--codiff-readonly-markdown-accent);
+  --mdx-editor-accent-contrast: #fff;
+  --mdx-editor-accent-soft: var(--tree-selection-bg);
+  --mdx-editor-bg: transparent;
+  --mdx-editor-border: transparent;
+  --mdx-editor-code-bg: var(--inline-code-bg);
+  --mdx-editor-font: var(--font-sans);
+  --mdx-editor-font-size: 14px;
+  --mdx-editor-hover: var(--hover-wash);
+  --mdx-editor-line-height: 1.55;
+  --mdx-editor-mono-font: var(--font-mono);
+  --mdx-editor-muted: var(--muted);
+  --mdx-editor-padding: 18px 24px 26px;
+  --mdx-editor-selection: var(--tree-selection-bg);
+  --mdx-editor-text: var(--text);
+  display: block;
 }
 
-.codiff-markdown-preview:not(.editable) .codiff-markdown-added::before {
-  background: var(--diff-addition);
+.codiff-markdown-preview-editor .mdx-editor-content {
+  border: 0;
   border-radius: 0;
-  bottom: 0;
-  content: '';
-  left: 0;
-  position: absolute;
-  top: 0;
-  width: 3px;
-}
-
-.codiff-markdown-preview:not(.editable)
-  :is(h1, h2, h3, h4, h5, h6, p, blockquote, pre).codiff-markdown-added {
-  margin-inline: -10px;
-  padding: 6px 10px 6px 14px;
 }
 
-.codiff-markdown-preview:not(.editable) ol,
-.codiff-markdown-preview:not(.editable) ul {
-  padding-left: 22px;
-}
-
-.codiff-markdown-preview:not(.editable) :is(ol, ul).codiff-markdown-added {
-  margin-inline: -10px;
-  padding: 6px 10px 6px 34px;
+.codiff-markdown-preview-editor .mdx-editor-content a,
+.review-comment-codex-reply-markdown .mdx-editor-content a {
+  color: var(--codiff-readonly-markdown-accent);
+  text-decoration: none;
 }
 
-.codiff-markdown-preview:not(.editable) li + li {
-  margin-top: 4px;
+.codiff-markdown-preview-editor .mdx-editor-content a:hover,
+.review-comment-codex-reply-markdown .mdx-editor-content a:hover {
+  text-decoration: underline;
+  text-decoration-color: currentColor;
 }
 
-.codiff-markdown-preview:not(.editable) blockquote {
-  border-left: 3px solid var(--file-border);
+.codiff-readonly-markdown-loading {
+  align-items: center;
   color: var(--muted);
-  padding-left: 12px;
-}
-
-.codiff-markdown-preview:not(.editable) pre {
-  background: rgb(127 127 127 / 0.09);
-  border: 1px solid rgb(127 127 127 / 0.14);
-  border-radius: 8px;
-  corner-shape: squircle;
-  font: 13px/1.45 var(--font-mono);
-  overflow-x: hidden;
-  padding: 12px;
-  white-space: pre-wrap;
-  word-break: break-word;
-}
-
-.codiff-markdown-code-block {
-  background: rgb(127 127 127 / 0.09);
-  border-radius: 8px;
-  box-shadow: none;
-  corner-shape: squircle;
-  display: block;
-  margin: 0 0 14px;
-  max-inline-size: 100%;
-  min-inline-size: 0;
-  overflow: clip;
-}
-
-.codiff-markdown-preview:not(.editable) .codiff-markdown-code-added {
-  margin-inline: -10px;
-  padding-inline: 10px;
-}
-
-.codiff-markdown-preview:not(.editable) code {
-  font-family: var(--font-mono);
-}
-
-.codiff-markdown-preview:not(.editable) hr {
-  border: 0;
-  border-top: 1px solid var(--file-border);
-  margin: 20px 0;
-}
-
-.codiff-markdown-preview:not(.editable) hr.codiff-markdown-added {
-  margin-inline: -10px;
-  padding: 6px 10px;
+  display: flex;
+  font: 13px/1.4 var(--font-sans);
+  min-height: 180px;
+  padding: 24px;
 }
 
 .codiff-markdown-document-editor,
@@ -2688,6 +2600,38 @@ html[data-codiff-platform='darwin'] .sidebar {
   color: var(--danger);
 }
 
+.review-comment-codex-reply-markdown {
+  --codiff-readonly-markdown-accent: var(--tree-selection-focus);
+  --mdx-editor-accent: var(--codiff-readonly-markdown-accent);
+  --mdx-editor-accent-contrast: #fff;
+  --mdx-editor-accent-soft: var(--tree-selection-bg);
+  --mdx-editor-bg: transparent;
+  --mdx-editor-border: transparent;
+  --mdx-editor-code-bg: var(--inline-code-bg);
+  --mdx-editor-font: var(--font-sans);
+  --mdx-editor-font-size: 13px;
+  --mdx-editor-hover: var(--hover-wash);
+  --mdx-editor-line-height: 1.5;
+  --mdx-editor-mono-font: var(--font-mono);
+  --mdx-editor-muted: var(--muted);
+  --mdx-editor-padding: 0;
+  --mdx-editor-selection: var(--tree-selection-bg);
+  --mdx-editor-text: var(--text);
+  display: block;
+}
+
+.review-comment-codex-reply-markdown.codiff-readonly-markdown-loading {
+  min-height: 76px;
+  padding: 0;
+  place-content: center;
+}
+
+.review-comment-codex-reply-markdown .mdx-editor-content {
+  border: 0;
+  border-radius: 0;
+  min-height: 0;
+}
+
 .review-comment-codex-reply p,
 .review-comment-codex-reply ul,
 .review-comment-codex-reply pre {

core/__tests__/App-render.test.tsx

@@ -245,7 +245,7 @@ const renderAppForOpenFileShortcut = async (file: ChangedFile) => {
 
   await waitFor(() => {
     expect(container.querySelector('.loading')).toBeNull();
-    expect(container.querySelector('.codiff-file-header.selected')).not.toBeNull();
+    expect(container.querySelector('.codiff-file-header')).not.toBeNull();
   });
 
   return {
@@ -433,9 +433,11 @@ test('repository reload restores the selected file when it still exists', async
   } satisfies RepositoryState;
 
   writeReloadSelection(nextState, secondFile.path);
+  const openFile = vi.fn(async () => {});
 
   window.codiff = createCodiffMock({
     getRepositoryState: vi.fn(async () => nextState),
+    openFile,
   });
 
   const container = document.createElement('div');
@@ -450,10 +452,13 @@ test('repository reload restores the selected file when it still exists', async
 
     await waitFor(() => {
       expect(container.querySelector('.loading')).toBeNull();
-      expect(
-        container.querySelector('.codiff-file-header.selected .codiff-file-path')?.textContent,
-      ).toBe(secondFile.path);
+      expect(container.querySelector('.codiff-file-header')).not.toBeNull();
+    });
+    await act(async () => {
+      dispatchModK();
+      await new Promise((resolve) => setTimeout(resolve, 0));
     });
+    expect(openFile).toHaveBeenCalledWith(secondFile.path);
   } finally {
     if (root) {
       await act(async () => root?.unmount());
@@ -477,9 +482,11 @@ test('repository reload restores the selected file from the previous source', as
   );
 
   writeReloadSelection(nextState, secondFile.path);
+  const openFile = vi.fn(async () => {});
 
   window.codiff = createCodiffMock({
     getRepositoryState,
+    openFile,
   });
 
   const container = document.createElement('div');
@@ -494,10 +501,13 @@ test('repository reload restores the selected file from the previous source', as
 
     await waitFor(() => {
       expect(container.querySelector('.loading')).toBeNull();
-      expect(
-        container.querySelector('.codiff-file-header.selected .codiff-file-path')?.textContent,
-      ).toBe(secondFile.path);
+      expect(container.querySelector('.codiff-file-header')).not.toBeNull();
+    });
+    await act(async () => {
+      dispatchModK();
+      await new Promise((resolve) => setTimeout(resolve, 0));
     });
+    expect(openFile).toHaveBeenCalledWith(secondFile.path);
     expect(getRepositoryState).toHaveBeenCalledWith(source);
   } finally {
     if (root) {

core/__tests__/App.test.tsx

@@ -1,4 +1,3 @@
-import { renderToStaticMarkup } from 'react-dom/server';
 import { expect, test } from 'vite-plus/test';
 import { getDiffSearchResult } from '../lib/diff-search.ts';
 import {
@@ -11,7 +10,6 @@ import {
   shouldLoadDiffSectionContents,
 } from '../lib/diff.ts';
 import { isDiffSearchShortcut } from '../lib/keyboard.ts';
-import { renderMarkdown } from '../lib/markdown.tsx';
 import {
   buildReviewCommentsMarkdown,
   shouldDiscardReviewCommentOnEscape,
@@ -373,7 +371,7 @@ test('total diff line counts sum countable files only', () => {
   });
 });
 
-test('markdown previews track added source lines for modified files', () => {
+test('markdown previews use new file contents for modified files', () => {
   const file = {
     fingerprint: 'markdown-preview-added-lines',
     path: 'README.md',
@@ -401,71 +399,6 @@ test('markdown previews track added source lines for modified files', () => {
   const preview = getMarkdownPreviewContents(file, section, fileDiff);
 
   expect(preview?.contents).toBe(file.sections[0].newFile?.contents);
-  expect([...(preview?.addedLines ?? [])]).toEqual([2, 3, 5]);
-});
-
-test('markdown previews do not mark every line for new files', () => {
-  const file = {
-    fingerprint: 'markdown-preview-new-file',
-    path: 'README.md',
-    sections: [
-      {
-        binary: false,
-        id: 'README.md:staged',
-        kind: 'staged',
-        loadState: 'ready',
-        newFile: {
-          contents: '# Title\nAll new.\n',
-          name: 'README.md',
-        },
-        oldFile: {
-          contents: '',
-          name: 'README.md',
-        },
-        patch: '',
-      },
-    ],
-    status: 'added',
-  } satisfies ChangedFile;
-  const [{ fileDiff, section }] = getVisibleDiffSections(file, false);
-
-  const preview = getMarkdownPreviewContents(file, section, fileDiff);
-
-  expect(preview?.addedLines.size).toBe(0);
-});
-
-test('rendered markdown marks blocks intersecting added source lines', () => {
-  const html = renderToStaticMarkup(
-    <>
-      {renderMarkdown(
-        [
-          '# Title',
-          '',
-          'paragraph line one',
-          'paragraph line two',
-          '',
-          '- kept',
-          '- added',
-          '',
-          '> quoted',
-          '',
-          '---',
-          '',
-          '```ts',
-          'const value = true;',
-          '```',
-        ].join('\n'),
-        { addedLines: new Set([4, 7, 9, 11, 14]) },
-      )}
-    </>,
-  );
-
-  expect(html.match(/codiff-markdown-added/g)).toHaveLength(5);
-  expect(html).toContain('<p class="codiff-markdown-added">');
-  expect(html).toContain('<ul class="codiff-markdown-added">');
-  expect(html).toContain('<blockquote class="codiff-markdown-added">');
-  expect(html).toContain('<hr class="codiff-markdown-added"/>');
-  expect(html).toContain('<div class="codiff-markdown-code-added codiff-markdown-added"><pre>');
 });
 
 test('diff search shortcut does not claim fullscreen shortcut', () => {