Skip to content

Commit 9d765f6

Browse files
loganjclaude
andauthored
feat(penpal): highlight expansion for images and mermaid diagrams (#547)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent dbd8a49 commit 9d765f6

10 files changed

Lines changed: 843 additions & 12 deletions

File tree

apps/penpal/ERD.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,14 @@ see-also:
136136
- <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.
137137
[P-PENPAL-HIGHLIGHT-CROSS](PRODUCT.md#P-PENPAL-HIGHLIGHT-CROSS)
138138

139+
- <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.
140+
[P-PENPAL-HIGHLIGHT-MEDIA](PRODUCT.md#P-PENPAL-HIGHLIGHT-MEDIA)
141+
139142
---
140143

141144
## Mermaid Diagram Anchoring
142145

143-
- <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.
146+
- <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)).
144147
[P-PENPAL-DIAGRAM-SELECT](PRODUCT.md#P-PENPAL-DIAGRAM-SELECT)
145148

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

apps/penpal/PRODUCT.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,8 @@ Global views aggregate content across all projects. They appear as top-level ite
236236

237237
- <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.
238238

239+
- <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.
240+
239241
---
240242

241243
## Mermaid Diagram Comments

apps/penpal/TESTING.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ see-also:
7575
| Anchor Resolution (P-PENPAL-ANCHOR-RESOLVE) | comments_test.go (implicit via round-trip) | rehypeCommentHighlights.test.ts |||
7676
| Comment Highlights (P-PENPAL-HIGHLIGHT) || rehypeCommentHighlights.test.ts, MarkdownViewer.test.tsx || review-workflow.spec.ts |
7777
| Cross-Element Highlights (P-PENPAL-HIGHLIGHT-CROSS) || rehypeCommentHighlights.test.ts |||
78+
| Media Highlight Expansion (P-PENPAL-HIGHLIGHT-MEDIA) || rehypeCommentHighlights.test.ts, MarkdownViewer.test.tsx || mermaid-comments.spec.ts |
7879
| Mermaid Diagram Comments (P-PENPAL-DIAGRAM-SELECT) |||| mermaid-comments.spec.ts |
7980
| Comment Threads (P-PENPAL-THREAD-PANEL, REPLY, STATES) | comments_test.go | CommentsPanel.test.tsx | api_threads_test.go | review-workflow.spec.ts |
8081
| Comment Ordering (E-PENPAL-COMMENT-ORDER) | ordering_test.go | utils/comments.test.ts |||

apps/penpal/e2e/tests/mermaid-comments.spec.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,111 @@ test.describe('mermaid diagram commenting', () => {
217217
await expect(highlight).not.toBeAttached({ timeout: 5000 });
218218
});
219219

220+
// E-PENPAL-HIGHLIGHT-MEDIA: verifies dragging out of a mermaid diagram cancels SVG rect
221+
// and transitions to text selection that includes the whole diagram.
222+
test('dragging out of mermaid diagram switches to text selection', async ({ page }) => {
223+
await blockPendingNavigation(page);
224+
await page.goto(`/file/${projectName}/${filePath}`);
225+
226+
// Wait for mermaid to render
227+
const container = page.locator('.mermaid-container');
228+
await expect(container).toBeVisible({ timeout: 10000 });
229+
const svg = container.locator('svg');
230+
await expect(svg).toBeVisible();
231+
232+
const svgBox = await svg.boundingBox();
233+
expect(svgBox).toBeTruthy();
234+
235+
// Find the "Text after" paragraph below the diagram
236+
const afterText = page.locator('p', { hasText: 'Text after the diagram' });
237+
await expect(afterText).toBeVisible();
238+
const afterBox = await afterText.boundingBox();
239+
expect(afterBox).toBeTruthy();
240+
241+
// Start drag inside the SVG (center area)
242+
const startX = svgBox!.x + svgBox!.width * 0.5;
243+
const startY = svgBox!.y + svgBox!.height * 0.5;
244+
await page.mouse.move(startX, startY);
245+
await page.mouse.down();
246+
247+
// Move past the 5px threshold (still inside SVG) to enter SVG rect mode
248+
await page.mouse.move(startX + 10, startY + 10, { steps: 2 });
249+
250+
// Verify we entered SVG rect mode — pending rect should exist
251+
const pendingRect = svg.locator('.penpal-pending-svg-highlight');
252+
await expect(pendingRect).toBeAttached();
253+
254+
// Now drag below the container — this should trigger escape to text selection
255+
const endX = afterBox!.x + afterBox!.width * 0.5;
256+
const endY = afterBox!.y + afterBox!.height * 0.5;
257+
await page.mouse.move(endX, endY, { steps: 5 });
258+
259+
// The SVG pending rect should be gone (cancelled on escape)
260+
await expect(pendingRect).not.toBeAttached({ timeout: 2000 });
261+
262+
// Release the mouse on the content area to trigger SelectionToolbar
263+
await page.mouse.up();
264+
265+
// A text selection should exist that includes the mermaid content
266+
const selectionText = await page.evaluate(() => window.getSelection()?.toString().trim() ?? '');
267+
expect(selectionText.length).toBeGreaterThan(0);
268+
269+
// The selection toolbar should appear
270+
const toolbar = page.locator('.selection-toolbar');
271+
await expect(toolbar).toBeVisible({ timeout: 5000 });
272+
await expect(toolbar.locator('button', { hasText: 'Comment' })).toBeVisible();
273+
});
274+
275+
// E-PENPAL-HIGHLIGHT-MEDIA: verifies dragging upward out of a mermaid diagram
276+
// creates a text selection anchored at the end of the container.
277+
test('dragging upward out of mermaid diagram switches to text selection', async ({ page }) => {
278+
await blockPendingNavigation(page);
279+
await page.goto(`/file/${projectName}/${filePath}`);
280+
281+
const container = page.locator('.mermaid-container');
282+
await expect(container).toBeVisible({ timeout: 10000 });
283+
const svg = container.locator('svg');
284+
await expect(svg).toBeVisible();
285+
286+
const svgBox = await svg.boundingBox();
287+
expect(svgBox).toBeTruthy();
288+
289+
// Find the "intro text" paragraph above the diagram
290+
const beforeText = page.locator('p', { hasText: 'intro text before' });
291+
await expect(beforeText).toBeVisible();
292+
const beforeBox = await beforeText.boundingBox();
293+
expect(beforeBox).toBeTruthy();
294+
295+
// Start drag inside the SVG (center area)
296+
const startX = svgBox!.x + svgBox!.width * 0.5;
297+
const startY = svgBox!.y + svgBox!.height * 0.5;
298+
await page.mouse.move(startX, startY);
299+
await page.mouse.down();
300+
301+
// Move past 5px threshold to enter SVG rect mode
302+
await page.mouse.move(startX - 10, startY - 10, { steps: 2 });
303+
const pendingRect = svg.locator('.penpal-pending-svg-highlight');
304+
await expect(pendingRect).toBeAttached();
305+
306+
// Drag upward above the container into the text before
307+
const endX = beforeBox!.x + beforeBox!.width * 0.5;
308+
const endY = beforeBox!.y + beforeBox!.height * 0.5;
309+
await page.mouse.move(endX, endY, { steps: 5 });
310+
311+
// SVG rect should be cancelled
312+
await expect(pendingRect).not.toBeAttached({ timeout: 2000 });
313+
314+
await page.mouse.up();
315+
316+
// Text selection should exist
317+
const selectionText = await page.evaluate(() => window.getSelection()?.toString().trim() ?? '');
318+
expect(selectionText.length).toBeGreaterThan(0);
319+
320+
// SelectionToolbar should appear
321+
const toolbar = page.locator('.selection-toolbar');
322+
await expect(toolbar).toBeVisible({ timeout: 5000 });
323+
});
324+
220325
// E-PENPAL-MD-RENDER: verifies normal text selection toolbar works alongside mermaid diagrams.
221326
test('normal text selection still works alongside diagrams', async ({ page }) => {
222327
await blockPendingNavigation(page);

apps/penpal/frontend/src/components/MarkdownViewer.test.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,34 @@ describe('MarkdownViewer', () => {
164164
expect(mermaidAfter).toBe(mermaidBefore);
165165
});
166166

167+
// E-PENPAL-HIGHLIGHT-MEDIA: mermaid container gets highlight class via annotation
168+
it('applies comment-highlight class to mermaid container when highlight spans into it', () => {
169+
const md = 'Before text\n\n```mermaid\ngraph TD\n A --> B\n```';
170+
const highlights = [
171+
{ threadId: 't-media', selectedText: 'Before text A B', startLine: 1 },
172+
];
173+
const { container } = render(
174+
<MarkdownViewer content={md} rawMarkdown={md} highlights={highlights} />,
175+
);
176+
const mermaidContainer = container.querySelector('.mermaid-container.comment-highlight');
177+
expect(mermaidContainer).not.toBeNull();
178+
expect(mermaidContainer?.getAttribute('data-thread-id')).toBe('t-media');
179+
});
180+
181+
// E-PENPAL-HIGHLIGHT-MEDIA: pending mermaid container gets pending-highlight class
182+
it('applies pending-highlight class to mermaid container for pending highlights', () => {
183+
const md = 'Before text\n\n```mermaid\ngraph TD\n A --> B\n```';
184+
const highlights = [
185+
{ threadId: 'pending', selectedText: 'Before text A B', startLine: 1, pending: true },
186+
];
187+
const { container } = render(
188+
<MarkdownViewer content={md} rawMarkdown={md} highlights={highlights} />,
189+
);
190+
const mermaidContainer = container.querySelector('.mermaid-container.pending-highlight');
191+
expect(mermaidContainer).not.toBeNull();
192+
expect(mermaidContainer?.classList.contains('comment-highlight')).toBe(true);
193+
});
194+
167195
it('renders pending highlights with pending-highlight class', () => {
168196
const md = 'Hello world';
169197
const highlights = [

apps/penpal/frontend/src/components/MarkdownViewer.tsx

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,31 @@ const MarkdownViewer = forwardRef<HTMLDivElement, MarkdownViewerProps>(
230230
// Use AST node position to set data-source-line at render time,
231231
// matching how the Go template sets it server-side.
232232
const sourceLine = node?.position?.start?.line;
233+
234+
// E-PENPAL-HIGHLIGHT-MEDIA: read highlight annotation from rehype plugin.
235+
// Applied as className (not a <mark> wrapper) to avoid changing tree
236+
// structure, which would cause React to recreate the DOM node and lose
237+
// the imperatively-rendered mermaid SVG.
238+
let highlightClass = 'mermaid-container';
239+
let highlightThreadId: string | undefined;
240+
const mermaidHighlightRaw = node?.properties?.dataMermaidHighlight;
241+
if (typeof mermaidHighlightRaw === 'string') {
242+
try {
243+
const parsed = JSON.parse(mermaidHighlightRaw) as { threadId: string; pending?: boolean };
244+
highlightClass += parsed.pending
245+
? ' comment-highlight pending-highlight'
246+
: ' comment-highlight';
247+
highlightThreadId = parsed.threadId;
248+
} catch { /* ignore malformed */ }
249+
}
250+
233251
return (
234252
<div
235-
className="mermaid-container"
253+
className={highlightClass}
236254
data-mermaid-source={String(children)}
237255
data-unwrap-pre=""
238256
{...(sourceLine ? { 'data-source-line': String(sourceLine) } : {})}
257+
{...(highlightThreadId ? { 'data-thread-id': highlightThreadId } : {})}
239258
>
240259
<pre>
241260
<code>{children}</code>

apps/penpal/frontend/src/components/MermaidSelection.tsx

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,74 @@ export default function MermaidSelection({
189189
const dy = e2.clientY - startY;
190190
if (!dragging && Math.sqrt(dx * dx + dy * dy) < 5) return;
191191

192+
// E-PENPAL-HIGHLIGHT-MEDIA: when mouse leaves the mermaid container
193+
// during drag, cancel SVG rect mode and switch to text selection
194+
// that includes the whole diagram plus whatever text the user drags to.
195+
const containerRect = containerEl.getBoundingClientRect();
196+
const outsideContainer =
197+
e2.clientX < containerRect.left || e2.clientX > containerRect.right ||
198+
e2.clientY < containerRect.top || e2.clientY > containerRect.bottom;
199+
200+
if (outsideContainer) {
201+
if (!dragging) {
202+
// Never entered SVG mode — just cancel our tracking and let browser handle
203+
document.removeEventListener('mousemove', onMouseMove);
204+
document.removeEventListener('mouseup', onMouseUp);
205+
dragActiveRef.current = false;
206+
if (draggingRef) draggingRef.current = false;
207+
return;
208+
}
209+
210+
// Was in SVG mode — switch to text selection including the whole diagram
211+
if (overlayRect) { overlayRect.remove(); overlayRect = null; }
212+
containerEl.style.userSelect = '';
213+
dragging = false;
214+
215+
// Create a text selection from the mermaid container to the mouse position
216+
const sel = window.getSelection();
217+
sel?.removeAllRanges();
218+
const caretRange = document.caretRangeFromPoint(e2.clientX, e2.clientY);
219+
220+
if (sel && caretRange) {
221+
const isAbove = e2.clientY < containerRect.top;
222+
if (isAbove) {
223+
// Dragging up: anchor at end of container, focus at mouse position
224+
sel.setBaseAndExtent(
225+
containerEl, containerEl.childNodes.length,
226+
caretRange.startContainer, caretRange.startOffset,
227+
);
228+
} else {
229+
// Dragging down/sideways: anchor at start of container, focus at mouse position
230+
sel.setBaseAndExtent(
231+
containerEl, 0,
232+
caretRange.startContainer, caretRange.startOffset,
233+
);
234+
}
235+
}
236+
237+
// Replace SVG handlers with text-extension handlers
238+
document.removeEventListener('mousemove', onMouseMove);
239+
document.removeEventListener('mouseup', onMouseUp);
240+
241+
const onTextMove = (e3: MouseEvent) => {
242+
const cr = document.caretRangeFromPoint(e3.clientX, e3.clientY);
243+
if (cr && sel) {
244+
try { sel.extend(cr.startContainer, cr.startOffset); } catch { /* ignore */ }
245+
}
246+
};
247+
const onTextUp = () => {
248+
document.removeEventListener('mousemove', onTextMove);
249+
document.removeEventListener('mouseup', onTextUp);
250+
dragActiveRef.current = false;
251+
if (draggingRef) draggingRef.current = false;
252+
// SelectionToolbar's mouseup handler on contentEl will fire and
253+
// show the comment button for the text+diagram selection.
254+
};
255+
document.addEventListener('mousemove', onTextMove);
256+
document.addEventListener('mouseup', onTextUp);
257+
return;
258+
}
259+
192260
if (!dragging) {
193261
dragging = true;
194262
dragActiveRef.current = true;

0 commit comments

Comments
 (0)