Skip to content

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

Merged
yumike merged 1 commit into
mainfrom
fix/comment-deeplink-vertical-pin
Jun 23, 2026
Merged

fix(viewer): keep deep-linked inline comment thread aligned to its highlight#570
yumike merged 1 commit into
mainfrom
fix/comment-deeplink-vertical-pin

Conversation

@yumike

@yumike yumike commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Problem

Opening an inline-comment deep link (#comment-<id>) could leave the comment thread in the right-margin column pinned hundreds of pixels above its highlighted passage. The thread is positioned by an article-relative offset (activeTop) that only recomputed on the article's ResizeObserver — which reports the article's box size, not the highlight's position within it. So when content above the highlight reflowed (a web-font swap on cold load, or a late image/diagram), the highlight slid down without the article resizing, the offset went stale, and the thread floated away from its passage.

A second symptom on hard refresh: the thread appeared instantly but mispositioned, then snapped into place ~1s later.

Fix

Trigger the re-measure at the right altitude, with two complementary mechanisms:

  • observeMove — a small IntersectionObserver primitive (dynamic rootMargin, re-arming on each move; a dependency-free MIT port of Floating UI's autoUpdate({layoutShift})) that fires when an anchored element moves even though its own box is unchanged. Exposed as useElementMove, wired into the margin pin (active highlight) and useAnchorOffset (so the narrow-screen comment popover re-aligns too). Catches any reflow after the thread exists — live-reload re-wraps, late images/diagrams. The move getter reads comments.items so it re-subscribes to the fresh wrapper after a reconcile rebuilds highlights.
  • A one-shot re-measure right after a deep-link reveal. On a cold load the stranding reflow happens during the load burst — before observeMove is watching, and while the highlight is still off-screen (so observeMove would only correct it via its ~1s poll). By reveal time the reflow has settled, so a single re-measure there aligns it immediately instead of after a visible ~1s delay.

Verification

  • Reproduced on a real long page: deep link landed −683px off on the old build → −39px aligned on the fixed build; the hard-refresh settle dropped from ~1000ms → ~3ms (1 frame), consistently across reloads.
  • Generalization confirmed: a simulated late image/diagram above the highlight re-aligns within ~2 frames (the case the article ResizeObserver structurally can't see).
  • 591 viewer unit tests (new observeMove/useElementMove/useAnchorOffset-move coverage), 45 comment e2e, typecheck + lint all green; no regression in existing deep-link/alignment tests.

Follow-up (not in this PR)

The upstream trigger is unmanaged web-font FOUT (fonts.css ships font-display: swap with no metric-matched fallback) plus dimensionless <img>. Adding size-adjust/ascent-override fallback faces is an independent CLS win that would shrink the deep-link settle to a rare belt-and-suspenders path — tracked separately.

🤖 Generated with Claude Code

…ghlight

An inline comment thread is pinned to its highlighted passage in the
right-margin column via an article-relative offset (`activeTop`) that only
recomputed on the article's ResizeObserver. That observer reports the
article's box size, not the highlight's position within it — so when content
*above* the highlight reflowed (a web-font swap on cold load, or a late
image/diagram) the highlight slid down without the article resizing, the
offset went stale, and the thread floated hundreds of px above its passage.

Fix the trigger at the right altitude with two complementary mechanisms:

- `observeMove` — a small IntersectionObserver primitive (dynamic rootMargin,
  re-arming on each move; a dependency-free MIT port of Floating UI's
  autoUpdate layoutShift) that fires when an anchored element *moves* even
  though its own box is unchanged. Exposed as `useElementMove` and wired into
  the margin pin (the active highlight) and `useAnchorOffset` (so the
  narrow-screen comment popover re-aligns too). Catches any reflow *after* the
  thread exists — live-reload re-wraps, late images/diagrams. The move getter
  reads `comments.items` so it re-subscribes to the fresh wrapper after a
  reconcile rebuilds highlights.

- A one-shot re-measure right after a deep-link reveal. On a cold load the
  stranding reflow happens during the load burst — before observeMove is
  watching, and while the highlight is still off-screen (so observeMove would
  only correct it via its ~1s poll). By reveal time the reflow has settled, so
  a single re-measure there aligns it immediately instead of after a visible
  ~1s delay.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yumike yumike merged commit 9dc2023 into main Jun 23, 2026
25 of 26 checks passed
@yumike yumike deleted the fix/comment-deeplink-vertical-pin branch June 23, 2026 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant