feat(penpal): highlight expansion for images and mermaid diagrams#547
feat(penpal): highlight expansion for images and mermaid diagrams#547
Conversation
…pping Define the product requirement for expanding highlights to encompass entire images and mermaid diagrams when they intersect with text, and the corresponding engineering requirement for the rehype plugin behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…PENPAL-HIGHLIGHT-MEDIA) Expand highlight marks to encompass entire images and mermaid diagrams when the highlight also spans adjacent text. Mermaid blocks use annotation (dataMermaidHighlight on <code>) + CSS classes instead of <mark> wrapping to avoid breaking imperative SVG rendering. Adds mermaidCrossed flag to continuation state so post-mermaid text matching handles SVG-polluted remaining text from sel.toString(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rmaid selection behaviors Broaden P-PENPAL-HIGHLIGHT-MEDIA to cover selections starting at or dragging out of diagrams. Update E-PENPAL-HIGHLIGHT-MEDIA with rehype plugin mermaid-start handling and MermaidSelection drag-escape. Cross-ref E-PENPAL-SVG-DRAG with the escape-to-text transition. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…am (E-PENPAL-HIGHLIGHT-MEDIA) Two fixes for mermaid + highlight interactions: 1. Rehype plugin: highlights whose startLine falls on a mermaid block now annotate the diagram and schedule continuation with mermaidCrossed, so text before/after the diagram is also highlighted. Previously the mermaid handler SKIP'd before reaching the "start new highlights" path. 2. MermaidSelection: dragging outside the container cancels SVG rect mode and transitions to a text selection (via setBaseAndExtent + extend) that includes the whole diagram, handing off to SelectionToolbar. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… (E-PENPAL-HIGHLIGHT-MEDIA) - Remove dead isMermaid check in handleCodeBlock (caller already SKIPs) - Media-only block: only SKIP when a wrap actually happened - Guard wrapInlineMedia behind continuationMatched flag - Add MarkdownViewer integration test for mermaid highlight class - Add rehype test for startLine lineOffset resolving to mermaid - Update TESTING.md with MarkdownViewer coverage and e2e drag-out gap Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…AL-HIGHLIGHT-MEDIA) Closes the e2e coverage gap: verifies that dragging from inside a mermaid diagram to outside cancels the SVG rect and transitions to a text selection with the SelectionToolbar visible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ard drag-out - MarkdownViewer: verify pending-highlight class applied to mermaid container div (not just prose marks) - e2e: verify dragging upward out of mermaid into preceding text cancels SVG rect and creates text selection (isAbove branch) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00d9a89021
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const normSelected = normalizeSelected(highlight.selectedText); | ||
| if (normSelected.length >= 3) { | ||
| continuing.set(highlight.threadId, { | ||
| highlight, |
There was a problem hiding this comment.
When a highlight starts on a mermaid block, this code always seeds continuing with the full selectedText, even if the user selected only the diagram. In that case, the next prose element is still eligible for continuation matching, so text after the diagram can be incorrectly marked if it shares a prefix with diagram labels (for example, a paragraph starting with "Login" after selecting a diagram containing "Login"). This causes false positive highlights outside the user’s selection.
Useful? React with 👍 / 👎.
| // 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); |
There was a problem hiding this comment.
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 👍 / 👎.
1 participant
Summary
🤖 Generated with Claude Code