From 6ed5540b1ac48f2945485eeac91fe64051072ecb Mon Sep 17 00:00:00 2001
From: Mike Yumatov <mike.yumatov@gmail.com>
Date: Mon, 22 Jun 2026 11:27:28 +0300
Subject: [PATCH] feat(viewer): icon-only "Add comment" selection popover
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The popover shown when you select text in a doc is now icon-only — the
speech-bubble icon, with the visible "Add comment" text dropped — for a
more compact affordance. The button is switched to the shared IconButton
primitive, which requires an aria-label, so the accessible name
"Add comment" is preserved (screen readers and the role+name e2e
selectors are unaffected). The floating chrome (shadow-lg) moves onto the
button and the Popover wrapper becomes a pure positioner, dropping the
hardcoded light/dark colors in favor of IconButton's theme tokens.

Also hardens the shared IconButton primitive: it spread props onto a bare
<button> defaulting to type="submit"; default it to type="button" (like
Button.svelte) so none of its call sites can accidentally submit an
enclosing form. Callers can still override.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 CHANGELOG.md                                  |  4 ++++
 .../viewer/src/components/PageContent.svelte  | 20 +++++++++----------
 .../src/lib/ui/primitives/IconButton.svelte   |  4 ++++
 .../src/lib/ui/primitives/IconButton.test.ts  | 10 ++++++++++
 4 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5bfc3363..3ea61e63 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Press `r` while a comment thread is active (highlighted with `n`/`p`) to move keyboard focus straight into that thread's reply box — no mouse needed. Works for both the inline margin thread and page-timeline/orphaned threads, scrolls the reply box into view on long threads, and announces the move to screen readers. The existing reply shortcuts still apply: Cmd/Ctrl+Enter submits and Escape releases the box so `n`/`p` resume. `r` is ignored while you're already typing, when no thread is active, or on a resolved thread.
 
+### Changed
+
+- The "Add comment" button that appears when you select text in a doc is now icon-only (a speech-bubble icon) instead of icon + "Add comment" text, for a more compact popover. The button keeps its "Add comment" accessible name, so screen readers and keyboard users are unaffected.
+
 ### Fixed
 
 - Comment highlighting no longer re-walks and re-wraps the entire article on every comment action (resolve, reply, reopen) or background live-refresh — only the changed highlight is updated. On long pages with many comments this removes a visible hitch, and a background refresh no longer wipes an in-progress text selection unless the change overlaps the selected text.
diff --git a/packages/viewer/src/components/PageContent.svelte b/packages/viewer/src/components/PageContent.svelte
index 1990c349..7d8a95bd 100644
--- a/packages/viewer/src/components/PageContent.svelte
+++ b/packages/viewer/src/components/PageContent.svelte
@@ -17,7 +17,7 @@
   import { documentIdFor } from "$lib/comments/documentId";
   import LoadingSkeleton from "$lib/ui/primitives/LoadingSkeleton.svelte";
   import Alert from "$lib/ui/primitives/Alert.svelte";
-  import Button from "$lib/ui/primitives/Button.svelte";
+  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 { useSelectionPopover } from "$lib/ui/hooks/useSelectionPopover.svelte";
@@ -578,21 +578,20 @@
     {#if comments.enabled && selectionPopover.pos}
       <!--
         Free-mode Popover anchored above the current selection.
-        `-translate-x-1/2 -translate-y-full` centers the panel above the anchor
-        point; the 8px gap is folded into `y` so the primitive's style stays
-        generic. `selectionPopover.pos` is article-relative.
+        `-translate-x-1/2` centers the icon button horizontally over the anchor;
+        `-translate-y-full` raises it so its bottom edge sits at the anchor, and
+        the 8px gap is folded into `y` so the primitive's style stays generic.
+        `selectionPopover.pos` is article-relative. Visual chrome (shadow, bg,
+        border) lives on the IconButton, so the Popover carries positioning only.
       -->
       <Popover
         open
         strategy="absolute"
         x={selectionPopover.pos.x}
         y={selectionPopover.pos.y - 8}
-        class="
-          -translate-x-1/2 -translate-y-full rounded-lg border border-gray-200 bg-white shadow-lg
-          dark:border-neutral-600 dark:bg-neutral-700
-        "
+        class="-translate-x-1/2 -translate-y-full"
       >
-        <Button variant="ghost" onclick={handleAddComment}>
+        <IconButton aria-label="Add comment" class="shadow-lg" onclick={handleAddComment}>
           <svg
             class="size-4"
             fill="none"
@@ -606,8 +605,7 @@
               d="M7 8h10M7 12h4m1 8l-4-4H5a2 2 0 01-2-2V6a2 2 0 012-2h14a2 2 0 012 2v8a2 2 0 01-2 2h-3l-4 4z"
             />
           </svg>
-          Add comment
-        </Button>
+        </IconButton>
       </Popover>
     {/if}
     <!-- svelte-ignore a11y_no_noninteractive_element_interactions -->
diff --git a/packages/viewer/src/lib/ui/primitives/IconButton.svelte b/packages/viewer/src/lib/ui/primitives/IconButton.svelte
index 77085dab..a825ec6a 100644
--- a/packages/viewer/src/lib/ui/primitives/IconButton.svelte
+++ b/packages/viewer/src/lib/ui/primitives/IconButton.svelte
@@ -15,6 +15,9 @@
   let {
     "aria-label": ariaLabel,
     active = false,
+    // Default to "button": a bare HTML <button> defaults to type="submit" and
+    // would accidentally submit any form it's nested in. Callers can override.
+    type = "button",
     class: extraClass = "",
     children,
     ...rest
@@ -23,6 +26,7 @@
 
 <button
   {...rest}
+  {type}
   class="
     flex size-8 cursor-pointer items-center justify-center rounded-sm border border-border-default
     bg-bg-raised text-fg-muted
diff --git a/packages/viewer/src/lib/ui/primitives/IconButton.test.ts b/packages/viewer/src/lib/ui/primitives/IconButton.test.ts
index b13fbf57..f3f99275 100644
--- a/packages/viewer/src/lib/ui/primitives/IconButton.test.ts
+++ b/packages/viewer/src/lib/ui/primitives/IconButton.test.ts
@@ -34,6 +34,16 @@ describe("IconButton", () => {
     expect(onclick).toHaveBeenCalledTimes(1);
   });
 
+  it("defaults to type=button so it never submits an enclosing form", () => {
+    const { getByRole } = render(Harness, { "aria-label": "X" });
+    expect(getByRole("button").getAttribute("type")).toBe("button");
+  });
+
+  it("lets callers override the button type", () => {
+    const { getByRole } = render(Harness, { "aria-label": "X", type: "submit" });
+    expect(getByRole("button").getAttribute("type")).toBe("submit");
+  });
+
   it("forwards extra attributes (e.g. Popover controlProps)", () => {
     const { getByRole } = render(Harness, {
       "aria-label": "X",