Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
20 changes: 9 additions & 11 deletions packages/viewer/src/components/PageContent.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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"
Expand All @@ -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 -->
Expand Down
4 changes: 4 additions & 0 deletions packages/viewer/src/lib/ui/primitives/IconButton.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 10 additions & 0 deletions packages/viewer/src/lib/ui/primitives/IconButton.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading