diff --git a/src/components/PlanViewerDialog.tsx b/src/components/PlanViewerDialog.tsx index af22096e..794d5352 100644 --- a/src/components/PlanViewerDialog.tsx +++ b/src/components/PlanViewerDialog.tsx @@ -1,4 +1,5 @@ -import { Show, For, createSignal, createEffect } from 'solid-js'; +import { Show, For, createSignal, createEffect, onCleanup } from 'solid-js'; +import { Portal } from 'solid-js/web'; import { Dialog } from './Dialog'; import { createDialogScroll } from '../lib/dialog-scroll'; import { ReviewProvider, useReview } from './ReviewProvider'; @@ -8,7 +9,15 @@ import { InlineInput } from './InlineInput'; import { AskCodeCard } from './AskCodeCard'; import { CloseIcon } from './icons'; import { createHighlightedMarkdown } from '../lib/marked-shiki'; -import { getPlanSelection } from '../lib/plan-selection'; +import { createReviewIdentity } from '../lib/diff-review-lifecycle'; +import { + getPlanSelection, + getPlanSelectionFlowAnchor, + getPlanSelectionTextRanges, + PLAN_REVIEW_FLOW_SLOT_SELECTOR, + trackPlanSelectionGeometry, + type PlanSelectionRect, +} from '../lib/plan-selection'; import { openFileInEditor } from '../lib/shell'; import { theme } from '../lib/theme'; import { sf } from '../lib/fontScale'; @@ -38,6 +47,12 @@ function compilePlanReview(annotations: ReviewAnnotation[]): string { } export function PlanViewerDialog(props: PlanViewerDialogProps) { + const reviewIdentity = () => + createReviewIdentity({ + taskId: props.taskId, + worktreePath: props.worktreePath ?? '', + }); + return ( - - - - - + + + ); } @@ -79,11 +94,27 @@ interface PlanViewerContentProps { } /** Inner content rendered inside ReviewProvider so it can call useReview(). */ -interface HighlightRect { - top: number; - left: number; - width: number; - height: number; +function insertPlanReviewFlowSlot(anchor: HTMLElement): HTMLDivElement { + const slot = document.createElement('div'); + slot.className = 'plan-review-flow-slot'; + slot.setAttribute('data-plan-review-flow-slot', ''); + + if (anchor.tagName === 'LI') { + const nestedList = Array.from(anchor.children).find((child) => child.matches('ul, ol')); + if (nestedList) { + anchor.insertBefore(slot, nestedList); + } else { + anchor.append(slot); + } + return slot; + } + + let insertionPoint: Element = anchor; + while (insertionPoint.nextElementSibling?.matches(PLAN_REVIEW_FLOW_SLOT_SELECTOR)) { + insertionPoint = insertionPoint.nextElementSibling; + } + insertionPoint.after(slot); + return slot; } function PlanViewerContent(props: PlanViewerContentProps) { @@ -93,9 +124,10 @@ function PlanViewerContent(props: PlanViewerContentProps) { let contentRef: HTMLDivElement | undefined; let scrollRef: HTMLDivElement | undefined; - const [selectionY, setSelectionY] = createSignal(0); - const [cardOffsets, setCardOffsets] = createSignal>({}); - const [highlightRects, setHighlightRects] = createSignal([]); + const [pendingFlowSlot, setPendingFlowSlot] = createSignal(); + const [flowSlots, setFlowSlots] = createSignal>({}); + const [highlightRects, setHighlightRects] = createSignal([]); + let stopHighlightTracking: (() => void) | undefined; createDialogScroll( () => scrollRef, @@ -126,47 +158,77 @@ function PlanViewerContent(props: PlanViewerContentProps) { createEffect(() => { const target = review.scrollTarget(); if (!target?.id) return; - const y = cardOffsets()[target.id]; - if (y !== undefined && scrollRef) { - scrollRef.scrollTo({ top: Math.max(0, y - 100), behavior: 'smooth' }); + const slot = flowSlots()[target.id]; + if (slot && scrollRef) { + const scrollRect = scrollRef.getBoundingClientRect(); + const slotRect = slot.getBoundingClientRect(); + const top = scrollRef.scrollTop + slotRect.top - scrollRect.top; + scrollRef.scrollTo({ top: Math.max(0, top - 100), behavior: 'smooth' }); + } + }); + + // Remove flow slots when their annotation or question is removed elsewhere (for example, + // from the review sidebar). + createEffect(() => { + const activeIds = new Set([ + ...review.annotations().map((annotation) => annotation.id), + ...review.activeQuestions().map((question) => question.id), + ]); + const currentSlots = flowSlots(); + const staleIds = Object.keys(currentSlots).filter((id) => !activeIds.has(id)); + if (staleIds.length === 0) return; + + for (const id of staleIds) { + currentSlots[id].remove(); } + setFlowSlots( + Object.fromEntries(Object.entries(currentSlots).filter(([id]) => activeIds.has(id))), + ); }); // Clear highlight overlays when pending selection is dismissed createEffect(() => { - if (!review.pendingSelection()) setHighlightRects([]); + if (!review.pendingSelection()) clearHighlightGeometry(); }); - /** Capture selection rects and Y offset relative to contentRef. */ - function captureSelectionGeometry(): { y: number; rects: HighlightRect[] } { - const domSel = window.getSelection(); - if (!domSel || domSel.rangeCount === 0 || !contentRef) return { y: 0, rects: [] }; - const range = domSel.getRangeAt(0); - const containerRect = contentRef.getBoundingClientRect(); - const rangeRect = range.getBoundingClientRect(); - const y = rangeRect.bottom - containerRect.top; - const clientRects = range.getClientRects(); - const rects: HighlightRect[] = []; - for (let i = 0; i < clientRects.length; i++) { - const r = clientRects[i]; - rects.push({ - top: r.top - containerRect.top, - left: r.left - containerRect.left, - width: r.width, - height: r.height, - }); + let previousPlanContent: string | undefined; + createEffect(() => { + const nextPlanContent = props.planContent; + if (previousPlanContent === undefined) { + previousPlanContent = nextPlanContent; + return; } - return { y, rects }; + if (nextPlanContent === previousPlanContent) return; + previousPlanContent = nextPlanContent; + pendingFlowSlot()?.remove(); + setPendingFlowSlot(undefined); + Object.values(flowSlots()).forEach((slot) => slot.remove()); + setFlowSlots({}); + clearHighlightGeometry(); + }); + + function clearHighlightGeometry() { + stopHighlightTracking?.(); + stopHighlightTracking = undefined; + setHighlightRects([]); } - function handleMouseUp() { + function handleMouseUp(event: MouseEvent) { if (!contentRef) return; + const eventTarget = event.target; + if (eventTarget instanceof Element && eventTarget.closest(PLAN_REVIEW_FLOW_SLOT_SELECTOR)) { + return; + } + const sel = getPlanSelection(contentRef, props.planFileName); - if (!sel) return; + const flowAnchor = getPlanSelectionFlowAnchor(contentRef); + const textRanges = getPlanSelectionTextRanges(contentRef); + if (!sel || !flowAnchor || textRanges.length === 0) return; - const { y, rects } = captureSelectionGeometry(); - setSelectionY(y); - setHighlightRects(rects); + stopHighlightTracking?.(); + pendingFlowSlot()?.remove(); + stopHighlightTracking = trackPlanSelectionGeometry(contentRef, textRanges, setHighlightRects); + setPendingFlowSlot(insertPlanReviewFlowSlot(flowAnchor)); // Clear native selection — overlay rects provide the visual highlight from here window.getSelection()?.removeAllRanges(); @@ -182,13 +244,46 @@ function PlanViewerContent(props: PlanViewerContentProps) { }); } - function handleSubmitWithPosition(text: string, mode: Parameters[1]) { - const y = selectionY(); + function handleSubmitInFlow(text: string, mode: Parameters[1]) { + const slot = pendingFlowSlot(); const id = review.handleSubmit(text, mode); - if (id) setCardOffsets((prev) => ({ ...prev, [id]: y })); - setHighlightRects([]); + if (!id) return; + if (slot) setFlowSlots((prev) => ({ ...prev, [id]: slot })); + setPendingFlowSlot(undefined); + clearHighlightGeometry(); } + function dismissPendingSelection() { + pendingFlowSlot()?.remove(); + setPendingFlowSlot(undefined); + review.clearPendingSelection(); + } + + function dismissAnnotation(id: string) { + review.dismissAnnotation(id); + removeFlowSlot(id); + } + + function dismissQuestion(id: string) { + review.dismissQuestion(id); + removeFlowSlot(id); + } + + function removeFlowSlot(id: string) { + const slot = flowSlots()[id]; + slot?.remove(); + setFlowSlots((prev) => { + if (!(id in prev)) return prev; + return Object.fromEntries(Object.entries(prev).filter(([slotId]) => slotId !== id)); + }); + } + + onCleanup(() => { + stopHighlightTracking?.(); + pendingFlowSlot()?.remove(); + Object.values(flowSlots()).forEach((slot) => slot.remove()); + }); + return ( <> {/* Header */} @@ -299,69 +394,52 @@ function PlanViewerContent(props: PlanViewerContentProps) { )} - {/* Inline input for pending selection — positioned near the selection */} - -
- -
+ {/* Inline input for pending selection — mounted after the selected block */} + + {(slot) => ( + + + + )} - {/* Annotation cards — positioned where the selection was made */} + {/* Annotation cards — mounted in document flow after the selected block */} {(annotation) => ( -
- review.dismissAnnotation(annotation.id)} - overlay - /> -
+ + {(slot) => ( + +
+ dismissAnnotation(annotation.id)} + /> +
+
+ )} +
)}
- {/* Active questions — positioned where the selection was made */} + {/* Active questions — mounted in document flow after the selected block */} {(q) => ( -
- review.dismissQuestion(q.id)} - /> -
+ + {(slot) => ( + + dismissQuestion(q.id)} + /> + + )} + )}
diff --git a/src/lib/plan-selection.client.test.tsx b/src/lib/plan-selection.client.test.tsx new file mode 100644 index 00000000..0a04b347 --- /dev/null +++ b/src/lib/plan-selection.client.test.tsx @@ -0,0 +1,310 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { Marked } from 'marked'; +import { + getPlanSelection, + getPlanSelectionFlowAnchor, + getPlanSelectionTextRanges, + PLAN_REVIEW_FLOW_SLOT_SELECTOR, + trackPlanSelectionGeometry, +} from './plan-selection'; + +afterEach(() => { + window.getSelection()?.removeAllRanges(); + document.body.replaceChildren(); + vi.unstubAllGlobals(); +}); + +function selectText(start: Text, end: Text): void { + const range = document.createRange(); + range.setStart(start, 0); + range.setEnd(end, end.length); + const selection = window.getSelection(); + selection?.removeAllRanges(); + selection?.addRange(range); +} + +function findTextNode(container: Node, value: string): Text { + const walker = document.createTreeWalker(container, NodeFilter.SHOW_TEXT); + let node = walker.nextNode(); + while (node) { + if (node.textContent === value) return node as Text; + node = walker.nextNode(); + } + throw new Error(`Could not find text node: ${value}`); +} + +function renderPlanMarkdown(markdown: string): HTMLDivElement { + const container = document.createElement('div'); + container.className = 'plan-markdown plan-markdown-dialog'; + container.innerHTML = new Marked().parse(markdown, { async: false }) as string; + document.body.append(container); + return container; +} + +function firstTextNodeIn(element: Element): Text { + const walker = document.createTreeWalker(element, NodeFilter.SHOW_TEXT); + const node = walker.nextNode(); + if (!node) throw new Error(`Could not find text in ${element.tagName}`); + return node as Text; +} + +function lastTextNodeIn(element: Element): Text { + const walker = document.createTreeWalker(element, NodeFilter.SHOW_TEXT); + let last: Node | null = null; + let node = walker.nextNode(); + while (node) { + last = node; + node = walker.nextNode(); + } + if (!last) throw new Error(`Could not find text in ${element.tagName}`); + return last as Text; +} + +function getNativeSelectionText(): string { + return window.getSelection()?.toString().trim() ?? ''; +} + +function selectAllRenderedText(container: HTMLElement): void { + selectText(firstTextNodeIn(container), lastTextNodeIn(container)); +} + +function rect(top: number, left = 0): DOMRect { + return { + x: left, + y: top, + top, + left, + right: left + 40, + bottom: top + 12, + width: 40, + height: 12, + toJSON: () => undefined, + }; +} + +describe('plan selection DOM behavior', () => { + // The full native-parity corpus runs in the browser-mode follow-up. Keeping + // it here documents the contract without making the happy-dom lane pretend + // to implement layout and CSS whitespace. + it.skip.each([ + { + name: 'paragraphs and headings', + markdown: '# Goal\n\nFirst paragraph.\n\n## Details\n\nSecond paragraph.', + }, + { + name: 'nested and loose lists', + markdown: '- Parent\n - Nested child\n\n- Loose item\n\n Continuation paragraph.', + }, + { + name: 'fenced code and blockquotes', + markdown: '> Quoted step\n\n```ts\nconst ready = true;\n```', + }, + { + name: 'inline emphasis, code, and links', + markdown: '**Bold** *emphasis* `inline code` [linked text](https://example.com)', + }, + { + name: 'soft and hard breaks', + markdown: 'Soft\nbreak \nHard break', + }, + { + name: 'tables', + markdown: '| Left | Right |\n| --- | --- |\n| A | B |\n| C | D |', + }, + ])('matches native selection for $name rendered by Marked', ({ markdown }) => { + const container = renderPlanMarkdown(markdown); + selectAllRenderedText(container); + + expect(getPlanSelection(container, 'plan.md')?.selectedText).toBe(getNativeSelectionText()); + }); + + it('excludes review-slot text from the selected prompt', () => { + const container = document.createElement('div'); + container.innerHTML = ` +

Before plan text

+

Review Goal Previous feedback

+

After plan text

+ `; + document.body.append(container); + const paragraphs = container.querySelectorAll('p'); + selectText(paragraphs[0].firstChild as Text, paragraphs[2].firstChild as Text); + + const selectedText = getPlanSelection(container, 'plan.md')?.selectedText ?? ''; + expect(selectedText).toContain('Before plan text'); + expect(selectedText).toContain('After plan text'); + expect(selectedText).not.toContain('Previous feedback'); + }); + + it('does not return highlight ranges inside a review slot', () => { + const container = document.createElement('div'); + container.innerHTML = ` +

Before plan text

+

Review Goal Previous feedback

+

After plan text

+ `; + document.body.append(container); + const paragraphs = container.querySelectorAll('p'); + selectText(paragraphs[0].firstChild as Text, paragraphs[2].firstChild as Text); + + const ranges = getPlanSelectionTextRanges(container); + expect(ranges.map((range) => range.toString()).join('')).not.toContain('Previous feedback'); + expect( + ranges.every( + (range) => + range.commonAncestorContainer.parentElement?.closest(PLAN_REVIEW_FLOW_SLOT_SELECTOR) === + null, + ), + ).toBe(true); + }); + + it('keeps block indices and the flow anchor on real plan content', () => { + const container = document.createElement('div'); + container.innerHTML = ` +

Before plan text

+

Review Goal Previous feedback

+

After plan text

+ `; + document.body.append(container); + const paragraphs = container.querySelectorAll('p'); + const range = document.createRange(); + range.setStart(paragraphs[0].firstChild as Text, 0); + range.setEnd(paragraphs[2].firstChild as Text, (paragraphs[2].textContent ?? '').length); + const selection = window.getSelection(); + selection?.removeAllRanges(); + selection?.addRange(range); + + expect(getPlanSelection(container, 'plan.md')).toMatchObject({ startLine: 0, endLine: 1 }); + expect(getPlanSelectionFlowAnchor(container)).toBe(paragraphs[2]); + }); + + it('preserves newlines and indentation for a selection contained in a code block', () => { + const container = document.createElement('div'); + container.className = 'plan-markdown plan-markdown-dialog'; + const pre = document.createElement('pre'); + pre.className = 'shiki-block'; + const code = document.createElement('code'); + for (const line of ['function a() {', ' return 1;', '}']) { + const span = document.createElement('span'); + span.className = 'line'; + span.textContent = line; + code.append(span); + } + pre.append(code); + container.append(pre); + document.body.append(container); + selectText(firstTextNodeIn(code), lastTextNodeIn(code)); + + const originalInnerText = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'innerText'); + Object.defineProperty(HTMLElement.prototype, 'innerText', { + configurable: true, + get() { + return this.querySelector('pre.shiki-block code') + ? 'function a() {\n return 1;\n}' + : (this.textContent ?? ''); + }, + }); + try { + expect(getPlanSelection(container, 'plan.md')?.selectedText).toBe( + 'function a() {\n return 1;\n}', + ); + } finally { + if (originalInnerText) { + Object.defineProperty(HTMLElement.prototype, 'innerText', originalInnerText); + } else { + delete (HTMLElement.prototype as { innerText?: unknown }).innerText; + } + } + }); + + it('anchors a selection ending at the next block boundary to the prior block', () => { + const container = document.createElement('div'); + container.innerHTML = '

First paragraph

Second paragraph

'; + document.body.append(container); + const paragraphs = container.querySelectorAll('p'); + const range = document.createRange(); + range.setStart(paragraphs[0].firstChild as Text, 0); + range.setEnd(paragraphs[1], 0); + const selection = window.getSelection(); + selection?.removeAllRanges(); + selection?.addRange(range); + + expect(getPlanSelectionFlowAnchor(container)).toBe(paragraphs[0]); + }); + + it('preserves rendered block breaks between selected code and prose', () => { + const container = renderPlanMarkdown('```ts\nconst x = 1;\n```\n\nAfter step'); + const codeText = firstTextNodeIn(container.querySelector('code') as HTMLElement); + const proseText = container.querySelector('p')?.firstChild as Text; + selectText(codeText, proseText); + + expect(getPlanSelection(container, 'plan.md')?.selectedText).toBe('const x = 1;\nAfter step'); + }); + + it('intentionally excludes non-rendered Mermaid SVG text from prompt text', () => { + const container = document.createElement('div'); + container.innerHTML = '

After diagram

'; + const mermaid = container.querySelector('.mermaid-block') as HTMLDivElement; + const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg'); + const defs = document.createElementNS('http://www.w3.org/2000/svg', 'defs'); + const hidden = document.createElementNS('http://www.w3.org/2000/svg', 'text'); + hidden.textContent = 'Hidden marker'; + const visible = document.createElementNS('http://www.w3.org/2000/svg', 'text'); + visible.textContent = 'Visible diagram label'; + defs.append(hidden); + svg.append(defs, visible); + mermaid.append(svg); + document.body.append(container); + + selectText( + findTextNode(container, 'Hidden marker'), + container.querySelector('p')?.firstChild as Text, + ); + + const selectedText = getPlanSelection(container, 'plan.md')?.selectedText ?? ''; + expect(selectedText).not.toContain('Hidden marker'); + expect(selectedText).toContain('After diagram'); + }); + + it('recalculates retained range geometry when the plan reflows', () => { + const container = document.createElement('div'); + container.innerHTML = '

First block

Second block

'; + document.body.append(container); + const paragraphs = container.querySelectorAll('p'); + selectText(paragraphs[0].firstChild as Text, paragraphs[1].firstChild as Text); + const ranges = getPlanSelectionTextRanges(container); + let rangeTops = [30, 110]; + ranges.forEach((range, index) => { + Object.defineProperty(range, 'getClientRects', { + value: () => [rect(rangeTops[index], 15)] as unknown as DOMRectList, + }); + }); + Object.defineProperty(container, 'getBoundingClientRect', { value: () => rect(10, 5) }); + + class FakeResizeObserver { + static callback: ResizeObserverCallback; + static disconnected = false; + constructor(callback: ResizeObserverCallback) { + FakeResizeObserver.callback = callback; + } + observe() {} + disconnect() { + FakeResizeObserver.disconnected = true; + } + static trigger() { + FakeResizeObserver.callback([], {} as ResizeObserver); + } + } + vi.stubGlobal('ResizeObserver', FakeResizeObserver); + + const updates: number[][] = []; + const stop = trackPlanSelectionGeometry(container, ranges, (next) => + updates.push(next.map((item) => item.top)), + ); + expect(updates.at(-1)).toEqual([20, 100]); + rangeTops = [30, 50]; + FakeResizeObserver.trigger(); + expect(updates.at(-1)).toEqual([20, 40]); + stop(); + expect(FakeResizeObserver.disconnected).toBe(true); + }); +}); diff --git a/src/lib/plan-selection.ts b/src/lib/plan-selection.ts index 99e84ba7..52ec2ac6 100644 --- a/src/lib/plan-selection.ts +++ b/src/lib/plan-selection.ts @@ -12,23 +12,159 @@ export interface PlanSelection { const BLOCK_SELECTOR = 'p, li, h1, h2, h3, h4, h5, h6, pre, tr'; const HEADING_SELECTOR = 'h1, h2, h3, h4, h5, h6'; +export const PLAN_REVIEW_FLOW_SLOT_SELECTOR = '[data-plan-review-flow-slot]'; + +export interface PlanSelectionRect { + top: number; + left: number; + width: number; + height: number; +} + +function getSelectionRange(containerEl: HTMLElement): Range | null { + const selection = window.getSelection(); + if (!selection || selection.isCollapsed || selection.rangeCount === 0) return null; + + const range = selection.getRangeAt(0); + return containerEl.contains(range.commonAncestorContainer) ? range : null; +} + +function isInPlanReviewFlowSlot(node: Node): boolean { + const element = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement; + return Boolean(element?.closest(PLAN_REVIEW_FLOW_SLOT_SELECTOR)); +} + +function cloneSelectionWithAncestors( + containerEl: HTMLElement, + selectedRange: Range, +): DocumentFragment { + let fragment = selectedRange.cloneContents(); + let ancestor: Node | null = + selectedRange.commonAncestorContainer.nodeType === Node.TEXT_NODE + ? selectedRange.commonAncestorContainer.parentNode + : selectedRange.commonAncestorContainer; + + while (ancestor && ancestor !== containerEl) { + if (ancestor instanceof Element) { + const wrapper = ancestor.cloneNode(false) as Element; + wrapper.append(fragment); + fragment = document.createDocumentFragment(); + fragment.append(wrapper); + } + ancestor = ancestor.parentNode; + } + + return fragment; +} + +function getPlanSelectionVisibleText(containerEl: HTMLElement, selectedRange: Range): string { + const host = document.createElement('div'); + const fragment = cloneSelectionWithAncestors(containerEl, selectedRange); + fragment.querySelectorAll(PLAN_REVIEW_FLOW_SLOT_SELECTOR).forEach((node) => node.remove()); + fragment + .querySelectorAll('style, script, defs, metadata, title, desc, [hidden], [aria-hidden="true"]') + .forEach((node) => node.remove()); + + host.className = containerEl.className; + host.style.cssText = [ + 'position:absolute', + 'left:-99999px', + 'top:0', + 'contain:layout style paint', + `width:${Math.max(containerEl.clientWidth, 1)}px`, + ].join(';'); + host.append(fragment); + + const parent = containerEl.parentElement ?? document.body; + parent.append(host); + try { + return host.innerText.trim(); + } finally { + host.remove(); + } +} + +/** Return the selected text ranges that belong to plan content, excluding inline review UI. */ +export function getPlanSelectionTextRanges(containerEl: HTMLElement): Range[] { + const selectedRange = getSelectionRange(containerEl); + if (!selectedRange) return []; + + const ranges: Range[] = []; + const walkerRoot = + selectedRange.commonAncestorContainer.nodeType === Node.TEXT_NODE + ? selectedRange.commonAncestorContainer.parentElement + : selectedRange.commonAncestorContainer; + const walker = document.createTreeWalker( + walkerRoot && containerEl.contains(walkerRoot) ? walkerRoot : containerEl, + NodeFilter.SHOW_TEXT, + ); + let node = walker.nextNode(); + while (node) { + if (!isInPlanReviewFlowSlot(node) && selectedRange.intersectsNode(node)) { + const text = node as Text; + const start = node === selectedRange.startContainer ? selectedRange.startOffset : 0; + const end = node === selectedRange.endContainer ? selectedRange.endOffset : text.length; + if (start < end) { + const range = document.createRange(); + range.setStart(text, start); + range.setEnd(text, end); + ranges.push(range); + } + } + node = walker.nextNode(); + } + return ranges; +} + +export function getPlanSelectionRects( + containerEl: HTMLElement, + ranges: readonly Range[], +): PlanSelectionRect[] { + const containerRect = containerEl.getBoundingClientRect(); + const rects: PlanSelectionRect[] = []; + for (const range of ranges) { + for (const rect of range.getClientRects()) { + rects.push({ + top: rect.top - containerRect.top, + left: rect.left - containerRect.left, + width: rect.width, + height: rect.height, + }); + } + } + return rects; +} + +/** Keep persisted selection overlays aligned while inline cards reflow the plan. */ +export function trackPlanSelectionGeometry( + containerEl: HTMLElement, + ranges: readonly Range[], + onChange: (rects: PlanSelectionRect[]) => void, +): () => void { + const refresh = () => onChange(getPlanSelectionRects(containerEl, ranges)); + refresh(); + + if (typeof ResizeObserver === 'undefined') return () => undefined; + const observer = new ResizeObserver(refresh); + observer.observe(containerEl); + return () => observer.disconnect(); +} /** * Extract structured selection info from the current DOM selection * within a plan viewer container. Returns null if no valid selection. */ export function getPlanSelection(containerEl: HTMLElement, source: string): PlanSelection | null { - const selection = window.getSelection(); - if (!selection || selection.isCollapsed) return null; - - const range = selection.getRangeAt(0); - if (!containerEl.contains(range.commonAncestorContainer)) return null; + const range = getSelectionRange(containerEl); + if (!range) return null; - const selectedText = selection.toString().trim(); + const selectedText = getPlanSelectionVisibleText(containerEl, range); if (!selectedText) return null; const nearestHeading = findNearestHeading(containerEl, range.startContainer); - const blocks = containerEl.querySelectorAll(BLOCK_SELECTOR); + const blocks = Array.from(containerEl.querySelectorAll(BLOCK_SELECTOR)).filter( + (block) => !block.closest(PLAN_REVIEW_FLOW_SLOT_SELECTOR), + ); const blockIndex = countBlocksBefore(blocks, range.startContainer); const endBlockIndex = countBlocksBefore(blocks, range.endContainer); @@ -41,6 +177,35 @@ export function getPlanSelection(containerEl: HTMLElement, source: string): Plan }; } +/** Find the block that should own an inline review card for the current selection. */ +export function getPlanSelectionFlowAnchor(containerEl: HTMLElement): HTMLElement | null { + const ranges = getPlanSelectionTextRanges(containerEl); + const range = ranges.at(-1); + if (!range) return null; + + let element: Element | null = + range.endContainer.nodeType === Node.ELEMENT_NODE + ? (range.endContainer as Element) + : range.endContainer.parentElement; + if (!element || element.closest(PLAN_REVIEW_FLOW_SLOT_SELECTOR)) return null; + + const block = element.closest(BLOCK_SELECTOR); + if (block && block !== containerEl && containerEl.contains(block)) { + // A div cannot be a child of a table row, so place table comments after the table. + if (block.tagName === 'TR') { + const table = block.closest('table'); + if (table instanceof HTMLElement && containerEl.contains(table)) return table; + } + if (block instanceof HTMLElement) return block; + } + + // Fallback for rendered blocks such as Mermaid diagrams that are not in BLOCK_SELECTOR. + while (element.parentElement && element.parentElement !== containerEl) { + element = element.parentElement; + } + return element instanceof HTMLElement && element.parentElement === containerEl ? element : null; +} + /** Walk backwards from the selection start to find the nearest heading. */ function findNearestHeading(container: HTMLElement, startNode: Node): string { let node: Node | null = startNode; @@ -88,7 +253,7 @@ function findNearestHeading(container: HTMLElement, startNode: Node): string { } /** Count block elements before the given node from a pre-queried list. */ -function countBlocksBefore(blocks: NodeListOf, node: Node): number { +function countBlocksBefore(blocks: readonly Element[], node: Node): number { let count = 0; for (const block of blocks) { // Is this block before or containing the node? diff --git a/src/styles.css b/src/styles.css index 9e011aca..7307f347 100644 --- a/src/styles.css +++ b/src/styles.css @@ -2099,6 +2099,14 @@ body.dragging-task * { margin: 0.6em 0; } +/* Inline plan-review controls occupy real layout space after the selected block. */ +.plan-review-flow-slot { + display: flow-root; + width: 100%; + min-width: 0; + font-style: normal; +} + /* Lists — better spacing */ .plan-markdown-dialog ul, .plan-markdown-dialog ol {