feat(viewer): inline comment threads in a popover on narrow screens

CHANGELOG.md

@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - A reply draft typed into one comment thread no longer appears in every other thread. Drafts are now scoped to the thread they were written in: switching threads (with `n`/`p` or the prev/next buttons) shows each thread's own draft, an untouched thread stays empty, and returning to a thread restores the draft you left there. Drafts clear on submit and when you navigate to another page.
 - Comments from the previous page no longer briefly reappear after navigating to a page that shows none (e.g. Home): a now-superseded comment fetch that resolves after you've navigated away is dropped instead of repopulating the just-cleared list. Resolving or reopening a comment while a navigation is in flight likewise no longer writes the change into the next page's comment view.
 - Accessibility: the active navigation link and the active "On this page" outline entry now expose `aria-current`, so screen readers announce which page and heading you're on (previously conveyed by color alone); copying a comment's share link is announced via a polite live region rather than only swapping the button icon; and the desktop navigation landmark now carries an accessible name ("Documentation"), distinct from the breadcrumb, table-of-contents, and mobile-navigation landmarks.
+- Inline-comment threads are now reachable on narrow windows and phones. Below the width where the right-margin comment column is hidden, tapping a highlighted passage opens its thread in a popover anchored to the highlight (with replies, resolve, and delete), and selecting text → "Add comment" opens the draft box there too — previously both silently did nothing because the only thread surface was the hidden margin column. Escape or tapping away dismisses it; `n`/`p` navigation and tapping another highlight move the popover to that comment.
 
 ## [0.1.26] - 2026-06-21
 

packages/viewer/e2e/comment-narrow-popover.spec.ts

@@ -0,0 +1,267 @@
+import { test, expect, Page } from "@playwright/test";
+import { resolveDocumentId } from "./comment-helpers";
+
+// Narrow viewport: below the 952px comments breakpoint, so the margin aside is
+// hidden and the CommentPopover is the only inline-thread surface.
+test.use({ viewport: { width: 700, height: 900 } });
+test.describe.configure({ mode: "serial" });
+
+// Own page so rows never race comments.spec (homepage) or the deeplink/nav specs.
+const DOC_URL = "advanced";
+const DOC_PATH = "/advanced";
+
+// Select `text` inside the article and fire the mouseup the viewer listens for.
+// Copied from comments.spec.ts.
+async function selectText(page: Page, text: string) {
+  await page.evaluate((targetText) => {
+    const article = document.querySelector("article");
+    if (!article) throw new Error("no article");
+    const fullText = article.textContent ?? "";
+    const startInDoc = fullText.indexOf(targetText);
+    if (startInDoc === -1) throw new Error(`text "${targetText}" not found`);
+    const endInDoc = startInDoc + targetText.length;
+    const walker = document.createTreeWalker(article, NodeFilter.SHOW_TEXT);
+    let offset = 0;
+    let startNode: Text | null = null;
+    let startOffset = 0;
+    let endNode: Text | null = null;
+    let endOffset = 0;
+    while (walker.nextNode()) {
+      const node = walker.currentNode as Text;
+      const len = node.data.length;
+      if (!startNode && offset + len > startInDoc) {
+        startNode = node;
+        startOffset = startInDoc - offset;
+      }
+      if (startNode && offset + len >= endInDoc) {
+        endNode = node;
+        endOffset = endInDoc - offset;
+        break;
+      }
+      offset += len;
+    }
+    if (!startNode || !endNode) throw new Error(`couldn't build range for "${targetText}"`);
+    const range = document.createRange();
+    range.setStart(startNode, startOffset);
+    range.setEnd(endNode, endOffset);
+    const selection = window.getSelection()!;
+    selection.removeAllRanges();
+    selection.addRange(range);
+    const rect = range.getBoundingClientRect();
+    article.dispatchEvent(
+      new MouseEvent("mouseup", {
+        bubbles: true,
+        clientX: rect.left + rect.width / 2,
+        clientY: rect.top + rect.height / 2,
+      }),
+    );
+  }, text);
+}
+
+// Seed an inline comment anchored to `quote`. Verified payload shape from
+// comment-deeplink.spec.ts: { documentId, body, quote }.
+async function seedInline(page: Page, body: string, quote: string): Promise<string> {
+  const doc = await resolveDocumentId(page, DOC_URL);
+  return page.evaluate(
+    async ({ body, quote, doc }) => {
+      const res = await fetch("/_api/comments", {
+        method: "POST",
+        headers: { "Content-Type": "application/json" },
+        body: JSON.stringify({ documentId: doc, body, quote }),
+      });
+      if (!res.ok) throw new Error(`POST /_api/comments -> ${res.status}`);
+      return (await res.json()).id as string;
+    },
+    { body, quote, doc },
+  );
+}
+
+// Resolve every open comment so each test starts clean. Copied from
+// comment-deeplink.spec.ts.
+async function resolveAllComments(page: Page) {
+  const doc = await resolveDocumentId(page, DOC_URL);
+  const open = await page.evaluate(async (docId) => {
+    const res = await fetch(`/_api/comments?documentId=${encodeURIComponent(docId)}&status=open`);
+    return res.json();
+  }, doc);
+  for (const c of open) {
+    await page.evaluate(async (id) => {
+      await fetch(`/_api/comments/${id}`, {
+        method: "PATCH",
+        headers: { "Content-Type": "application/json" },
+        body: JSON.stringify({ status: "resolved" }),
+      });
+    }, c.id);
+  }
+}
+
+test.beforeEach(async ({ page }) => {
+  await page.goto(DOC_PATH);
+  await resolveAllComments(page);
+});
+
+// The popover is a labelled group (role="group" aria-label="Comments"), distinct
+// from the wide-screen <aside> complementary "Comments" landmark and from the
+// page-comments <section> region (also named "Comments").
+const popover = (page: Page) => page.getByRole("group", { name: "Comments" });
+
+test("tapping a highlight opens the thread in a popover; no margin aside", async ({ page }) => {
+  await seedInline(page, "Needs a definition here", "Performance optimization");
+  await page.reload();
+
+  await page.locator("rw-annotation").first().click();
+
+  await expect(popover(page)).toBeVisible();
+  await expect(popover(page).getByText("Needs a definition here")).toBeVisible();
+  // The 320px margin aside is not rendered at this width.
+  await expect(page.getByRole("complementary", { name: "Comments" })).toHaveCount(0);
+});
+
+test("Escape dismisses the popover", async ({ page }) => {
+  await seedInline(page, "Dismiss me", "Performance optimization");
+  await page.reload();
+
+  await page.locator("rw-annotation").first().click();
+  await expect(popover(page)).toBeVisible();
+
+  await page.keyboard.press("Escape");
+  await expect(popover(page)).toBeHidden();
+});
+
+test("Escape inside the reply box blurs the field without closing the thread", async ({ page }) => {
+  await seedInline(page, "Has a reply box", "Performance optimization");
+  await page.reload();
+
+  await page.locator("rw-annotation").first().click();
+  const reply = popover(page).getByPlaceholder("Write a reply...");
+  await reply.fill("draft reply text");
+  await reply.press("Escape");
+
+  // The thread stays open and the in-progress reply draft is preserved — Escape
+  // only blurs the field; it does not tear the whole popover down.
+  await expect(popover(page)).toBeVisible();
+  await expect(reply).toHaveValue("draft reply text");
+});
+
+test("Escape during an IME composition does not close the popover", async ({ page }) => {
+  await seedInline(page, "Has a reply box", "Performance optimization");
+  await page.reload();
+
+  await page.locator("rw-annotation").first().click();
+  const reply = popover(page).getByPlaceholder("Write a reply...");
+  await reply.focus();
+
+  // Simulate Escape cancelling an in-progress IME composition (isComposing=true).
+  // CommentForm deliberately skips its blur/preventDefault while composing, so the
+  // popover's own handler must also ignore it rather than tear the thread down.
+  await reply.evaluate((el) =>
+    el.dispatchEvent(
+      new KeyboardEvent("keydown", { key: "Escape", isComposing: true, bubbles: true }),
+    ),
+  );
+
+  await expect(popover(page)).toBeVisible();
+});
+
+test("clicking outside (not on a highlight) dismisses the popover", async ({ page }) => {
+  await seedInline(page, "Outside click", "Performance optimization");
+  await page.reload();
+
+  await page.locator("rw-annotation").first().click();
+  await expect(popover(page)).toBeVisible();
+
+  // Click the page heading — outside the popover and not a highlight.
+  await page.getByRole("heading", { name: "Advanced Topics" }).click();
+  await expect(popover(page)).toBeHidden();
+});
+
+test("selecting text and adding a comment shows the draft in the popover", async ({ page }) => {
+  await selectText(page, "Performance Tips");
+  await page.getByRole("button", { name: "Add comment" }).click();
+
+  await expect(popover(page)).toBeVisible();
+  const draft = popover(page).getByRole("textbox");
+  await draft.fill("A brand new inline comment");
+  await popover(page).getByRole("button", { name: "Comment", exact: true }).click();
+
+  await expect(popover(page).getByText("A brand new inline comment")).toBeVisible();
+});
+
+test("tapping a second highlight switches the popover instead of closing it", async ({ page }) => {
+  // Two passages far apart vertically (top Topics list vs the bottom Performance
+  // Tips list) so the first comment's popover does not overlap — and intercept
+  // the click on — the second highlight.
+  await seedInline(page, "First comment", "Performance optimization");
+  await seedInline(page, "Second comment", "Enable compression");
+  await page.reload();
+
+  const highlights = page.locator("rw-annotation");
+  await highlights.nth(0).click();
+  await expect(popover(page).getByText("First comment")).toBeVisible();
+
+  await highlights.nth(1).click();
+  // Switched, not closed.
+  await expect(popover(page).getByText("Second comment")).toBeVisible();
+  await expect(popover(page).getByText("First comment")).toHaveCount(0);
+});
+
+test("n opens the popover on the navigated comment", async ({ page }) => {
+  await seedInline(page, "Nav target comment", "Performance optimization");
+  await page.reload();
+
+  // Wait for the highlight to anchor before navigating, else `n` fires against an
+  // empty navigable list (the comment hasn't loaded yet).
+  await expect(page.locator("rw-annotation").first()).toBeVisible();
+  await page.keyboard.press("n");
+
+  await expect(popover(page)).toBeVisible();
+  await expect(popover(page).getByText("Nav target comment")).toBeVisible();
+});
+
+test("r moves focus into the popover's reply box", async ({ page }) => {
+  await seedInline(page, "Focus my reply", "Performance optimization");
+  await page.reload();
+
+  await page.locator("rw-annotation").first().click();
+  await expect(popover(page)).toBeVisible();
+
+  // `r` is a global shortcut that focuses the active thread's reply box; it must
+  // reach the box rendered inside the popover (not only the wide aside).
+  await page.keyboard.press("r");
+  await expect(popover(page).getByPlaceholder("Write a reply...")).toBeFocused();
+});
+
+test("a reply draft survives switching threads in the popover", async ({ page }) => {
+  await seedInline(page, "First thread", "Performance optimization");
+  await seedInline(page, "Second thread", "Enable compression");
+  await page.reload();
+
+  const highlights = page.locator("rw-annotation");
+  await highlights.nth(0).click();
+  await popover(page).getByPlaceholder("Write a reply...").fill("draft for first");
+
+  // Switch to the second thread (the draft is per-thread, so its box is empty)…
+  await highlights.nth(1).click();
+  await expect(popover(page).getByText("Second thread")).toBeVisible();
+  await expect(popover(page).getByPlaceholder("Write a reply...")).toHaveValue("");
+
+  // …then back to the first: the draft is restored, not lost on the remount.
+  await highlights.nth(0).click();
+  await expect(popover(page).getByText("First thread")).toBeVisible();
+  await expect(popover(page).getByPlaceholder("Write a reply...")).toHaveValue("draft for first");
+});
+
+test("at wide widths the margin aside is used and the popover is absent", async ({ page }) => {
+  await page.setViewportSize({ width: 1400, height: 900 });
+  await page.goto(DOC_PATH);
+  await resolveAllComments(page);
+  await seedInline(page, "Wide comment", "Performance optimization");
+  await page.reload();
+
+  await page.locator("rw-annotation").first().click();
+
+  const aside = page.getByRole("complementary", { name: "Comments" });
+  await expect(aside).toBeVisible();
+  await expect(aside.getByText("Wide comment")).toBeVisible();
+  await expect(popover(page)).toHaveCount(0);
+});

packages/viewer/src/components/Layout.svelte

@@ -9,11 +9,13 @@
   import MobileDrawer from "./MobileDrawer.svelte";
   import IconButton from "$lib/ui/primitives/IconButton.svelte";
   import LoadingBar from "$lib/ui/primitives/LoadingBar.svelte";
-  import CommentSidebar from "./comments/CommentSidebar.svelte";
+  import CommentPanel from "./comments/CommentPanel.svelte";
   import Alert from "$lib/ui/primitives/Alert.svelte";
   import Toaster from "./Toaster.svelte";
   import { useActiveHeading } from "$lib/ui/hooks/useActiveHeading.svelte";
   import { useCommentNavigation } from "$lib/ui/hooks/useCommentNavigation.svelte";
+  import { useElementSize } from "$lib/ui/hooks/useElementSize.svelte";
+  import { isLayoutNarrow } from "$lib/ui/breakpoints";
 
   interface Props {
     children: Snippet;
@@ -46,6 +48,16 @@
     requestReplyFocus: comments.focusReply,
   });
 
+  // The layout's breakpoints are CSS container queries (for embedded mode), so
+  // JS has no inherent knowledge of the 952px comments breakpoint. Measure the
+  // container (the pattern Breadcrumbs already uses) to drive ui.narrow, which
+  // chooses the in-flow comments aside (wide) vs the CommentPopover (narrow).
+  let layoutContainerEl = $state<HTMLDivElement>();
+  const layoutSize = useElementSize(() => layoutContainerEl ?? null);
+  $effect(() => {
+    ui.narrow = isLayoutNarrow(layoutSize.width);
+  });
+
   function onTocNavigate(id: string) {
     const element = document.getElementById(id);
     if (!element) return;
@@ -92,9 +104,10 @@
 </script>
 
 <div
+  bind:this={layoutContainerEl}
   class="layout-container"
   data-testid="viewer-root"
-  data-comments-active={comments.activeIsInline || comments.pending ? "" : undefined}
+  data-comments-active={comments.inlineSurfaceActive ? "" : undefined}
 >
   <LoadingBar loading={page.loading} />
   <Toaster />
@@ -210,10 +223,10 @@
           </main>
 
           <!-- Right Sidebar: Comments (when active or drafting) or Table of Contents -->
-          {#if comments.activeIsInline || comments.pending}
+          {#if comments.inlineSurfaceActive && !ui.narrow}
             <aside aria-label="Comments" class="layout-comments hidden w-[320px] shrink-0">
               <div class="pl-8">
-                <CommentSidebar />
+                <CommentPanel />
               </div>
             </aside>
           {:else if page.data && tocEntries.length > 0}
@@ -329,7 +342,9 @@
 
   /* When comments are active, prioritize comments over nav sidebar.
      At medium widths: show comments, hide nav.
-     At wider widths: show both. */
+     At wider widths: show both.
+     NOTE: 952px mirrors COMMENTS_BREAKPOINT_PX in $lib/ui/breakpoints — keep in
+     sync; below it the CommentPopover replaces this aside. */
   @container (min-width: 952px) {
     :global([data-comments-active]) .layout-sidebar {
       display: none;