From 70c6bb26ebab862503968f5791138c2f7e3bf2f2 Mon Sep 17 00:00:00 2001 From: Paul Sonnentag Date: Tue, 13 Oct 2026 14:37:17 +0200 Subject: [PATCH 1/4] comments-view: list every comment except ranges that collapsed to empty Threads targeting a sub-doc path (e.g. a notebook cell) have no text range, so they were filtered out of the sidebar. Only hide a thread when all its targets are text ranges whose text was deleted. --- comments-view/src/CommentsView.tsx | 35 +++++++++++++----------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/comments-view/src/CommentsView.tsx b/comments-view/src/CommentsView.tsx index 82b75ff0..dae383bf 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( @@ -403,14 +400,12 @@ function flatHandles( return out; } -// A target "points to" a real value only when its anchored range still -// resolves (`rangePositions` is defined) and spans at least one character -// (`start !== end`). A missing range is an undefined value (the anchor no -// longer resolves); an empty range is an empty string (the commented text was -// deleted). Mirrors `buildCommentDecorations` in the codemirror tool. +// A target is hidden only when it is a text range that has collapsed to an +// empty string (the commented text was deleted). Every other target — a whole +// document, a sub-doc path such as a notebook cell — stays listed. function targetIsVisible(handle: DocHandle): boolean { const positions = handle.rangePositions(); - if (!positions) return false; + if (!positions) return true; const [start, end] = positions; return start !== end; } From aa0eee318ec1aa620ee7676e3b9e240ae02e2f4a Mon Sep 17 00:00:00 2001 From: Paul Sonnentag Date: Thu, 15 Oct 2026 10:35:44 +0200 Subject: [PATCH 2/4] comments-view: highlight a thread's targets on hover; match sub-doc selections Hovering a thread card puts its targets in the focus doc's `highlight`, so the view showing them can emphasise them. A selection on a sub-doc (e.g. a notebook cell) now selects threads targeting it, not only overlapping text ranges. --- comments-view/src/CommentsView.tsx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/comments-view/src/CommentsView.tsx b/comments-view/src/CommentsView.tsx index dae383bf..d13eafde 100644 --- a/comments-view/src/CommentsView.tsx +++ b/comments-view/src/CommentsView.tsx @@ -162,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; @@ -252,6 +258,10 @@ export function CommentsView(props: { element: HTMLElement }) {
onClickThreadCard(e, threadUrl)} + onMouseEnter={() => setHoveredThread(threadUrl)} + onMouseLeave={() => + setHoveredThread((u) => (u === threadUrl ? undefined : u)) + } > ): boolean { return start !== end; } +// A selected text range overlaps a ranged target; a selected sub-doc (such as +// a notebook cell) matches a target at or inside it. function threadOverlapsSelection( targets: DocHandle[], selection: DocHandle[] ): boolean { - return targets.some((t) => selection.some((s) => s.overlaps(t))); + return targets.some((t) => + selection.some((s) => s.overlaps(t) || s.url === t.url || s.contains(t)) + ); } From 4ea6cc7d082cb552a94af8dbdf9c7fc85e3d2daf Mon Sep 17 00:00:00 2001 From: Paul Sonnentag Date: Thu, 15 Oct 2026 11:13:02 +0200 Subject: [PATCH 3/4] comments-view: make the selected thread stand out in the sidebar Primary, secondary and inactive cards differed only by shadow. In the panel, the selected thread now gets an accent border, ring and tint, overlapping threads a softer accent border, and inactive ones an accent hint on hover. Threads embedded on their own keep the old look. --- comments-view/src/styles.css | 41 +++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/comments-view/src/styles.css b/comments-view/src/styles.css index 2c7fa312..275ae28d 100644 --- a/comments-view/src/styles.css +++ b/comments-view/src/styles.css @@ -88,7 +88,11 @@ flex-direction: column; gap: 0.5rem; cursor: pointer; - transition: box-shadow 0.15s ease; + border-radius: 6px; + transition: + border-color 0.15s ease, + background 0.15s ease, + box-shadow 0.15s ease; } .comments-thread-card--primary { @@ -107,6 +111,41 @@ box-shadow: 0 1px 2px 0 rgb(0 0 0 / 0.05); } +/* In the sidebar, where threads sit side by side, the selected one needs to + stand out from the rest: an accent border and ring, faintly tinted. A thread + embedded on its own defaults to primary, so this is scoped to the panel. */ +.comments-panel .comments-thread-card--primary { + border-color: var(--comments-accent); + background: color-mix(in srgb, var(--comments-accent) 6%, var(--comments-bg)); + box-shadow: + 0 0 0 1px var(--comments-accent), + 0 6px 14px -4px rgb(0 0 0 / 0.15); +} + +/* Threads that also overlap the selection: a softer accent border. */ +.comments-panel .comments-thread-card--secondary { + border-color: color-mix( + in srgb, + var(--comments-accent) 50%, + var(--comments-border) + ); + box-shadow: 0 2px 4px -2px rgb(0 0 0 / 0.1); +} + +.comments-panel .comments-thread-card--inactive { + box-shadow: none; +} + +/* Hovering a thread highlights its target, so hint that it's live. */ +.comments-panel .comments-thread:hover .comments-thread-card--inactive { + border-color: color-mix( + in srgb, + var(--comments-accent) 40%, + var(--comments-border) + ); + background: var(--comments-hover-bg); +} + /* Resolved threads are hidden in the sidebar but shown when embedded. Keep them clearly readable, just visibly "settled": muted fill, dashed border, and slightly dimmed so they read as archived rather than active. */ From 843f3cfdbf9b40ecbcfff05fff418a340505e9ba Mon Sep 17 00:00:00 2001 From: Paul Sonnentag Date: Thu, 15 Oct 2026 11:16:29 +0200 Subject: [PATCH 4/4] comments-view: read a thread's selection state from its patchwork-view The sidebar sets data-thread-state on each , but the thread tool is handed the view's inner content element, so it never saw the attribute and every thread fell back to primary (all highlighted, all showing Resolve/Reply). --- comments-view/src/CommentThread.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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(() => {