Comment viewer fixes: rendering, dark treatment, composer UX

app/d/[slug]/CommentsShell.tsx

@@ -127,6 +127,11 @@ export default function CommentsShell(props: Props) {
   const [pinnedId, setPinnedId] = useState<number | null>(null);
   const [activeId, setActiveId] = useState<number | null>(null);
   const [positions, setPositions] = useState<Record<number, number>>({});
+  // 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
@@ -463,12 +470,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]
@@ -494,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();
@@ -509,6 +512,31 @@ 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: 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(() => {
+    const el = railRef.current;
+    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;
+    // 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, <light-literal>)`, so
   // when the vars are UNSET (light docs / no theme) the bytes resolve to today's
@@ -561,6 +589,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" });
@@ -632,6 +664,8 @@ export default function CommentsShell(props: Props) {
           <RailCards
             threads={visibleThreads}
             positions={positions}
+            aligned={!isMobile}
+            docHeight={docHeight}
             pinnedId={pinnedId}
             activeId={activeId}
             canComment={canComment}
@@ -806,6 +840,8 @@ function SelectionToolbar({
 function RailCards(props: {
   threads: Thread[];
   positions: Record<number, number>;
+  aligned: boolean;
+  docHeight: number;
   pinnedId: number | null;
   activeId: number | null;
   canComment: boolean;
@@ -822,6 +858,8 @@ function RailCards(props: {
   const {
     threads,
     positions,
+    aligned,
+    docHeight,
     pinnedId,
     activeId,
     canComment,
@@ -844,8 +882,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;
@@ -863,10 +912,10 @@ function RailCards(props: {
       lastBottom = el.offsetTop + el.offsetHeight + 8;
     }
     if (changed) force((n) => n + 1);
-  }, [threads, positions]);
+  }, [threads, positions, aligned]);
 
   return (
-    <div style={{ position: "relative", padding: "6px 8px 200px" }}>
+    <div data-jh-cards="" style={{ position: "relative", padding: "6px 8px 200px", minHeight: aligned && docHeight ? docHeight : undefined }}>
       {threads.map((t) => (
         <Card
           key={t.id}
@@ -889,10 +938,16 @@ function RailCards(props: {
       ))}
 
       {draft ? (
-        <DraftCard
-          onSubmit={onSubmitDraft}
-          onCancel={onCancelDraft}
-        />
+        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.
+          <div style={{ position: "absolute", left: 8, right: 8, top: Math.max(0, draft.top) }}>
+            <DraftCard onSubmit={onSubmitDraft} onCancel={onCancelDraft} />
+          </div>
+        ) : (
+          <DraftCard onSubmit={onSubmitDraft} onCancel={onCancelDraft} />
+        )
       ) : null}
 
       {threads.length === 0 && !draft ? (

lib/docs/overlay.ts

@@ -12,10 +12,9 @@
 //                     { 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:positions", positions:{ [id]: yTopPx }, docHeight, scrollY }
+//                          (comment highlight y in doc space; doc scroll for rail sync)
 //                     { type:"jh:selection", anchor:{exact,prefix,suffix}, rect:{...} }
 //                     { type:"jh:selectionCleared" }
 //                     { type:"jh:focus", key, keys }      (a segment was clicked: focused key + full covering set)
@@ -65,7 +64,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,"&amp;").replace(/</g,"&lt;").replace(/>/g,"&gt;"); }
@@ -120,13 +118,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 +158,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 <span> — 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<nodes.length;i++){
+      var e = nodes[i], len = e.node.nodeValue.length;
+      if (offset >= 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 +213,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; }
   }
@@ -231,17 +245,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 <html> set from sampleTheme (which
-      // honors the shell's light/dark toggle via forcedMode).
-      + "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 <html> 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;"
@@ -543,7 +556,7 @@ export const OVERLAY_SCRIPT = String.raw`
       rec.segEls.forEach(function(el){ var rt = el.getBoundingClientRect().top + window.scrollY; if (rt < top) top = rt; });
       if (top !== Infinity) pos[it.id] = top;
     });
-    send({type:"jh:positions", positions: pos, docHeight: document.documentElement.scrollHeight});
+    send({type:"jh:positions", positions: pos, docHeight: document.documentElement.scrollHeight, scrollY: window.scrollY});
   }
 
   function scrollToKey(key){
@@ -647,7 +660,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"}); }
   });