Skip to content
5 changes: 4 additions & 1 deletion apps/penpal/ERD.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,14 @@ see-also:
- <a id="E-PENPAL-HIGHLIGHT-CROSS"></a>**E-PENPAL-HIGHLIGHT-CROSS**: The rehype plugin splits and wraps `<mark>` elements across inline formatting boundaries (bold, italic, code, links) and across block boundaries between code blocks and prose. Each contiguous segment of highlighted text gets its own `<mark>` wrapper so the highlight renders correctly regardless of intervening HTML structure.
← [P-PENPAL-HIGHLIGHT-CROSS](PRODUCT.md#P-PENPAL-HIGHLIGHT-CROSS)

- <a id="E-PENPAL-HIGHLIGHT-MEDIA"></a>**E-PENPAL-HIGHLIGHT-MEDIA**: The rehype plugin expands highlights to encompass entire media elements. Inline images sandwiched between `<mark>` elements with the same `threadId` are wrapped in a matching `<mark>`. Block-level image paragraphs are wrapped during continuation. Mermaid `<pre>` blocks are annotated (not wrapped) via `dataMermaidHighlight` on the `<code>` element — MarkdownViewer reads this to add CSS highlight classes without changing tree structure, preserving the imperatively-rendered SVG. When a highlight's `startLine` falls on a mermaid block, the plugin annotates the mermaid and schedules the full `selectedText` for continuation with `mermaidCrossed: true`, which relaxes Strategy 3 matching thresholds so post-mermaid text is found despite SVG label pollution in `remaining`. Highlighted mermaid containers render with a yellow `border` + `box-shadow` instead of a background overlay. `MermaidSelection` detects when a drag leaves the container bounds and transitions from SVG rect mode to a programmatic text selection (via `Selection.setBaseAndExtent` + `Selection.extend`) that includes the whole diagram, handing off to `SelectionToolbar` on mouseup.
← [P-PENPAL-HIGHLIGHT-MEDIA](PRODUCT.md#P-PENPAL-HIGHLIGHT-MEDIA)

---

## Mermaid Diagram Anchoring

- <a id="E-PENPAL-SVG-DRAG"></a>**E-PENPAL-SVG-DRAG**: `MermaidSelection` handles drag on `.mermaid-container` elements. After a 5px movement threshold, a live `.penpal-pending-svg-highlight` rect tracks the selection. On mouseup, SVG coordinates are computed.
- <a id="E-PENPAL-SVG-DRAG"></a>**E-PENPAL-SVG-DRAG**: `MermaidSelection` handles drag on `.mermaid-container` elements. After a 5px movement threshold, a live `.penpal-pending-svg-highlight` rect tracks the selection. On mouseup, SVG coordinates are computed. If the mouse leaves the container bounds during drag, SVG rect mode is cancelled and the drag transitions to a text selection that includes the whole diagram (see [E-PENPAL-HIGHLIGHT-MEDIA](#E-PENPAL-HIGHLIGHT-MEDIA)).
← [P-PENPAL-DIAGRAM-SELECT](PRODUCT.md#P-PENPAL-DIAGRAM-SELECT)

- <a id="E-PENPAL-SVG-EXTRACT"></a>**E-PENPAL-SVG-EXTRACT**: The SVG snippet is extracted by cloning the SVG, setting a cropped `viewBox`, scaling to max 300x200px, and re-IDing all elements with a random prefix to prevent DOM ID collisions. CSS `url(#id)` and `href="#id"` references are rewritten.
Expand Down
2 changes: 2 additions & 0 deletions apps/penpal/PRODUCT.md
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,8 @@ Global views aggregate content across all projects. They appear as top-level ite

- <a id="P-PENPAL-HIGHLIGHT-CROSS"></a>**P-PENPAL-HIGHLIGHT-CROSS**: A single highlight can span across formatting boundaries — bold, italic, code, links, and other inline markup — as well as cross between code blocks and surrounding prose.

- <a id="P-PENPAL-HIGHLIGHT-MEDIA"></a>**P-PENPAL-HIGHLIGHT-MEDIA**: A highlight that intersects an image or mermaid diagram expands to encompass the entire media element. Selecting text that spans into, through, or starting within a diagram highlights the diagram with a visible yellow border and highlights the adjacent text normally. Dragging a selection out of a mermaid diagram transitions from rectangle selection to text selection, including the whole diagram in the resulting highlight.

---

## Mermaid Diagram Comments
Expand Down
1 change: 1 addition & 0 deletions apps/penpal/TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ see-also:
| Anchor Resolution (P-PENPAL-ANCHOR-RESOLVE) | comments_test.go (implicit via round-trip) | rehypeCommentHighlights.test.ts | — | — |
| Comment Highlights (P-PENPAL-HIGHLIGHT) | — | rehypeCommentHighlights.test.ts, MarkdownViewer.test.tsx | — | review-workflow.spec.ts |
| Cross-Element Highlights (P-PENPAL-HIGHLIGHT-CROSS) | — | rehypeCommentHighlights.test.ts | — | — |
| Media Highlight Expansion (P-PENPAL-HIGHLIGHT-MEDIA) | — | rehypeCommentHighlights.test.ts, MarkdownViewer.test.tsx | — | mermaid-comments.spec.ts |
| Mermaid Diagram Comments (P-PENPAL-DIAGRAM-SELECT) | — | — | — | mermaid-comments.spec.ts |
| Comment Threads (P-PENPAL-THREAD-PANEL, REPLY, STATES) | comments_test.go | CommentsPanel.test.tsx | api_threads_test.go | review-workflow.spec.ts |
| Comment Ordering (E-PENPAL-COMMENT-ORDER) | ordering_test.go | utils/comments.test.ts | — | — |
Expand Down
105 changes: 105 additions & 0 deletions apps/penpal/e2e/tests/mermaid-comments.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,111 @@ test.describe('mermaid diagram commenting', () => {
await expect(highlight).not.toBeAttached({ timeout: 5000 });
});

// E-PENPAL-HIGHLIGHT-MEDIA: verifies dragging out of a mermaid diagram cancels SVG rect
// and transitions to text selection that includes the whole diagram.
test('dragging out of mermaid diagram switches to text selection', async ({ page }) => {
await blockPendingNavigation(page);
await page.goto(`/file/${projectName}/${filePath}`);

// Wait for mermaid to render
const container = page.locator('.mermaid-container');
await expect(container).toBeVisible({ timeout: 10000 });
const svg = container.locator('svg');
await expect(svg).toBeVisible();

const svgBox = await svg.boundingBox();
expect(svgBox).toBeTruthy();

// Find the "Text after" paragraph below the diagram
const afterText = page.locator('p', { hasText: 'Text after the diagram' });
await expect(afterText).toBeVisible();
const afterBox = await afterText.boundingBox();
expect(afterBox).toBeTruthy();

// Start drag inside the SVG (center area)
const startX = svgBox!.x + svgBox!.width * 0.5;
const startY = svgBox!.y + svgBox!.height * 0.5;
await page.mouse.move(startX, startY);
await page.mouse.down();

// Move past the 5px threshold (still inside SVG) to enter SVG rect mode
await page.mouse.move(startX + 10, startY + 10, { steps: 2 });

// Verify we entered SVG rect mode — pending rect should exist
const pendingRect = svg.locator('.penpal-pending-svg-highlight');
await expect(pendingRect).toBeAttached();

// Now drag below the container — this should trigger escape to text selection
const endX = afterBox!.x + afterBox!.width * 0.5;
const endY = afterBox!.y + afterBox!.height * 0.5;
await page.mouse.move(endX, endY, { steps: 5 });

// The SVG pending rect should be gone (cancelled on escape)
await expect(pendingRect).not.toBeAttached({ timeout: 2000 });

// Release the mouse on the content area to trigger SelectionToolbar
await page.mouse.up();

// A text selection should exist that includes the mermaid content
const selectionText = await page.evaluate(() => window.getSelection()?.toString().trim() ?? '');
expect(selectionText.length).toBeGreaterThan(0);

// The selection toolbar should appear
const toolbar = page.locator('.selection-toolbar');
await expect(toolbar).toBeVisible({ timeout: 5000 });
await expect(toolbar.locator('button', { hasText: 'Comment' })).toBeVisible();
});

// E-PENPAL-HIGHLIGHT-MEDIA: verifies dragging upward out of a mermaid diagram
// creates a text selection anchored at the end of the container.
test('dragging upward out of mermaid diagram switches to text selection', async ({ page }) => {
await blockPendingNavigation(page);
await page.goto(`/file/${projectName}/${filePath}`);

const container = page.locator('.mermaid-container');
await expect(container).toBeVisible({ timeout: 10000 });
const svg = container.locator('svg');
await expect(svg).toBeVisible();

const svgBox = await svg.boundingBox();
expect(svgBox).toBeTruthy();

// Find the "intro text" paragraph above the diagram
const beforeText = page.locator('p', { hasText: 'intro text before' });
await expect(beforeText).toBeVisible();
const beforeBox = await beforeText.boundingBox();
expect(beforeBox).toBeTruthy();

// Start drag inside the SVG (center area)
const startX = svgBox!.x + svgBox!.width * 0.5;
const startY = svgBox!.y + svgBox!.height * 0.5;
await page.mouse.move(startX, startY);
await page.mouse.down();

// Move past 5px threshold to enter SVG rect mode
await page.mouse.move(startX - 10, startY - 10, { steps: 2 });
const pendingRect = svg.locator('.penpal-pending-svg-highlight');
await expect(pendingRect).toBeAttached();

// Drag upward above the container into the text before
const endX = beforeBox!.x + beforeBox!.width * 0.5;
const endY = beforeBox!.y + beforeBox!.height * 0.5;
await page.mouse.move(endX, endY, { steps: 5 });

// SVG rect should be cancelled
await expect(pendingRect).not.toBeAttached({ timeout: 2000 });

await page.mouse.up();

// Text selection should exist
const selectionText = await page.evaluate(() => window.getSelection()?.toString().trim() ?? '');
expect(selectionText.length).toBeGreaterThan(0);

// SelectionToolbar should appear
const toolbar = page.locator('.selection-toolbar');
await expect(toolbar).toBeVisible({ timeout: 5000 });
});

// E-PENPAL-MD-RENDER: verifies normal text selection toolbar works alongside mermaid diagrams.
test('normal text selection still works alongside diagrams', async ({ page }) => {
await blockPendingNavigation(page);
Expand Down
28 changes: 28 additions & 0 deletions apps/penpal/frontend/src/components/MarkdownViewer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,34 @@ describe('MarkdownViewer', () => {
expect(mermaidAfter).toBe(mermaidBefore);
});

// E-PENPAL-HIGHLIGHT-MEDIA: mermaid container gets highlight class via annotation
it('applies comment-highlight class to mermaid container when highlight spans into it', () => {
const md = 'Before text\n\n```mermaid\ngraph TD\n A --> B\n```';
const highlights = [
{ threadId: 't-media', selectedText: 'Before text A B', startLine: 1 },
];
const { container } = render(
<MarkdownViewer content={md} rawMarkdown={md} highlights={highlights} />,
);
const mermaidContainer = container.querySelector('.mermaid-container.comment-highlight');
expect(mermaidContainer).not.toBeNull();
expect(mermaidContainer?.getAttribute('data-thread-id')).toBe('t-media');
});

// E-PENPAL-HIGHLIGHT-MEDIA: pending mermaid container gets pending-highlight class
it('applies pending-highlight class to mermaid container for pending highlights', () => {
const md = 'Before text\n\n```mermaid\ngraph TD\n A --> B\n```';
const highlights = [
{ threadId: 'pending', selectedText: 'Before text A B', startLine: 1, pending: true },
];
const { container } = render(
<MarkdownViewer content={md} rawMarkdown={md} highlights={highlights} />,
);
const mermaidContainer = container.querySelector('.mermaid-container.pending-highlight');
expect(mermaidContainer).not.toBeNull();
expect(mermaidContainer?.classList.contains('comment-highlight')).toBe(true);
});

it('renders pending highlights with pending-highlight class', () => {
const md = 'Hello world';
const highlights = [
Expand Down
21 changes: 20 additions & 1 deletion apps/penpal/frontend/src/components/MarkdownViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,31 @@ const MarkdownViewer = forwardRef<HTMLDivElement, MarkdownViewerProps>(
// Use AST node position to set data-source-line at render time,
// matching how the Go template sets it server-side.
const sourceLine = node?.position?.start?.line;

// E-PENPAL-HIGHLIGHT-MEDIA: read highlight annotation from rehype plugin.
// Applied as className (not a <mark> wrapper) to avoid changing tree
// structure, which would cause React to recreate the DOM node and lose
// the imperatively-rendered mermaid SVG.
let highlightClass = 'mermaid-container';
let highlightThreadId: string | undefined;
const mermaidHighlightRaw = node?.properties?.dataMermaidHighlight;
if (typeof mermaidHighlightRaw === 'string') {
try {
const parsed = JSON.parse(mermaidHighlightRaw) as { threadId: string; pending?: boolean };
highlightClass += parsed.pending
? ' comment-highlight pending-highlight'
: ' comment-highlight';
highlightThreadId = parsed.threadId;
} catch { /* ignore malformed */ }
}

return (
<div
className="mermaid-container"
className={highlightClass}
data-mermaid-source={String(children)}
data-unwrap-pre=""
{...(sourceLine ? { 'data-source-line': String(sourceLine) } : {})}
{...(highlightThreadId ? { 'data-thread-id': highlightThreadId } : {})}
>
<pre>
<code>{children}</code>
Expand Down
68 changes: 68 additions & 0 deletions apps/penpal/frontend/src/components/MermaidSelection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,74 @@ export default function MermaidSelection({
const dy = e2.clientY - startY;
if (!dragging && Math.sqrt(dx * dx + dy * dy) < 5) return;

// E-PENPAL-HIGHLIGHT-MEDIA: when mouse leaves the mermaid container
// during drag, cancel SVG rect mode and switch to text selection
// that includes the whole diagram plus whatever text the user drags to.
const containerRect = containerEl.getBoundingClientRect();
const outsideContainer =
e2.clientX < containerRect.left || e2.clientX > containerRect.right ||
e2.clientY < containerRect.top || e2.clientY > containerRect.bottom;

if (outsideContainer) {
if (!dragging) {
// Never entered SVG mode — just cancel our tracking and let browser handle
document.removeEventListener('mousemove', onMouseMove);
document.removeEventListener('mouseup', onMouseUp);
dragActiveRef.current = false;
if (draggingRef) draggingRef.current = false;
return;
}

// Was in SVG mode — switch to text selection including the whole diagram
if (overlayRect) { overlayRect.remove(); overlayRect = null; }
containerEl.style.userSelect = '';
dragging = false;

// Create a text selection from the mermaid container to the mouse position
const sel = window.getSelection();
sel?.removeAllRanges();
const caretRange = document.caretRangeFromPoint(e2.clientX, e2.clientY);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add fallback when creating caret range from pointer position

The drag-to-text handoff calls document.caretRangeFromPoint(...) directly, but that API is non-standard and absent in some browser engines (notably Firefox, which uses caretPositionFromPoint). In those environments this throws at runtime on drag-out, breaking mermaid selection handoff entirely instead of showing the text-selection toolbar.

Useful? React with 👍 / 👎.


if (sel && caretRange) {
const isAbove = e2.clientY < containerRect.top;
if (isAbove) {
// Dragging up: anchor at end of container, focus at mouse position
sel.setBaseAndExtent(
containerEl, containerEl.childNodes.length,
caretRange.startContainer, caretRange.startOffset,
);
} else {
// Dragging down/sideways: anchor at start of container, focus at mouse position
sel.setBaseAndExtent(
containerEl, 0,
caretRange.startContainer, caretRange.startOffset,
);
}
}

// Replace SVG handlers with text-extension handlers
document.removeEventListener('mousemove', onMouseMove);
document.removeEventListener('mouseup', onMouseUp);

const onTextMove = (e3: MouseEvent) => {
const cr = document.caretRangeFromPoint(e3.clientX, e3.clientY);
if (cr && sel) {
try { sel.extend(cr.startContainer, cr.startOffset); } catch { /* ignore */ }
}
};
const onTextUp = () => {
document.removeEventListener('mousemove', onTextMove);
document.removeEventListener('mouseup', onTextUp);
dragActiveRef.current = false;
if (draggingRef) draggingRef.current = false;
// SelectionToolbar's mouseup handler on contentEl will fire and
// show the comment button for the text+diagram selection.
};
document.addEventListener('mousemove', onTextMove);
document.addEventListener('mouseup', onTextUp);
return;
}

if (!dragging) {
dragging = true;
dragActiveRef.current = true;
Expand Down
Loading