fix(viewer): keep deep-linked inline comment thread aligned to its highlight

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+
+- Opening an inline-comment deep link (`#comment-<id>`) no longer leaves the comment thread pinned in the wrong vertical position. The thread in the right-margin column (and the narrow-screen comment popover) could land hundreds of pixels above its highlighted passage and stay there when content above the passage reflowed *after* the thread was positioned — e.g. a web-font swap on first load, or a late-loading image or diagram. Threads now re-align whenever their highlighted passage moves, not only when the article is resized, so they track the highlight through any late layout shift. A normal click was never affected (it happens after the page has settled).
+
 ## [0.1.27] - 2026-06-22
 
 ### Added

packages/viewer/src/components/PageContent.svelte

@@ -21,6 +21,7 @@
   import IconButton from "$lib/ui/primitives/IconButton.svelte";
   import Popover from "$lib/ui/primitives/Popover.svelte";
   import { useElementSize } from "$lib/ui/hooks/useElementSize.svelte";
+  import { useElementMove } from "$lib/ui/hooks/useElementMove.svelte";
   import { useSelectionPopover } from "$lib/ui/hooks/useSelectionPopover.svelte";
   import { useScrollIntoViewOnNav } from "$lib/ui/hooks/useScrollIntoViewOnNav.svelte";
   import PageComments from "./comments/PageComments.svelte";
@@ -32,10 +33,37 @@
   let articleRef: HTMLElement | undefined = $state();
   let showSkeleton = $state(false);
 
+  // Bumped after a deep-link reveal to force one re-measure of the thread pin.
+  // On a cold load the thread's vertical offset is first computed while the page
+  // is still settling (a web-font subset reflowing the text above the highlight),
+  // and no observer catches that promptly: the article ResizeObserver misses the
+  // load burst, and observeMove — armed while the highlight is still off-screen —
+  // only falls back to its ~1s poll. By the time landOnComment reveals the
+  // target, the reflow has settled, so re-measuring then aligns it immediately.
+  let deepLinkSettleSeq = $state(0);
+
   const docId = $derived(page.data ? documentIdFor(page.data.meta) : null);
 
   const articleSize = useElementSize(() => articleRef ?? null);
 
+  // Track the active inline highlight's position so the pinned thread re-aligns
+  // when content above it reflows (FOUT, late image/diagram load) — a move the
+  // article ResizeObserver can't see. Null when no inline thread is active or
+  // its passage isn't wrapped (resolved / orphaned), which is harmless.
+  const activeHighlightMove = useElementMove(() => {
+    // Read `items` so this re-evaluates (and observeMove re-subscribes to the
+    // fresh wrapper node) after a reconcile rebuilds the highlights — e.g. a
+    // live-reload replaces the article HTML, destroying the old wrapper while
+    // activeId/articleRef are unchanged. Mirrors the data-active effect below;
+    // querySelector alone is not reactive to that node swap.
+    void comments.items;
+    const id = comments.activeId;
+    if (!id || !articleRef) return null;
+    return articleRef.querySelector<HTMLElement>(
+      `rw-annotation[data-comment-id="${escapeId(id)}"]`,
+    );
+  });
+
   // Text-selection state for the Add-comment popover. The hook owns the
   // captured Range, the article-relative anchor point, and dismiss-on-collapse.
   const selectionPopover = useSelectionPopover(() => articleRef ?? null, articleSize);
@@ -144,6 +172,11 @@
       if (comments.linkedId === id) return; // already landed (a racing tick won)
       if (revealCommentTarget(id, kind)) {
         comments.linkedId = id;
+        // Re-measure the pin now the target is revealed and the cold-load reflow
+        // has settled (see deepLinkSettleSeq). rAF so the scroll/layout flushes.
+        requestAnimationFrame(() => {
+          deepLinkSettleSeq++;
+        });
       }
     });
   }
@@ -338,7 +371,15 @@
       comments.activeLeft = null;
       return;
     }
+    // Recompute on article resize AND on the active highlight *moving* — the
+    // latter catches content above it reflowing (web-font swap, a late image /
+    // diagram load) which slides the highlight without changing the article's
+    // box, so the ResizeObserver alone would miss it. Also re-measure once right
+    // after a deep-link reveal, when the cold-load reflow has settled but no
+    // observer has fired yet (see deepLinkSettleSeq).
     void articleSize.version;
+    void activeHighlightMove.version;
+    void deepLinkSettleSeq;
     const anchor = getHighlightAnchor(activeId);
     comments.activeTop = anchor?.top ?? null;
     comments.activeLeft = anchor ? clampPopoverLeft(anchor.centerX, articleRef.clientWidth) : null;

packages/viewer/src/lib/ui/hooks/__fixtures__/ElementMoveHarness.svelte

@@ -0,0 +1,18 @@
+<!--
+  Minimal harness that exercises useElementMove through a real component
+  lifecycle (mount / prop-change / unmount), so its internal $effect actually
+  runs. Tests read the reactive version from a data-* attribute.
+-->
+<script lang="ts">
+  import { useElementMove } from "../useElementMove.svelte";
+
+  interface Props {
+    el: HTMLElement | null;
+  }
+
+  let { el }: Props = $props();
+
+  const move = useElementMove(() => el);
+</script>
+
+<div data-testid="element-move" data-version={move.version}></div>

packages/viewer/src/lib/ui/hooks/__fixtures__/intersection-observer-mock.ts

@@ -0,0 +1,47 @@
+// jsdom does not implement IntersectionObserver, and `observeMove` needs
+// programmatic control over the callback to simulate the element moving. A
+// minimal mock: store the callback + options, track observed elements, expose
+// a `trigger(ratio)` helper. Mirrors the ResizeObserver mock in this folder.
+export class MockIntersectionObserver {
+  static instances: MockIntersectionObserver[] = [];
+  callback: IntersectionObserverCallback;
+  options?: IntersectionObserverInit;
+  observed: Element[] = [];
+  disconnected = false;
+
+  constructor(cb: IntersectionObserverCallback, options?: IntersectionObserverInit) {
+    this.callback = cb;
+    this.options = options;
+    MockIntersectionObserver.instances.push(this);
+  }
+
+  observe(el: Element) {
+    this.observed.push(el);
+  }
+
+  unobserve() {}
+
+  disconnect() {
+    this.disconnected = true;
+    this.observed = [];
+  }
+
+  takeRecords(): IntersectionObserverEntry[] {
+    return [];
+  }
+
+  /** Simulate an intersection change at the given ratio (1 = unmoved). */
+  trigger(ratio: number) {
+    this.callback(
+      this.observed.map(
+        (target) => ({ target, intersectionRatio: ratio }) as IntersectionObserverEntry,
+      ),
+      this as unknown as IntersectionObserver,
+    );
+  }
+
+  /** The most recently constructed (currently-armed) observer. */
+  static get latest(): MockIntersectionObserver | undefined {
+    return MockIntersectionObserver.instances[MockIntersectionObserver.instances.length - 1];
+  }
+}

packages/viewer/src/lib/ui/hooks/observeElement.svelte.ts

@@ -9,6 +9,10 @@
  * viewport-relative coordinates; skip it for size-only hooks, since size is
  * scroll-invariant and listening would only produce redundant work.
  *
+ * This observes the element's own box only. Position changes caused by content
+ * *above* it reflowing (which leave its box unchanged) are the job of
+ * `observeMove`; consumers that anchor to an element use both.
+ *
  * Implementation detail of `useElementSize` / `useAnchorOffset` — not exported
  * outside the hooks/ layer.
  */

packages/viewer/src/lib/ui/hooks/observeMove.svelte.ts

@@ -0,0 +1,102 @@
+/**
+ * Run `onMove(el)` whenever the element's *position* changes — even when its
+ * own box does not resize. Re-subscribes automatically when the getter starts
+ * returning a different element, and tears down on cleanup.
+ *
+ * This fills the gap a `ResizeObserver` cannot: when content *above* an element
+ * reflows (web-font swap / FOUT, a late-loading image or diagram, an expanding
+ * block), the element slides down without its own width/height changing — so a
+ * ResizeObserver on it (or on an ancestor whose box is unchanged) never fires.
+ * Positioning that is anchored to such an element (a comment thread pinned to
+ * its highlight) would otherwise go stale.
+ *
+ * Implemented with an `IntersectionObserver` whose `rootMargin` is computed to
+ * frame the element exactly, so the observed ratio drops the instant the
+ * element moves; the callback then re-measures and re-arms. This is the
+ * mechanism behind Floating UI's `autoUpdate({ layoutShift: true })`, ported
+ * here (MIT-licensed) so the viewer needs no extra dependency.
+ *
+ * Like a scroll listener it also fires on scroll (the element moves in the
+ * viewport); consumers whose computed value is scroll-invariant simply
+ * recompute the same value — cheap and idempotent.
+ *
+ * Implementation detail of the hooks/ layer — not exported outside it.
+ */
+export function observeMove(
+  getEl: () => HTMLElement | null,
+  onMove: (el: HTMLElement) => void,
+): void {
+  $effect(() => {
+    const el = getEl();
+    if (!el || typeof IntersectionObserver === "undefined") return;
+
+    let alive = true;
+    let io: IntersectionObserver | null = null;
+    let timeoutId: ReturnType<typeof setTimeout> | undefined;
+    const root = el.ownerDocument.documentElement;
+
+    function cleanup() {
+      if (timeoutId !== undefined) clearTimeout(timeoutId);
+      timeoutId = undefined;
+      io?.disconnect();
+      io = null;
+    }
+
+    function refresh(skip: boolean, threshold: number) {
+      cleanup();
+      if (!alive || !el) return;
+
+      const { left, top, width, height } = el.getBoundingClientRect();
+      if (!skip) onMove(el);
+      // A zero-area (detached / display:none) element can't be framed; bail and
+      // wait for the next re-subscribe to re-arm once it has a box again.
+      if (!width || !height) return;
+
+      // Negative insets frame the element exactly, so threshold=1 means "fully
+      // in view at this precise position". Any movement drops the ratio.
+      const rootMargin =
+        `${-Math.floor(top)}px ${-Math.floor(root.clientWidth - (left + width))}px ` +
+        `${-Math.floor(root.clientHeight - (top + height))}px ${-Math.floor(left)}px`;
+
+      let isFirstUpdate = true;
+      const handle = (entries: IntersectionObserverEntry[]) => {
+        // A delivery already queued when the effect tore down must not re-arm a
+        // timer on the dead closure (cleanup has already run and won't clear it).
+        if (!alive) return;
+        // Use the most recent entry; a batched callback is not ordered latest-last.
+        const ratio = entries[entries.length - 1].intersectionRatio;
+        if (ratio !== threshold) {
+          if (!isFirstUpdate) {
+            refresh(false, 1);
+          } else if (ratio) {
+            // Re-arm at the actual ratio so the next genuine move retriggers.
+            refresh(false, ratio);
+          } else {
+            // Ratio 0 (e.g. scrolled fully off-screen): poll back in a beat.
+            timeoutId = setTimeout(() => refresh(false, 1e-7), 1000);
+          }
+        }
+        isFirstUpdate = false;
+      };
+
+      const options: IntersectionObserverInit = {
+        rootMargin,
+        threshold: Math.max(0, Math.min(1, threshold)) || 1,
+      };
+      try {
+        // Observe relative to the document so an ancestor's reflow is caught.
+        io = new IntersectionObserver(handle, { ...options, root: el.ownerDocument });
+      } catch {
+        io = new IntersectionObserver(handle, options);
+      }
+      io.observe(el);
+    }
+
+    refresh(true, 1);
+
+    return () => {
+      alive = false;
+      cleanup();
+    };
+  });
+}

packages/viewer/src/lib/ui/hooks/useAnchorOffset.svelte.test.ts

@@ -3,11 +3,14 @@ import { flushSync } from "svelte";
 import { render } from "@testing-library/svelte";
 import Harness from "./__fixtures__/AnchorOffsetHarness.svelte";
 import { MockResizeObserver, makeAnchor } from "./__fixtures__/resize-observer-mock";
+import { MockIntersectionObserver } from "./__fixtures__/intersection-observer-mock";
 
 describe("useAnchorOffset", () => {
   beforeEach(() => {
     MockResizeObserver.instances = [];
+    MockIntersectionObserver.instances = [];
     vi.stubGlobal("ResizeObserver", MockResizeObserver);
+    vi.stubGlobal("IntersectionObserver", MockIntersectionObserver);
   });
 
   afterEach(() => {
@@ -191,4 +194,25 @@ describe("useAnchorOffset", () => {
     expect(out.dataset.top).toBe("500");
     expect(out.dataset.width).toBe("30");
   });
+
+  it("updates the rect fields when the anchor moves (content above it reflows)", () => {
+    // The anchor's own box is unchanged (no ResizeObserver fire) — it just slid
+    // down because content above it grew. observeMove must catch this.
+    let currentRect = { top: 100, left: 50, width: 100, height: 50 };
+    const el = document.createElement("div");
+    el.getBoundingClientRect = () =>
+      ({ ...currentRect, right: 0, bottom: 0, x: 0, y: 0, toJSON: () => ({}) }) as DOMRect;
+
+    const { getByTestId } = render(Harness, { el });
+    const out = getByTestId("anchor-offset");
+    expect(out.dataset.top).toBe("100");
+    expect(MockIntersectionObserver.instances.length).toBeGreaterThanOrEqual(1);
+
+    // Anchor slid down to 600 with no size change; the move observer fires.
+    currentRect = { top: 600, left: 50, width: 100, height: 50 };
+    MockIntersectionObserver.latest!.trigger(0.5);
+    flushSync();
+
+    expect(out.dataset.top).toBe("600");
+  });
 });

packages/viewer/src/lib/ui/hooks/useAnchorOffset.svelte.ts

@@ -1,14 +1,17 @@
 import { observeElement } from "./observeElement.svelte";
+import { observeMove } from "./observeMove.svelte";
 
 /**
- * Track the viewport-relative bounding rect of an element across resizes and
- * ancestor scrolls.
+ * Track the viewport-relative bounding rect of an element across resizes,
+ * ancestor scrolls, and position changes.
  *
  * Returns a reactive object whose `top`/`left`/`width`/`height` values update
- * whenever the element resizes, the window resizes, or any scroll container
- * scrolls. Consumers read the fields inside `$derived` (or directly in markup)
- * to drive positioning — typically for popover panels that need to sit beside
- * an external anchor.
+ * whenever the element resizes, the window resizes, any scroll container
+ * scrolls, or the element *moves* (content above it reflows — web-font swap, a
+ * late image/diagram load — sliding the anchor without resizing it). Consumers
+ * read the fields inside `$derived` (or directly in markup) to drive
+ * positioning — typically for popover panels that need to sit beside an
+ * external anchor. Together these triggers mirror Floating UI's `autoUpdate`.
  *
  * The getter form lets the anchor target switch at runtime (e.g. when a parent
  * mounts a different trigger); the internal `$effect` re-subscribes whenever
@@ -36,18 +39,17 @@ export interface AnchorOffset {
 export function useAnchorOffset(getEl: () => HTMLElement | null): AnchorOffset {
   const rect = $state({ top: 0, left: 0, width: 0, height: 0, measured: false });
 
-  observeElement(
-    getEl,
-    (el) => {
-      const r = el.getBoundingClientRect();
-      rect.top = r.top;
-      rect.left = r.left;
-      rect.width = r.width;
-      rect.height = r.height;
-      rect.measured = true;
-    },
-    { trackWindow: true },
-  );
+  const measure = (el: HTMLElement) => {
+    const r = el.getBoundingClientRect();
+    rect.top = r.top;
+    rect.left = r.left;
+    rect.width = r.width;
+    rect.height = r.height;
+    rect.measured = true;
+  };
+
+  observeElement(getEl, measure, { trackWindow: true });
+  observeMove(getEl, measure);
 
   return {
     get top() {