diff --git a/comments-view/src/CommentThread.tsx b/comments-view/src/CommentThread.tsx index 5fbfbda4..087aaecb 100644 --- a/comments-view/src/CommentThread.tsx +++ b/comments-view/src/CommentThread.tsx @@ -392,12 +392,15 @@ function CuteDraftEditor(props: { // Reactively track a single attribute on `element`, seeded from its current // value and updated via a `MutationObserver`. Used to receive the host's -// per-thread selection state as a `data-*` attribute. +// per-thread selection state as a `data-*` attribute. The host sets it on the +// ``, but the tool is handed the content element that view +// renders into, so it's read from the view. function createElementAttribute( - element: HTMLElement, + toolElement: HTMLElement, name: string, fallback: string ): () => string { + const element = toolElement.closest("patchwork-view") ?? toolElement; const read = () => element.getAttribute(name) ?? fallback; const [value, setValue] = createSignal(read()); onMount(() => { diff --git a/comments-view/src/CommentsView.tsx b/comments-view/src/CommentsView.tsx index 82b75ff0..d13eafde 100644 --- a/comments-view/src/CommentsView.tsx +++ b/comments-view/src/CommentsView.tsx @@ -93,22 +93,15 @@ export function CommentsView(props: { element: HTMLElement }) { const threadTargetHandleMap = useResolvedHandleMap(threadTargetUrlMap, repo); - // A thread is only worth showing if at least one of its targets still - // resolves to a real, non-empty span. Targets that resolve to an empty - // string (a collapsed range) or an undefined value (an anchor that no - // longer resolves) are excluded — see `targetIsVisible`. + // A thread is worth showing unless every one of its targets is a text range + // that has collapsed to an empty string — see `targetIsVisible`. const threadsWithVisibleTarget = useThreadsWithVisibleTarget( threadTargetUrlMap, repo ); - const renderableThreadUrls = createMemo(() => - threadUrls().filter((url) => threadsWithVisibleTarget().has(url)) - ); - // Document-level threads target the whole document (a bare url, no `/` path - // or `#` heads), so they have no resolvable range and never pass the - // visible-target filter. They should always be listed regardless. + // or `#` heads). const docLevelThreadUrls = createMemo>(() => { const map = threadTargetUrlMap(); const set = new Set(); @@ -123,15 +116,19 @@ export function CommentsView(props: { element: HTMLElement }) { return set; }); - // What the panel actually renders: ranged threads whose target is still - // visible, plus all document-level threads. - const displayedThreadUrls = createMemo(() => + // Threads that can overlap the editor's selection. Document-level threads + // are left out: they cover the whole doc, so they'd match every selection. + const renderableThreadUrls = createMemo(() => threadUrls().filter( (url) => - threadsWithVisibleTarget().has(url) || docLevelThreadUrls().has(url) + threadsWithVisibleTarget().has(url) && !docLevelThreadUrls().has(url) ) ); + const displayedThreadUrls = createMemo(() => + threadUrls().filter((url) => threadsWithVisibleTarget().has(url)) + ); + const overlappingThreads = createMemo(() => renderableThreadUrls().filter((url) => threadOverlapsSelection( @@ -165,10 +162,16 @@ export function CommentsView(props: { element: HTMLElement }) { return new Set(overlappingThreads().filter((u) => u !== p)); }); + // The thread card under the pointer, which highlights its targets in place + // of the primary thread's while hovered. + const [hoveredThread, setHoveredThread] = createSignal< + AutomergeUrl | undefined + >(); + createEffect(() => { const handle = focusHandle(); if (!handle) return; - const p = primaryThreadUrl(); + const p = hoveredThread() ?? primaryThreadUrl(); const urls = p ? (threadTargetUrlMap().get(p) ?? []) : []; const desired: Record = {}; for (const u of urls) desired[u] = true; @@ -255,6 +258,10 @@ export function CommentsView(props: { element: HTMLElement }) {
onClickThreadCard(e, threadUrl)} + onMouseEnter={() => setHoveredThread(threadUrl)} + onMouseLeave={() => + setHoveredThread((u) => (u === threadUrl ? undefined : u)) + } >