From 10ea254169d058fafe91fb47aa6f30b1844e7658 Mon Sep 17 00:00:00 2001 From: AnnaXWang <6621137+AnnaXWang@users.noreply.github.com> Date: Wed, 1 Jul 2026 23:23:30 +0000 Subject: [PATCH 1/5] Fix highlight/chrome coupling and block-anchor rendering from review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decouple the in-document highlight from the chrome toggle: the .jh-dark treatment now follows the document's sampled darkness, never the viewer's light/dark chrome choice. A highlight is painted ON the doc, so it must contrast with the page's real background — forcing light chrome on a dark doc previously flipped the highlight toward the light treatment and it stopped reading. This removes jh:setThemeMode entirely, so the "second jh:ready doesn't re-send the mode" gap no longer exists. Fix block-boundary anchors: an anchor whose first character starts a block (a heading, a paragraph) resolved its range to the block's leading edge, so wrapping pulled the whole element into an inline span whose background never paints — the highlight silently vanished. locateStart biases a boundary start into the next text node so we wrap the text, not the block. Co-Authored-By: Claude Opus 4.7 --- app/d/[slug]/CommentsShell.tsx | 6 ------ lib/docs/overlay.ts | 39 ++++++++++++++++++++++------------ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/app/d/[slug]/CommentsShell.tsx b/app/d/[slug]/CommentsShell.tsx index f19bd32..178c91a 100644 --- a/app/d/[slug]/CommentsShell.tsx +++ b/app/d/[slug]/CommentsShell.tsx @@ -463,12 +463,6 @@ export default function CommentsShell(props: Props) { if (overlayReady) postToOverlay({ type: "jh:active", id: activeId }); }, [activeId, overlayReady, postToOverlay]); - // Tell the overlay which highlight treatment to paint. A forced light/dark - // overrides the overlay's own doc-darkness sampling; "auto" hands control back. - useEffect(() => { - if (overlayReady) postToOverlay({ type: "jh:setThemeMode", mode }); - }, [mode, overlayReady, postToOverlay]); - const visibleThreads = useMemo( () => threads.filter((t) => showResolved || !t.resolved), [threads, showResolved] diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts index a06946a..01404e8 100644 --- a/lib/docs/overlay.ts +++ b/lib/docs/overlay.ts @@ -12,8 +12,6 @@ // { type:"jh:focus", key } (focus a key from the rail; null clears) // { type:"jh:scrollTo", id } // { type:"jh:clearSelection" } -// { type:"jh:setThemeMode", mode } ("light"|"dark" force the highlight -// treatment; "auto"/null = sample the doc) // overlay → shell: { type:"jh:ready" } // { type:"jh:positions", positions:{ [id]: yTopPx } } (comment highlight y) // { type:"jh:selection", anchor:{exact,prefix,suffix}, rect:{...} } @@ -65,7 +63,6 @@ export const OVERLAY_SCRIPT = String.raw` var focusKey = null; // focused (pinned) key var lastClickKeys = null; // covering set of the last focus click (for cycle) var lastClickPos = -1; // doc-text offset of the last focus click (cycle reset on move) - var forcedMode = null; // "light"|"dark" from the shell's theme toggle; null = auto (sample the doc) function send(msg){ try { parent.postMessage(msg, "*"); } catch(e){} } function esc(s){ return (s||"").replace(/&/g,"&").replace(//g,">"); } @@ -120,13 +117,11 @@ export const OVERLAY_SCRIPT = String.raw` else dark = lum < 0.4; lastDark = dark; - // The highlight treatment follows the shell's explicit light/dark toggle - // when one is set (forcedMode), otherwise the sampled darkness. jh:theme - // still reports the SAMPLED value below so the shell's auto path stays honest. - var effDark = forcedMode === "dark" ? true : forcedMode === "light" ? false : dark; - - // toggle the dark-highlight stylesheet branch (needs the style present) - try { ensureStyle(); if (document.documentElement) document.documentElement.classList.toggle("jh-dark", !!effDark); } catch(e){} + // toggle the dark-highlight stylesheet branch (needs the style present). + // Keyed on the doc's SAMPLED darkness — never the chrome theme — because the + // highlight is painted ON the document, so it must contrast with the page's + // real background regardless of what the viewer picked for the rail chrome. + try { ensureStyle(); if (document.documentElement) document.documentElement.classList.toggle("jh-dark", !!dark); } catch(e){} send({ type:"jh:theme", bg: "rgb("+Math.round(bgRgb[0])+","+Math.round(bgRgb[1])+","+Math.round(bgRgb[2])+")", @@ -162,6 +157,24 @@ export const OVERLAY_SCRIPT = String.raw` } return null; } + // Like locate but forward-biased for a range START: an offset sitting exactly on + // a text-node boundary resolves to the NEXT node's start, not the previous node's + // end. Otherwise a range whose first character is the start of a block (a heading, + // a paragraph) begins at the block's leading edge, and wrapping it pulls the whole + // block into an inline — whose background never paints, so the highlight + // silently vanishes. Bias the start inward so we wrap the text, not the block. + function locateStart(nodes, offset){ + for (var i=0;i= e.start && offset < e.start + len) return { node: e.node, offset: offset - e.start }; + if (offset === e.start + len){ + var nx = nodes[i+1]; + if (nx && nx.start === offset) return { node: nx.node, offset: 0 }; + return { node: e.node, offset: len }; + } + } + return null; + } function squash(s){ return (s||"").replace(/\s+/g," "); } // Resolve an anchor against the snapshot text → {start, len} (offsets into full), @@ -199,7 +212,7 @@ export const OVERLAY_SCRIPT = String.raw` } function mkRange(nodes, start, len){ - var a = locate(nodes, start), b = locate(nodes, start+len); + var a = locateStart(nodes, start), b = locate(nodes, start+len); if (!a || !b) return null; try { var r = document.createRange(); r.setStart(a.node, a.offset); r.setEnd(b.node, b.offset); return r; } catch(e){ return null; } } @@ -235,8 +248,7 @@ export const OVERLAY_SCRIPT = String.raw` // invisible on a dark page, so when the doc is dark we repaint highlights as a // stronger warm amber wash (~.30–.54 alpha by depth) with a near-opaque warm // underline — legible on dark while keeping the doc's own (light) text - // readable. Gated by a .jh-dark class on set from sampleTheme (which - // honors the shell's light/dark toggle via forcedMode). + // readable. Gated by a .jh-dark class on set from sampleTheme. + "html.jh-dark span[data-jh-seg].d1{background:rgba(245,197,24,.30);border-bottom-color:rgba(245,197,24,.95)}" + "html.jh-dark span[data-jh-seg].d2{background:rgba(245,197,24,.42);border-bottom-color:rgba(245,197,24,.98)}" + "html.jh-dark span[data-jh-seg].d3{background:rgba(245,197,24,.54);border-bottom-color:#f5c518}" @@ -647,7 +659,6 @@ export const OVERLAY_SCRIPT = String.raw` } else if (d.type === "jh:scrollTo"){ var sk = (typeof d.id === "number") ? "c:"+d.id : String(d.id); scrollToKey(sk); } else if (d.type === "jh:clearSelection"){ var s=window.getSelection(); if(s) s.removeAllRanges(); } - else if (d.type === "jh:setThemeMode"){ forcedMode = (d.mode === "dark" || d.mode === "light") ? d.mode : null; sampleTheme(); } else if (d.type === "jh:ping"){ send({type:"jh:ready"}); } }); From 626aba5e32da21aa6035b8988b2f4f56f8a5ad0f Mon Sep 17 00:00:00 2001 From: AnnaXWang <6621137+AnnaXWang@users.noreply.github.com> Date: Thu, 2 Jul 2026 00:11:18 +0000 Subject: [PATCH 2/5] Dark comment marker as underline; open rail when starting a comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On dark documents, mark commented spans with a warm amber underline instead of a filled background wash — the fill read as muddy over dark backgrounds and fought the doc's own text. Hover and focus keep a faint transient wash for feedback only. Clicking "comment" in the selection toolbar now opens the rail, so the draft composer is visible even when the rail started closed (a doc with no comments on desktop, or any doc on mobile). Previously the draft landed in a hidden rail and the click looked like a no-op. Co-Authored-By: Claude Opus 4.7 --- app/d/[slug]/CommentsShell.tsx | 4 ++++ lib/docs/overlay.ts | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/d/[slug]/CommentsShell.tsx b/app/d/[slug]/CommentsShell.tsx index 178c91a..139ea41 100644 --- a/app/d/[slug]/CommentsShell.tsx +++ b/app/d/[slug]/CommentsShell.tsx @@ -555,6 +555,10 @@ export default function CommentsShell(props: Props) { canComment={canComment} canReact={canReact} onComment={() => { + // Open the rail so the draft composer is actually visible — on a + // doc with no comments (desktop) or on mobile the rail starts + // closed, and a draft dropped into a hidden rail looks like a no-op. + setRailOpen(true); setDraft({ anchor: selection.anchor, top: selection.top }); setSelection(null); postToOverlay({ type: "jh:clearSelection" }); diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts index 01404e8..db9b4d2 100644 --- a/lib/docs/overlay.ts +++ b/lib/docs/overlay.ts @@ -244,16 +244,16 @@ export const OVERLAY_SCRIPT = String.raw` + "span[data-jh-seg].jh-hover{background:#ffd76b}" + "span[data-jh-seg].jh-focus{background:#ffce3a;box-shadow:inset 0 0 0 9999px rgba(255,179,0,.18)}" + "span[data-jh-seg].jh-dim{opacity:.4}" - // DARK DOC (adaptive chrome, variant D): the light #fff3bf wash is nearly - // invisible on a dark page, so when the doc is dark we repaint highlights as a - // stronger warm amber wash (~.30–.54 alpha by depth) with a near-opaque warm - // underline — legible on dark while keeping the doc's own (light) text - // readable. Gated by a .jh-dark class on set from sampleTheme. - + "html.jh-dark span[data-jh-seg].d1{background:rgba(245,197,24,.30);border-bottom-color:rgba(245,197,24,.95)}" - + "html.jh-dark span[data-jh-seg].d2{background:rgba(245,197,24,.42);border-bottom-color:rgba(245,197,24,.98)}" - + "html.jh-dark span[data-jh-seg].d3{background:rgba(245,197,24,.54);border-bottom-color:#f5c518}" - + "html.jh-dark span[data-jh-seg].jh-hover{background:rgba(245,197,24,.5)}" - + "html.jh-dark span[data-jh-seg].jh-focus{background:rgba(245,197,24,.44);box-shadow:inset 0 0 0 9999px rgba(245,197,24,.12),0 0 0 1px rgba(245,197,24,.95)}" + // DARK DOC (adaptive chrome, variant D): a filled wash reads as muddy on a + // dark page, so instead of a background we mark the span with a warm amber + // UNDERLINE (depth = opacity) and leave the doc's own text untouched. Hover + // and focus add a faint transient wash for feedback only. Gated by a .jh-dark + // class on set from sampleTheme. + + "html.jh-dark span[data-jh-seg].d1{background:transparent;border-bottom:2px solid rgba(245,197,24,.8)}" + + "html.jh-dark span[data-jh-seg].d2{background:transparent;border-bottom:2px solid rgba(245,197,24,.92)}" + + "html.jh-dark span[data-jh-seg].d3{background:transparent;border-bottom:2px solid #f5c518}" + + "html.jh-dark span[data-jh-seg].jh-hover{background:rgba(245,197,24,.14)}" + + "html.jh-dark span[data-jh-seg].jh-focus{background:rgba(245,197,24,.2);box-shadow:0 0 0 1px rgba(245,197,24,.85)}" + "span[data-jh-chip]{display:inline-flex;align-items:center;gap:2px;font-size:11.5px;line-height:1;" + "background:#fbfbfb;border:1px solid #e0e0e0;border-radius:10px;padding:1px 6px 1px 5px;margin-left:4px;" + "vertical-align:.12em;font-family:ui-monospace,Menlo,Consolas,monospace;cursor:pointer;user-select:none;" From 1bb55b01080cc6a54dd2f223bee47c7ee8fca18c Mon Sep 17 00:00:00 2001 From: AnnaXWang <6621137+AnnaXWang@users.noreply.github.com> Date: Thu, 2 Jul 2026 00:26:31 +0000 Subject: [PATCH 3/5] Sync comment rail scroll to the document (fix stranded cards) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cards are laid out at their highlight's absolute document Y, but the rail never followed the document's scroll — so a comment anchored deep in a long doc left its card stranded far below an otherwise-empty rail ("N comments" header with nothing under it). The overlay now reports the document's scrollY, and on desktop the rail scrolls in lockstep so each card sits next to its highlight; the rail's content height is pinned to the document height so the sync has room to move. On mobile (overlay drawer, no doc alongside) cards stack normally instead of using absolute Y, so they're never stranded there either. Co-Authored-By: Claude Opus 4.7 --- app/d/[slug]/CommentsShell.tsx | 44 ++++++++++++++++++++++++++++++---- lib/docs/overlay.ts | 5 ++-- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/app/d/[slug]/CommentsShell.tsx b/app/d/[slug]/CommentsShell.tsx index 139ea41..d0fc72b 100644 --- a/app/d/[slug]/CommentsShell.tsx +++ b/app/d/[slug]/CommentsShell.tsx @@ -127,6 +127,11 @@ export default function CommentsShell(props: Props) { const [pinnedId, setPinnedId] = useState(null); const [activeId, setActiveId] = useState(null); const [positions, setPositions] = useState>({}); + // The doc's live scroll offset + total height (from the overlay). On desktop the + // rail scrolls to match docScrollY so each card tracks its highlight instead of + // being stranded at an absolute Y in an otherwise-empty rail. + const [docScrollY, setDocScrollY] = useState(0); + const [docHeight, setDocHeight] = useState(0); const [overlayReady, setOverlayReady] = useState(false); // Adaptive chrome (variant D). The server may hand us a coarse dark theme for @@ -266,6 +271,8 @@ export default function CommentsShell(props: Props) { break; case "jh:positions": setPositions(d.positions || {}); + if (typeof d.scrollY === "number") setDocScrollY(d.scrollY); + if (typeof d.docHeight === "number") setDocHeight(d.docHeight); break; case "jh:theme": // Adaptive chrome: the overlay sampled the doc's effective colors. Drive @@ -488,12 +495,14 @@ export default function CommentsShell(props: Props) { const hadCommentsAtLoad = props.initialThreads.length > 0; const [railOpen, setRailOpen] = useState(false); const isMobileRef = useRef(false); + const [isMobile, setIsMobile] = useState(false); useEffect(() => { const mq = window.matchMedia("(max-width: 768px)"); // Desktop: open by default ONLY if the doc already has comments. Mobile: // always closed by default (the toggle opens the right drawer). const applyDefault = () => { isMobileRef.current = mq.matches; + setIsMobile(mq.matches); setRailOpen(!mq.matches && hadCommentsAtLoad); }; applyDefault(); @@ -503,6 +512,16 @@ export default function CommentsShell(props: Props) { // The count shown in the toggle: number of threads (visible roots). const commentCount = threads.length; + // Docs-style scroll sync (desktop only): cards are laid out at their highlight's + // absolute document Y, so the rail must scroll in lockstep with the document or a + // deep comment's card is stranded far below an empty rail. On mobile the rail is + // an overlay drawer with no doc alongside it, so cards stack normally instead. + useEffect(() => { + if (isMobile) return; + const el = railRef.current; + if (el) el.scrollTop = docScrollY; + }, [docScrollY, isMobile]); + // When dark, expose the variant-D palette as CSS custom properties on the // wrapper. Every themed color below reads `var(--jh-x, )`, so // when the vars are UNSET (light docs / no theme) the bytes resolve to today's @@ -630,6 +649,8 @@ export default function CommentsShell(props: Props) { ; + aligned: boolean; + docHeight: number; pinnedId: number | null; activeId: number | null; canComment: boolean; @@ -820,6 +843,8 @@ function RailCards(props: { const { threads, positions, + aligned, + docHeight, pinnedId, activeId, canComment, @@ -842,8 +867,19 @@ function RailCards(props: { const [, force] = useState(0); useEffect(() => { - // Re-clamp after layout: walk cards in DOM order, push each down so its top - // is >= previous card's bottom + gap, and >= its anchor y. + // Mobile (drawer, no doc alongside): stack cards normally — clear any Y offset + // from a previous desktop layout so nothing is stranded below an empty drawer. + if (!aligned) { + for (const t of threads) { + const el = cardRefs.current.get(t.id); + if (el && el.style.marginTop) el.style.marginTop = ""; + } + return; + } + // Desktop: re-clamp after layout — walk cards in DOM order, push each down so + // its top is >= previous card's bottom + gap, and >= its anchor y. The rail + // scrolls in lockstep with the document (see CommentsShell), so a card sits + // next to its highlight. const anchored = threads.filter((t) => t.group === "anchored"); let lastBottom = 0; let changed = false; @@ -861,10 +897,10 @@ function RailCards(props: { lastBottom = el.offsetTop + el.offsetHeight + 8; } if (changed) force((n) => n + 1); - }, [threads, positions]); + }, [threads, positions, aligned]); return ( -
+
{threads.map((t) => ( Date: Thu, 2 Jul 2026 00:42:12 +0000 Subject: [PATCH 4/5] Correct rail scroll-sync: chrome offset, draft position, mobile reset Three corrections to the desktop rail scroll-sync from review: - The cards list sits below the sticky header + doc-reaction/sign-in rows, but card Y is measured from the document top. Offset the rail scrollTop by the list's own offsetTop so a card lines up with its highlight instead of sitting that chrome-height below it. - Position the draft composer at the selection's document Y, so clicking Comment on a deep selection opens it next to the selected text instead of at the top of a list that's scrolled far away. - Reset the rail scrollTop to 0 when the viewport switches to mobile, so the drawer doesn't open scrolled past its stacked cards onto empty space. Co-Authored-By: Claude Opus 4.7 --- app/d/[slug]/CommentsShell.tsx | 42 ++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/app/d/[slug]/CommentsShell.tsx b/app/d/[slug]/CommentsShell.tsx index d0fc72b..2dbb662 100644 --- a/app/d/[slug]/CommentsShell.tsx +++ b/app/d/[slug]/CommentsShell.tsx @@ -512,15 +512,27 @@ export default function CommentsShell(props: Props) { // The count shown in the toggle: number of threads (visible roots). const commentCount = threads.length; - // Docs-style scroll sync (desktop only): cards are laid out at their highlight's - // absolute document Y, so the rail must scroll in lockstep with the document or a - // deep comment's card is stranded far below an empty rail. On mobile the rail is - // an overlay drawer with no doc alongside it, so cards stack normally instead. + // Docs-style scroll sync: cards are laid out at their highlight's absolute + // document Y, so the rail must scroll in lockstep with the document or a deep + // comment's card is stranded far below an empty rail. useEffect(() => { - if (isMobile) return; const el = railRef.current; - if (el) el.scrollTop = docScrollY; - }, [docScrollY, isMobile]); + if (!el) return; + // Mobile drawer: cards stack from the top (no absolute Y), so clear any leftover + // desktop scroll offset — a stale large scrollTop would open the drawer scrolled + // past the stacked cards onto empty space. + if (isMobile) { + el.scrollTop = 0; + return; + } + // The cards list starts BELOW the sticky header + doc-reaction/sign-in rows, but + // card Y is measured from the document top. Offset the sync by that chrome height + // (the list's own offsetTop) so a card lines up with its highlight instead of + // sitting that far below it. + const cards = el.querySelector("[data-jh-cards]") as HTMLElement | null; + const chromeH = cards ? cards.offsetTop : 0; + el.scrollTop = docScrollY + chromeH; + }, [docScrollY, isMobile, railOpen]); // When dark, expose the variant-D palette as CSS custom properties on the // wrapper. Every themed color below reads `var(--jh-x, )`, so @@ -900,7 +912,7 @@ function RailCards(props: { }, [threads, positions, aligned]); return ( -
+
{threads.map((t) => ( + aligned ? ( + // Place the composer at the selection's document Y so, once the rail is + // synced to the doc scroll, it opens next to the selected text — not + // stranded at the top of a list that's scrolled far away. +
+ +
+ ) : ( + + ) ) : null} {threads.length === 0 && !draft ? ( From be668792e517642b79d0ae8a3211677752868299 Mon Sep 17 00:00:00 2001 From: AnnaXWang <6621137+AnnaXWang@users.noreply.github.com> Date: Thu, 2 Jul 2026 00:58:24 +0000 Subject: [PATCH 5/5] Reapply rail scrollTop when docHeight grows A late layout or resize can report a taller docHeight with unchanged scrollY. The rail's scrollHeight grows via minHeight but the sync effect didn't re-run, so a scrollTop the browser had clamped too low stayed there and cards drifted from their highlights until the next doc scroll. Add docHeight to the effect deps so scrollTop is reapplied. Co-Authored-By: Claude Opus 4.7 --- app/d/[slug]/CommentsShell.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/d/[slug]/CommentsShell.tsx b/app/d/[slug]/CommentsShell.tsx index 2dbb662..3759567 100644 --- a/app/d/[slug]/CommentsShell.tsx +++ b/app/d/[slug]/CommentsShell.tsx @@ -532,7 +532,10 @@ export default function CommentsShell(props: Props) { const cards = el.querySelector("[data-jh-cards]") as HTMLElement | null; const chromeH = cards ? cards.offsetTop : 0; el.scrollTop = docScrollY + chromeH; - }, [docScrollY, isMobile, railOpen]); + // docHeight is a dep though unused above: when it grows (late layout / resize) the + // rail's scrollHeight grows with it, so a scrollTop the browser previously clamped + // too low must be reapplied — otherwise cards stay offset until the next doc scroll. + }, [docScrollY, docHeight, isMobile, railOpen]); // When dark, expose the variant-D palette as CSS custom properties on the // wrapper. Every themed color below reads `var(--jh-x, )`, so