diff --git a/packages/viewer/e2e/comments.spec.ts b/packages/viewer/e2e/comments.spec.ts index d06ce054..5d0f058c 100644 --- a/packages/viewer/e2e/comments.spec.ts +++ b/packages/viewer/e2e/comments.spec.ts @@ -148,6 +148,34 @@ test.describe("Inline comments", () => { await expect(commentButton).toBeVisible(); }); + test("popover follows the highlighted text on scroll", async ({ page }) => { + // Shrink the viewport so the homepage overflows and the window scrolls. + await page.setViewportSize({ width: 1400, height: 400 }); + await page.goto("/"); + await page.getByRole("article").waitFor(); + + await selectText(page, "Navigation sidebar"); + + const commentButton = page.getByRole("button", { name: "Add comment" }); + await expect(commentButton).toBeVisible(); + + const before = await commentButton.boundingBox(); + expect(before).not.toBeNull(); + + const scrollDelta = 50; + await page.evaluate((y) => window.scrollBy(0, y), scrollDelta); + + await expect + .poll(async () => (await commentButton.boundingBox())?.y, { timeout: 1000 }) + .not.toBe(before!.y); + + const after = await commentButton.boundingBox(); + expect(after).not.toBeNull(); + // Popover sits in viewport-fixed coords, so a downward scroll of N pushes + // the highlighted text — and therefore the popover — up by N. + expect(after!.y).toBeCloseTo(before!.y - scrollDelta, 0); + }); + test("clicking the selected text dismisses the comment popover", async ({ page }) => { await page.goto("/"); await page.getByRole("article").waitFor(); diff --git a/packages/viewer/src/components/PageContent.svelte b/packages/viewer/src/components/PageContent.svelte index 07d5f3d6..ee9cbc99 100644 --- a/packages/viewer/src/components/PageContent.svelte +++ b/packages/viewer/src/components/PageContent.svelte @@ -8,6 +8,7 @@ import Button from "$lib/ui/primitives/Button.svelte"; import Popover from "$lib/ui/primitives/Popover.svelte"; import { useElementSize } from "$lib/ui/hooks/useElementSize.svelte"; + import { useAnchorOffset } from "$lib/ui/hooks/useAnchorOffset.svelte"; import PageComments from "./comments/PageComments.svelte"; const ctx = getRwContext(); @@ -18,8 +19,10 @@ const articleSize = useElementSize(() => articleRef ?? null); - // Comment selection state - let selectionRect: { x: number; y: number } | null = $state(null); + // The popover renders in `position: fixed` coords, so its anchor must + // re-measure on scroll — hold the Range, not a one-shot rect. + let selectionRange: Range | null = $state.raw(null); + const selectionRect = useAnchorOffset(() => selectionRange); // Dismiss the popover when the selection collapses (e.g. user clicks on the // selected text). Blink runs the click-on-selection collapse as a default @@ -27,15 +30,24 @@ // still returns the active range — handleMouseUp would re-pin the popover // at the same coords and only the highlight would disappear. $effect(() => { - if (!selectionRect) return; + if (!selectionRange) return; const handler = () => { const sel = window.getSelection(); - if (!sel || sel.isCollapsed) selectionRect = null; + if (!sel || sel.isCollapsed) selectionRange = null; }; document.addEventListener("selectionchange", handler); return () => document.removeEventListener("selectionchange", handler); }); + // Drop any in-flight selection when the article content changes (live reload, + // navigation). The cached Range's start/end nodes get detached, so + // getBoundingClientRect() returns zeros and would briefly jump the popover to + // the top-left corner before anything else dismissed it. + $effect(() => { + void page.data; + selectionRange = null; + }); + // Delay skeleton appearance so fast page loads don't flash it. const SKELETON_DELAY_MS = 300; $effect(() => { @@ -232,7 +244,7 @@ // If no text selected, check if click landed on a comment highlight if (!selection || selection.isCollapsed) { - selectionRect = null; + selectionRange = null; // Toggle: click an inactive highlight to activate, click the active one to dismiss. const hitId = findCommentAtPoint(event); @@ -242,12 +254,13 @@ const range = selection.getRangeAt(0); if (!articleRef || !articleRef.contains(range.commonAncestorContainer)) { - selectionRect = null; + selectionRange = null; return; } - const rect = range.getBoundingClientRect(); - selectionRect = { x: rect.left + rect.width / 2, y: rect.top }; + // `Selection.getRangeAt(0)` returns a live Range that mutates when the + // user re-selects; clone so the captured range stays put. + selectionRange = range.cloneRange(); } function handleMouseMove(event: MouseEvent) { @@ -335,12 +348,12 @@ comments.pending = { documentId: docId, selectors }; comments.activeId = null; - selectionRect = null; + selectionRange = null; window.getSelection()?.removeAllRanges(); } -{#if comments.enabled && selectionRect} +{#if comments.enabled && selectionRect.measured} import { useAnchorOffset } from "../useAnchorOffset.svelte"; interface Props { - el: HTMLElement | null; + el: Element | Range | null; } let { el }: Props = $props(); diff --git a/packages/viewer/src/lib/ui/hooks/__fixtures__/resize-observer-mock.ts b/packages/viewer/src/lib/ui/hooks/__fixtures__/resize-observer-mock.ts index da1d7da9..0368c233 100644 --- a/packages/viewer/src/lib/ui/hooks/__fixtures__/resize-observer-mock.ts +++ b/packages/viewer/src/lib/ui/hooks/__fixtures__/resize-observer-mock.ts @@ -32,11 +32,8 @@ export class MockResizeObserver { } } -// Not redefining getBoundingClientRect on the prototype keeps other tests -// (and any unrelated rects read from the document root) untouched. -export function makeAnchor(rect: Partial): HTMLElement { - const el = document.createElement("div"); - const get = () => ({ +export function fakeRect(rect: Partial): DOMRect { + return { top: 0, left: 0, width: 0, @@ -47,7 +44,21 @@ export function makeAnchor(rect: Partial): HTMLElement { y: 0, toJSON: () => ({}), ...rect, - }); - el.getBoundingClientRect = () => get() as DOMRect; + } as DOMRect; +} + +// Not redefining getBoundingClientRect on the prototype keeps other tests +// (and any unrelated rects read from the document root) untouched. +export function makeAnchor(rect: Partial | (() => Partial)): HTMLElement { + const get = typeof rect === "function" ? rect : () => rect; + const el = document.createElement("div"); + el.getBoundingClientRect = () => fakeRect(get()); return el; } + +export function makeRange(rect: Partial | (() => Partial)): Range { + const get = typeof rect === "function" ? rect : () => rect; + const range = document.createRange(); + range.getBoundingClientRect = () => fakeRect(get()); + return range; +} diff --git a/packages/viewer/src/lib/ui/hooks/observeElement.svelte.ts b/packages/viewer/src/lib/ui/hooks/observeElement.svelte.ts deleted file mode 100644 index b1c541b3..00000000 --- a/packages/viewer/src/lib/ui/hooks/observeElement.svelte.ts +++ /dev/null @@ -1,41 +0,0 @@ -/** - * Run `onMeasure(el)` once on subscription and again whenever the element - * resizes. Re-subscribes automatically when the getter starts returning a - * different element, and tears down the observer (and any window listeners) - * on cleanup. - * - * `trackWindow`: also re-measure on window scroll (capture phase, so ancestor - * scroll containers count) and window resize. Use this for hooks that report - * viewport-relative coordinates; skip it for size-only hooks, since size is - * scroll-invariant and listening would only produce redundant work. - * - * Implementation detail of `useElementSize` / `useAnchorOffset` — not exported - * outside the hooks/ layer. - */ -export function observeElement( - getEl: () => HTMLElement | null, - onMeasure: (el: HTMLElement) => void, - { trackWindow = false }: { trackWindow?: boolean } = {}, -): void { - $effect(() => { - const el = getEl(); - if (!el) return; - - const measure = () => onMeasure(el); - - measure(); - const observer = new ResizeObserver(measure); - observer.observe(el); - if (trackWindow) { - window.addEventListener("scroll", measure, { capture: true, passive: true }); - window.addEventListener("resize", measure); - } - return () => { - observer.disconnect(); - if (trackWindow) { - window.removeEventListener("scroll", measure, { capture: true }); - window.removeEventListener("resize", measure); - } - }; - }); -} diff --git a/packages/viewer/src/lib/ui/hooks/observeTarget.svelte.ts b/packages/viewer/src/lib/ui/hooks/observeTarget.svelte.ts new file mode 100644 index 00000000..c2e6eba9 --- /dev/null +++ b/packages/viewer/src/lib/ui/hooks/observeTarget.svelte.ts @@ -0,0 +1,55 @@ +/** + * Run `onMeasure(target)` once on subscription and again whenever the target + * resizes. Re-subscribes automatically when the getter starts returning a + * different target, and tears down the observer (and any window listeners) + * on cleanup. + * + * `target` can be an `Element` (observed via `ResizeObserver`) or a `Range` + * (no resize observer — Ranges aren't observable, so re-measurement relies + * solely on `trackWindow`). + * + * `trackWindow`: also re-measure on window scroll (capture phase, so ancestor + * scroll containers count) and window resize. Use this for hooks that report + * viewport-relative coordinates; skip it for size-only hooks, since size is + * scroll-invariant and listening would only produce redundant work. + * + * `onLost`: called whenever the getter returns `null`, so consumers can roll + * any "anchor disappeared" state in without spawning a second `$effect` that + * re-subscribes to the same getter. + * + * Implementation detail of `useElementSize` / `useAnchorOffset` — not + * exported outside the hooks/ layer. + */ +export function observeTarget( + getTarget: () => T | null, + onMeasure: (target: T) => void, + { trackWindow = false, onLost }: { trackWindow?: boolean; onLost?: () => void } = {}, +): void { + $effect(() => { + const target = getTarget(); + if (!target) { + onLost?.(); + return; + } + + const measure = () => onMeasure(target); + + measure(); + let observer: ResizeObserver | null = null; + if (target instanceof Element) { + observer = new ResizeObserver(measure); + observer.observe(target); + } + if (trackWindow) { + window.addEventListener("scroll", measure, { capture: true, passive: true }); + window.addEventListener("resize", measure); + } + return () => { + observer?.disconnect(); + if (trackWindow) { + window.removeEventListener("scroll", measure, { capture: true }); + window.removeEventListener("resize", measure); + } + }; + }); +} diff --git a/packages/viewer/src/lib/ui/hooks/useAnchorOffset.svelte.ts b/packages/viewer/src/lib/ui/hooks/useAnchorOffset.svelte.ts index 98697150..956754e5 100644 --- a/packages/viewer/src/lib/ui/hooks/useAnchorOffset.svelte.ts +++ b/packages/viewer/src/lib/ui/hooks/useAnchorOffset.svelte.ts @@ -1,18 +1,20 @@ -import { observeElement } from "./observeElement.svelte"; +import { observeTarget } from "./observeTarget.svelte"; /** - * Track the viewport-relative bounding rect of an element across resizes and - * ancestor scrolls. + * Track the viewport-relative bounding rect of a target across resizes and + * ancestor scrolls. The target may be an `Element` (observed via + * `ResizeObserver` for size changes) or a `Range` (re-measured only on + * window scroll/resize, since Ranges aren't observable). * * Returns a reactive object whose `top`/`left`/`width`/`height` values update - * whenever the element resizes, the window resizes, or any scroll container + * whenever the target 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. + * an external anchor or a selected text passage. * * 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 - * the getter returns a different element. + * the getter returns a different target. * * Scroll tracking uses a capture-phase listener on `window`, which catches * scrolls from any ancestor scroll container without walking the DOM — this @@ -25,28 +27,36 @@ export interface AnchorOffset { readonly width: number; readonly height: number; /** - * False until the first `getBoundingClientRect` read completes. Consumers - * use this to suppress a panel's initial paint at (0,0) while waiting for - * the anchor's coordinates — the classic Radix/Floating UI "positioning" - * state. True in all later states, including after the anchor changes. + * False until the first `getBoundingClientRect` read completes, and again + * whenever the target getter returns `null`. Consumers use this to suppress + * a panel's initial paint at (0,0) while waiting for coordinates — the + * classic Radix/Floating UI "positioning" state — and to hide the panel + * once the anchor disappears. */ readonly measured: boolean; } -export function useAnchorOffset(getEl: () => HTMLElement | null): AnchorOffset { +export function useAnchorOffset( + getTarget: () => T | null, +): AnchorOffset { const rect = $state({ top: 0, left: 0, width: 0, height: 0, measured: false }); - observeElement( - getEl, - (el) => { - const r = el.getBoundingClientRect(); + observeTarget( + getTarget, + (target) => { + const r = target.getBoundingClientRect(); rect.top = r.top; rect.left = r.left; rect.width = r.width; rect.height = r.height; rect.measured = true; }, - { trackWindow: true }, + { + trackWindow: true, + onLost: () => { + rect.measured = false; + }, + }, ); return { diff --git a/packages/viewer/src/lib/ui/hooks/useAnchorOffset.test.svelte.ts b/packages/viewer/src/lib/ui/hooks/useAnchorOffset.test.svelte.ts index 709ba7e0..c28dfe6f 100644 --- a/packages/viewer/src/lib/ui/hooks/useAnchorOffset.test.svelte.ts +++ b/packages/viewer/src/lib/ui/hooks/useAnchorOffset.test.svelte.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; 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 { MockResizeObserver, makeAnchor, makeRange } from "./__fixtures__/resize-observer-mock"; describe("useAnchorOffset", () => { beforeEach(() => { @@ -15,9 +15,9 @@ describe("useAnchorOffset", () => { }); it("populates the rect fields from getBoundingClientRect on mount", () => { - const el = makeAnchor({ top: 10, left: 20, width: 100, height: 50 }); + const anchor = makeAnchor({ top: 10, left: 20, width: 100, height: 50 }); - const { getByTestId } = render(Harness, { el }); + const { getByTestId } = render(Harness, { el: anchor }); const out = getByTestId("anchor-offset"); expect(out.dataset.top).toBe("10"); @@ -26,7 +26,7 @@ describe("useAnchorOffset", () => { expect(out.dataset.height).toBe("50"); }); - it("leaves the rect fields at zero when the element is null", () => { + it("leaves the rect fields at zero when the anchor is null", () => { const { getByTestId } = render(Harness, { el: null }); const out = getByTestId("anchor-offset"); @@ -39,34 +39,44 @@ describe("useAnchorOffset", () => { }); it("flips measured to true after the initial synchronous measurement", () => { - const el = makeAnchor({ top: 10, left: 20, width: 100, height: 50 }); + const anchor = makeAnchor({ top: 10, left: 20, width: 100, height: 50 }); - const { getByTestId } = render(Harness, { el }); + const { getByTestId } = render(Harness, { el: anchor }); const out = getByTestId("anchor-offset"); expect(out.dataset.measured).toBe("true"); }); - it("keeps measured=true when the element switches to a new one", async () => { - const elA = makeAnchor({ top: 10, width: 100, height: 20 }); - const elB = makeAnchor({ top: 500, width: 30, height: 40 }); + it("flips measured back to false when the anchor becomes null after measure", async () => { + const anchor = makeAnchor({ top: 10, left: 20, width: 100, height: 50 }); - const { getByTestId, rerender } = render(Harness, { el: elA }); + const { getByTestId, rerender } = render(Harness, { el: anchor }); const out = getByTestId("anchor-offset"); expect(out.dataset.measured).toBe("true"); - await rerender({ el: elB }); + await rerender({ el: null }); + + expect(out.dataset.measured).toBe("false"); + }); + + it("keeps measured=true when the anchor switches to a new one", async () => { + const anchorA = makeAnchor({ top: 10, width: 100, height: 20 }); + const anchorB = makeAnchor({ top: 500, width: 30, height: 40 }); + + const { getByTestId, rerender } = render(Harness, { el: anchorA }); + const out = getByTestId("anchor-offset"); + expect(out.dataset.measured).toBe("true"); + + await rerender({ el: anchorB }); expect(out.dataset.measured).toBe("true"); }); it("updates the rect fields when ResizeObserver fires", () => { - let currentRect = { top: 0, left: 0, width: 100, height: 50 }; - const el = document.createElement("div"); - el.getBoundingClientRect = () => - ({ ...currentRect, right: 0, bottom: 0, x: 0, y: 0, toJSON: () => ({}) }) as DOMRect; + let currentRect: Partial = { top: 0, left: 0, width: 100, height: 50 }; + const anchor = makeAnchor(() => currentRect); - const { getByTestId } = render(Harness, { el }); + const { getByTestId } = render(Harness, { el: anchor }); const out = getByTestId("anchor-offset"); expect(out.dataset.top).toBe("0"); expect(out.dataset.width).toBe("100"); @@ -82,9 +92,9 @@ describe("useAnchorOffset", () => { }); it("disconnects the observer on unmount", () => { - const el = makeAnchor({ width: 10, height: 10 }); + const anchor = makeAnchor({ width: 10, height: 10 }); - const { unmount } = render(Harness, { el }); + const { unmount } = render(Harness, { el: anchor }); const observer = MockResizeObserver.instances[0]; expect(observer.disconnected).toBe(false); @@ -93,12 +103,10 @@ describe("useAnchorOffset", () => { }); it("updates the rect fields when window scrolls", () => { - 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; + let currentRect: Partial = { top: 100, left: 50, width: 100, height: 50 }; + const anchor = makeAnchor(() => currentRect); - const { getByTestId } = render(Harness, { el }); + const { getByTestId } = render(Harness, { el: anchor }); const out = getByTestId("anchor-offset"); expect(out.dataset.top).toBe("100"); @@ -110,16 +118,14 @@ describe("useAnchorOffset", () => { }); it("updates the rect fields when an ancestor scroll-container scrolls (capture phase)", () => { - let currentRect = { top: 100, left: 50, width: 100, height: 50 }; + let currentRect: Partial = { top: 100, left: 50, width: 100, height: 50 }; const scroller = document.createElement("div"); - const el = document.createElement("div"); - scroller.appendChild(el); + const anchor = makeAnchor(() => currentRect); + scroller.appendChild(anchor); document.body.appendChild(scroller); - el.getBoundingClientRect = () => - ({ ...currentRect, right: 0, bottom: 0, x: 0, y: 0, toJSON: () => ({}) }) as DOMRect; try { - const { getByTestId } = render(Harness, { el }); + const { getByTestId } = render(Harness, { el: anchor }); const out = getByTestId("anchor-offset"); expect(out.dataset.top).toBe("100"); @@ -134,12 +140,10 @@ describe("useAnchorOffset", () => { }); it("updates the rect fields when the window resizes", () => { - let currentRect = { top: 10, left: 10, width: 100, height: 50 }; - const el = document.createElement("div"); - el.getBoundingClientRect = () => - ({ ...currentRect, right: 0, bottom: 0, x: 0, y: 0, toJSON: () => ({}) }) as DOMRect; + let currentRect: Partial = { top: 10, left: 10, width: 100, height: 50 }; + const anchor = makeAnchor(() => currentRect); - const { getByTestId } = render(Harness, { el }); + const { getByTestId } = render(Harness, { el: anchor }); const out = getByTestId("anchor-offset"); expect(out.dataset.left).toBe("10"); @@ -151,11 +155,11 @@ describe("useAnchorOffset", () => { }); it("removes window listeners on unmount", () => { - const el = makeAnchor({ top: 10, left: 10, width: 100, height: 50 }); + const anchor = makeAnchor({ top: 10, left: 10, width: 100, height: 50 }); const addSpy = vi.spyOn(window, "addEventListener"); const removeSpy = vi.spyOn(window, "removeEventListener"); - const { unmount } = render(Harness, { el }); + const { unmount } = render(Harness, { el: anchor }); const added = addSpy.mock.calls .filter(([type]) => type === "scroll" || type === "resize") .map(([type]) => type); @@ -173,22 +177,50 @@ describe("useAnchorOffset", () => { removeSpy.mockRestore(); }); - it("re-subscribes when the element prop changes", async () => { - const elA = makeAnchor({ top: 10, width: 100, height: 20 }); - const elB = makeAnchor({ top: 500, width: 30, height: 40 }); + it("re-subscribes when the anchor prop changes", async () => { + const anchorA = makeAnchor({ top: 10, width: 100, height: 20 }); + const anchorB = makeAnchor({ top: 500, width: 30, height: 40 }); - const { getByTestId, rerender } = render(Harness, { el: elA }); + const { getByTestId, rerender } = render(Harness, { el: anchorA }); const out = getByTestId("anchor-offset"); expect(out.dataset.top).toBe("10"); expect(out.dataset.width).toBe("100"); expect(MockResizeObserver.instances).toHaveLength(1); const firstObserver = MockResizeObserver.instances[0]; - await rerender({ el: elB }); + await rerender({ el: anchorB }); expect(firstObserver.disconnected).toBe(true); expect(MockResizeObserver.instances).toHaveLength(2); expect(out.dataset.top).toBe("500"); expect(out.dataset.width).toBe("30"); }); + + describe("with a Range anchor", () => { + it("populates the rect fields without subscribing to ResizeObserver", () => { + const anchor = makeRange({ top: 10, left: 20, width: 100, height: 50 }); + + const { getByTestId } = render(Harness, { el: anchor }); + const out = getByTestId("anchor-offset"); + + expect(out.dataset.top).toBe("10"); + expect(out.dataset.measured).toBe("true"); + expect(MockResizeObserver.instances).toHaveLength(0); + }); + + it("re-measures when window scrolls", () => { + let current: Partial = { top: 100, left: 50, width: 100, height: 50 }; + const anchor = makeRange(() => current); + + const { getByTestId } = render(Harness, { el: anchor }); + const out = getByTestId("anchor-offset"); + expect(out.dataset.top).toBe("100"); + + current = { top: 20, left: 50, width: 100, height: 50 }; + window.dispatchEvent(new Event("scroll")); + flushSync(); + + expect(out.dataset.top).toBe("20"); + }); + }); }); diff --git a/packages/viewer/src/lib/ui/hooks/useElementSize.svelte.ts b/packages/viewer/src/lib/ui/hooks/useElementSize.svelte.ts index ff9da788..52df7066 100644 --- a/packages/viewer/src/lib/ui/hooks/useElementSize.svelte.ts +++ b/packages/viewer/src/lib/ui/hooks/useElementSize.svelte.ts @@ -1,4 +1,4 @@ -import { observeElement } from "./observeElement.svelte"; +import { observeTarget } from "./observeTarget.svelte"; /** * Track an element's content-box size across resizes. @@ -34,7 +34,7 @@ export function useElementSize(getEl: () => HTMLElement | null): ElementSize { // self-dependency that re-fires the effect on its own write. let counter = 0; - observeElement(getEl, (el) => { + observeTarget(getEl, (el) => { const r = el.getBoundingClientRect(); size.width = r.width; size.height = r.height; diff --git a/packages/viewer/src/lib/ui/hooks/useElementSize.test.svelte.ts b/packages/viewer/src/lib/ui/hooks/useElementSize.test.svelte.ts index 42269ee2..f7a1cffc 100644 --- a/packages/viewer/src/lib/ui/hooks/useElementSize.test.svelte.ts +++ b/packages/viewer/src/lib/ui/hooks/useElementSize.test.svelte.ts @@ -36,19 +36,8 @@ describe("useElementSize", () => { }); it("updates width/height when ResizeObserver fires", () => { - let currentRect = { width: 100, height: 50 }; - const el = document.createElement("div"); - el.getBoundingClientRect = () => - ({ - ...currentRect, - top: 0, - left: 0, - right: 0, - bottom: 0, - x: 0, - y: 0, - toJSON: () => ({}), - }) as DOMRect; + let currentRect: Partial = { width: 100, height: 50 }; + const el = makeAnchor(() => currentRect); const { getByTestId } = render(Harness, { el }); const out = getByTestId("element-size"); diff --git a/packages/viewer/src/lib/ui/primitives/Popover.test.svelte.ts b/packages/viewer/src/lib/ui/primitives/Popover.test.svelte.ts index 225b8654..fb554c70 100644 --- a/packages/viewer/src/lib/ui/primitives/Popover.test.svelte.ts +++ b/packages/viewer/src/lib/ui/primitives/Popover.test.svelte.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { flushSync } from "svelte"; import { render, fireEvent } from "@testing-library/svelte"; import Harness from "./__fixtures__/PopoverHarness.svelte"; -import { MockResizeObserver, mockRect } from "./__fixtures__/overlay-testing"; +import { MockResizeObserver, fakeRect, mockRect } from "./__fixtures__/overlay-testing"; // The panel is the immediate parent of the // tested content — getByTestId returns the span, so stepping up one level @@ -206,7 +206,7 @@ describe("Popover", () => { const original = HTMLElement.prototype.getBoundingClientRect; HTMLElement.prototype.getBoundingClientRect = function () { if (this.tagName === "SPAN" && this.classList.contains("inline-block")) { - return { + return fakeRect({ top: 10, left: 20, width: 50, @@ -215,8 +215,7 @@ describe("Popover", () => { bottom: 24, x: 20, y: 10, - toJSON: () => ({}), - } as DOMRect; + }); } return original.call(this); }; @@ -497,7 +496,7 @@ describe("Popover", () => { const original = HTMLElement.prototype.getBoundingClientRect; HTMLElement.prototype.getBoundingClientRect = function () { if (this.tagName === "SPAN" && this.classList.contains("inline-block")) { - return { + return fakeRect({ top: 10, left: 20, width: 50, @@ -506,8 +505,7 @@ describe("Popover", () => { bottom: 24, x: 20, y: 10, - toJSON: () => ({}), - } as DOMRect; + }); } return original.call(this); }; diff --git a/packages/viewer/src/lib/ui/primitives/__fixtures__/overlay-testing.ts b/packages/viewer/src/lib/ui/primitives/__fixtures__/overlay-testing.ts index fbbd7571..df9f7820 100644 --- a/packages/viewer/src/lib/ui/primitives/__fixtures__/overlay-testing.ts +++ b/packages/viewer/src/lib/ui/primitives/__fixtures__/overlay-testing.ts @@ -3,22 +3,12 @@ // on via useAnchorOffset — re-export the richer mock from the hooks fixture. // Primitives tests don't need `instances`/`trigger()`, but a single shared // implementation keeps the global stub consistent across suites. -export { MockResizeObserver } from "../../hooks/__fixtures__/resize-observer-mock"; +import { fakeRect } from "../../hooks/__fixtures__/resize-observer-mock"; + +export { MockResizeObserver, fakeRect } from "../../hooks/__fixtures__/resize-observer-mock"; export function mockRect(el: HTMLElement, rect: Partial): void { - el.getBoundingClientRect = () => - ({ - top: 0, - left: 0, - width: 0, - height: 0, - right: 0, - bottom: 0, - x: 0, - y: 0, - toJSON: () => ({}), - ...rect, - }) as DOMRect; + el.getBoundingClientRect = () => fakeRect(rect); } export function createAnchor(): HTMLButtonElement {