diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 7f7ccce4f..9ceca2d7e 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -52,6 +52,7 @@ export default defineConfig({ "**/reminders-screenshots.spec.ts", "**/virtualization-screenshots.spec.ts", "**/scroll-history.spec.ts", + "**/stick-to-bottom-scroll-jump.spec.ts", ], use: { ...devices["Desktop Chrome"], diff --git a/desktop/src/shared/hooks/useStickToBottom.ts b/desktop/src/shared/hooks/useStickToBottom.ts index 09fedbdea..d9df005e2 100644 --- a/desktop/src/shared/hooks/useStickToBottom.ts +++ b/desktop/src/shared/hooks/useStickToBottom.ts @@ -11,16 +11,40 @@ import { useCallback, useEffect, useRef } from "react"; * * Scroll calls are batched via `requestAnimationFrame` so rapid streaming * updates (e.g. token-by-token SSE) don't cause layout thrashing. + * + * `onScroll` is direction-aware: it only unsticks when the user moves + * the viewport *upward*. Intermediate scroll events emitted by the + * browser during the smooth scroll-to-bottom animation see scrollTop + * climbing toward the new bottom, so they leave the sticky bit alone. + * Without that guard, a smooth-scroll mid-animation would flip + * `isNearBottom` to false (the visible scrollTop is still well above + * scrollHeight - clientHeight - 100), and the next content update + * would refuse to follow — the user-visible symptom is the feed + * appearing to "jump back a couple lines" as new content pushes the + * old bottom out of view. */ export function useStickToBottom() { const ref = useRef(null); const isNearBottomRef = useRef(true); + const lastScrollTopRef = useRef(0); const onScroll = useCallback(() => { const el = ref.current; if (!el) return; const { scrollTop, scrollHeight, clientHeight } = el; - isNearBottomRef.current = scrollHeight - scrollTop - clientHeight < 100; + const distance = scrollHeight - scrollTop - clientHeight; + if (distance < 100) { + // At (or very near) the bottom — always sticky, regardless of + // direction. Resticks the container once a smooth-scroll + // animation settles, and handles the initial state. + isNearBottomRef.current = true; + } else if (scrollTop < lastScrollTopRef.current) { + // User pulled the viewport upward. Detach. + isNearBottomRef.current = false; + } + // Otherwise: scrollTop is climbing toward the bottom (smooth-scroll + // in flight) or unchanged. Leave the sticky bit as-is. + lastScrollTopRef.current = scrollTop; }, []); useEffect(() => { @@ -29,6 +53,7 @@ export function useStickToBottom() { // Start at the bottom; the observer below only reacts to later changes. el.scrollTop = el.scrollHeight; + lastScrollTopRef.current = el.scrollTop; let rafId: number | null = null; diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 37de50c5e..c0cedb087 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -644,6 +644,25 @@ declare global { __BUZZ_E2E_QUERY_CLIENT__?: { invalidateQueries: (filters: { queryKey: readonly unknown[] }) => unknown; }; + // Lazy-installed by the stick-to-bottom-scroll-jump spec via + // __BUZZ_E2E_INSTALL_STICK_FIXTURE__; declared here so the rest of + // the bridge typechecks. + __BUZZ_E2E_INSTALL_STICK_FIXTURE__?: () => Promise; + __BUZZ_E2E_MOUNT_STICK_FIXTURE__?: (options?: { seedItems?: number }) => { + push: (text?: string) => number; + scrollToBottom: () => void; + scrollToTop: () => void; + simulateScroll: (scrollTop: number) => void; + state: () => { + scrollTop: number; + scrollHeight: number; + clientHeight: number; + isNearBottom: boolean; + distanceFromBottom: number; + itemCount: number; + }; + unmount: () => void; + }; } } @@ -6899,5 +6918,14 @@ export function maybeInstallE2eTauriMocks() { handleMockCommand(command, payload ?? null); mockIPC(handleMockCommand); + // Lazy-installer for the stick-to-bottom test fixture. The fixture + // pulls in `react-dom/client` and the production hook, so we only + // import it on demand from the spec that needs it — avoids bloating + // the main e2e bundle for every other test. + window.__BUZZ_E2E_INSTALL_STICK_FIXTURE__ = async () => { + const mod = await import("./stickToBottomFixture"); + mod.installStickFixtureBridge(); + }; + installed = true; } diff --git a/desktop/src/testing/stickToBottomFixture.tsx b/desktop/src/testing/stickToBottomFixture.tsx new file mode 100644 index 000000000..7c1b89c0a --- /dev/null +++ b/desktop/src/testing/stickToBottomFixture.tsx @@ -0,0 +1,216 @@ +import * as React from "react"; +import { createRoot, type Root } from "react-dom/client"; + +import { useStickToBottom } from "@/shared/hooks/useStickToBottom"; + +/** + * E2E test fixture: mounts a scroll container that uses the real + * `useStickToBottom` hook so a Playwright spec can drive it through the + * exact code path the activity feed uses. Lives under `src/testing/` so + * the production bundle ignores it unless the E2E bridge boots. + */ + +const HOST_ID = "buzz-e2e-stick-fixture"; + +type FixtureHandle = { + push: (text?: string) => number; + scrollToBottom: () => void; + scrollToTop: () => void; + /** + * Manually set scrollTop and dispatch a `scroll` event so the + * React-attached `onScroll` handler observes it. Lets specs + * deterministically simulate a mid-animation intermediate scroll + * event without depending on Chromium's `behavior: "smooth"` + * timing in a tiny test container. + */ + simulateScroll: (scrollTop: number) => void; + state: () => { + scrollTop: number; + scrollHeight: number; + clientHeight: number; + isNearBottom: boolean; + distanceFromBottom: number; + itemCount: number; + }; + unmount: () => void; +}; + +declare global { + interface Window { + __BUZZ_E2E_MOUNT_STICK_FIXTURE__?: (options?: { + seedItems?: number; + }) => FixtureHandle; + } +} + +function StickToBottomFixture({ + initialItems, + registerHandle, +}: { + initialItems: number; + registerHandle: (handle: Omit) => void; +}) { + const [items, setItems] = React.useState(() => + Array.from({ length: initialItems }, (_, i) => `seed-${i}`), + ); + const { ref, onScroll, isNearBottomRef } = useStickToBottom(); + const seqRef = React.useRef(initialItems); + + React.useEffect(() => { + registerHandle({ + push: (text?: string) => { + seqRef.current += 1; + const label = text ?? `item-${seqRef.current}`; + setItems((prev) => [...prev, label]); + return seqRef.current; + }, + scrollToBottom: () => { + const el = ref.current; + if (!el) return; + el.scrollTop = el.scrollHeight; + // Manually fire onScroll so the hook updates its internal state + // — synthetic scrollTop changes don't dispatch a scroll event. + onScroll(); + }, + scrollToTop: () => { + const el = ref.current; + if (!el) return; + el.scrollTop = 0; + onScroll(); + }, + simulateScroll: (scrollTop: number) => { + const el = ref.current; + if (!el) return; + el.scrollTop = scrollTop; + // Dispatch a real scroll event so React's onScroll fires. + el.dispatchEvent(new Event("scroll", { bubbles: true })); + }, + state: () => { + const el = ref.current; + if (!el) { + return { + scrollTop: 0, + scrollHeight: 0, + clientHeight: 0, + isNearBottom: isNearBottomRef.current, + distanceFromBottom: 0, + itemCount: 0, + }; + } + return { + scrollTop: el.scrollTop, + scrollHeight: el.scrollHeight, + clientHeight: el.clientHeight, + isNearBottom: isNearBottomRef.current, + distanceFromBottom: el.scrollHeight - el.scrollTop - el.clientHeight, + itemCount: el.querySelectorAll("[data-stick-item]").length, + }; + }, + }); + }, [isNearBottomRef, onScroll, ref, registerHandle]); + + return ( +
+ {items.map((item) => ( +
+ {item} +
+ ))} +
+ ); +} + +let activeRoot: Root | null = null; +let activeHost: HTMLDivElement | null = null; + +export function installStickFixtureBridge() { + if (window.__BUZZ_E2E_MOUNT_STICK_FIXTURE__) return; + + window.__BUZZ_E2E_MOUNT_STICK_FIXTURE__ = (options) => { + if (activeRoot) { + activeRoot.unmount(); + activeRoot = null; + } + if (activeHost) { + activeHost.remove(); + activeHost = null; + } + + const host = document.createElement("div"); + host.id = HOST_ID; + host.style.position = "fixed"; + host.style.top = "0"; + host.style.left = "0"; + host.style.zIndex = "2147483647"; + host.style.background = "white"; + host.style.color = "black"; + document.body.appendChild(host); + + const root = createRoot(host); + activeRoot = root; + activeHost = host; + + let resolveHandle: (handle: Omit) => void; + const handlePromise = new Promise>( + (resolve) => { + resolveHandle = resolve; + }, + ); + + root.render( + resolveHandle(handle)} + />, + ); + + // The bridge contract is synchronous — but we need React to have + // rendered and registered the handle before the caller proceeds. + // Spin the microtask queue once; if the handle isn't ready, fall + // back to a stub that retries each call. In practice React 19's + // createRoot + initial render registers within a microtask, so the + // promise is usually already resolved by the time the user reads + // the returned methods via await page.evaluateHandle. + let registered: Omit | null = null; + void handlePromise.then((h) => { + registered = h; + }); + + const ensure = () => { + if (!registered) { + throw new Error( + "stick-to-bottom fixture not yet mounted — await an animation frame first", + ); + } + return registered; + }; + + return { + push: (text) => ensure().push(text), + scrollToBottom: () => ensure().scrollToBottom(), + scrollToTop: () => ensure().scrollToTop(), + simulateScroll: (top) => ensure().simulateScroll(top), + state: () => ensure().state(), + unmount: () => { + root.unmount(); + host.remove(); + activeRoot = null; + activeHost = null; + delete (window as Window).__BUZZ_E2E_MOUNT_STICK_FIXTURE__; + }, + }; + }; +} diff --git a/desktop/tests/e2e/stick-to-bottom-scroll-jump.spec.ts b/desktop/tests/e2e/stick-to-bottom-scroll-jump.spec.ts new file mode 100644 index 000000000..b8c16ba37 --- /dev/null +++ b/desktop/tests/e2e/stick-to-bottom-scroll-jump.spec.ts @@ -0,0 +1,210 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge } from "../helpers/bridge"; + +/** + * Regression test for the agent activity / observer feed "jump back a + * couple lines" bug. + * + * The hook `useStickToBottom` (used by `AgentSessionThreadPanel`) + * scrolls smoothly to the bottom when new content arrives while the + * user is already near the bottom. A browser `behavior: "smooth"` + * scroll emits intermediate `scroll` events whose `scrollTop` is + * mid-animation — well above `scrollHeight - clientHeight - 100`. The + * old `onScroll` recomputed `isNearBottom` from raw `scrollTop`, so + * those intermediate events flipped it to `false` mid-flight. The next + * content append refused to follow the bottom and the feed appeared + * to "jump back a couple lines." + * + * The fix makes `onScroll` direction-aware: distance < 100 always + * sticks; otherwise, only `scrollTop` decreasing (user pulling up) + * unsticks. Intermediate animation events climb toward the bottom, so + * they leave the sticky bit alone. + * + * The two specs below pin both halves of that contract: + * 1. A mid-animation intermediate scroll event does not unstick. + * 2. A genuine user scroll-up still unsticks (no over-correction). + */ + +type StickFixtureState = { + scrollTop: number; + scrollHeight: number; + clientHeight: number; + isNearBottom: boolean; + distanceFromBottom: number; + itemCount: number; +}; + +type StickFixtureWindow = Window & { + __BUZZ_E2E_STICK_FIXTURE__: { + push: (text?: string) => number; + scrollToBottom: () => void; + scrollToTop: () => void; + simulateScroll: (scrollTop: number) => void; + state: () => StickFixtureState; + }; +}; + +async function mountFixture( + page: import("@playwright/test").Page, + seedItems: number, +): Promise { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_INSTALL_STICK_FIXTURE__ === "function", + ); + await page.evaluate(async () => { + const install = window.__BUZZ_E2E_INSTALL_STICK_FIXTURE__; + if (!install) throw new Error("install hook missing"); + await install(); + }); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_MOUNT_STICK_FIXTURE__ === "function", + ); + await page.evaluate((seeds) => { + const mount = window.__BUZZ_E2E_MOUNT_STICK_FIXTURE__; + if (!mount) throw new Error("mount hook missing"); + const handle = mount({ seedItems: seeds }); + (window as unknown as StickFixtureWindow).__BUZZ_E2E_STICK_FIXTURE__ = + handle; + }, seedItems); + // React 19's createRoot registers the handle in a microtask; poll + // until state() doesn't throw. + await page.waitForFunction(() => { + try { + ( + window as unknown as StickFixtureWindow + ).__BUZZ_E2E_STICK_FIXTURE__?.state(); + return true; + } catch { + return false; + } + }); +} + +async function readState( + page: import("@playwright/test").Page, +): Promise { + return page.evaluate(() => + ( + window as unknown as StickFixtureWindow + ).__BUZZ_E2E_STICK_FIXTURE__.state(), + ); +} + +test("intermediate smooth-scroll events do not unstick the activity feed", async ({ + page, +}) => { + // Seed enough items that the scroll range is much larger than the + // 100 px "near bottom" threshold, leaving room to put scrollTop at + // a mid-animation value with distance > 100. + await mountFixture(page, 60); + + const initial = await readState(page); + expect(initial.isNearBottom).toBe(true); + expect(initial.distanceFromBottom).toBeLessThan(5); + // The initial scroll-to-bottom in the hook's useEffect uses a raw + // assignment, so onScroll doesn't fire — lastScrollTopRef is seeded + // by that effect (or by a synthetic event below). To make the + // direction comparison meaningful, fire a baseline onScroll at the + // current bottom so lastScrollTopRef reflects the resting scrollTop. + await page.evaluate(() => { + ( + window as unknown as StickFixtureWindow + ).__BUZZ_E2E_STICK_FIXTURE__.simulateScroll( + ( + window as unknown as StickFixtureWindow + ).__BUZZ_E2E_STICK_FIXTURE__.state().scrollTop, + ); + }); + + // Now simulate the exact race the bug reporter sees: a smooth-scroll + // is animating *toward* the new bottom (scrollTop climbing upward in + // absolute terms), but content was appended faster than the + // animation, so distance from bottom is still > 100. The + // intermediate scroll event must NOT unstick the feed. + // + // The deterministic way to fake this without depending on + // Chromium's smooth-scroll timing: grow the scroll range by pushing + // a burst, then set scrollTop to a value that is higher than the + // pre-burst max but still > 100 px from the new bottom, and fire + // a scroll event. + await page.evaluate(() => { + const f = (window as unknown as StickFixtureWindow) + .__BUZZ_E2E_STICK_FIXTURE__; + // Add enough items to push the bottom well past the current + // scrollTop. + for (let i = 0; i < 30; i += 1) { + f.push(`burst-${i}`); + } + }); + // Read the new layout BEFORE the rAF scroll-to-bottom completes. + // (The mutation observer schedules a smooth-scroll, but we + // intercept by firing a synthetic intermediate scroll event + // ourselves.) + const beforeIntermediate = await page.evaluate(() => { + const f = (window as unknown as StickFixtureWindow) + .__BUZZ_E2E_STICK_FIXTURE__; + return f.state(); + }); + // Pick a scrollTop that is climbing toward the new bottom but still + // > 100 px away from it. + const intermediateScrollTop = + beforeIntermediate.scrollHeight - beforeIntermediate.clientHeight - 250; + await page.evaluate((top) => { + ( + window as unknown as StickFixtureWindow + ).__BUZZ_E2E_STICK_FIXTURE__.simulateScroll(top); + }, intermediateScrollTop); + + const midAnimation = await readState(page); + expect(midAnimation.distanceFromBottom).toBeGreaterThan(100); + // The actual regression assertion: an intermediate scroll event + // *toward* the bottom does not unstick the feed. Pre-fix this is + // `false`; post-fix it stays `true`. + expect(midAnimation.isNearBottom).toBe(true); + + // Confirm the end-to-end contract: the next content append still + // schedules a scroll-to-bottom because isNearBottom is sticky. + await page.evaluate(() => { + const f = (window as unknown as StickFixtureWindow) + .__BUZZ_E2E_STICK_FIXTURE__; + f.push("after-mid-anim-1"); + f.push("after-mid-anim-2"); + }); + await page.waitForTimeout(800); + const settled = await readState(page); + expect(settled.itemCount).toBe(60 + 30 + 2); + expect(settled.distanceFromBottom).toBeLessThan(5); + expect(settled.isNearBottom).toBe(true); +}); + +test("user scrolling up still unsticks the activity feed", async ({ page }) => { + // Direction-awareness guard: the fix must not over-correct and pin + // the container even when the user genuinely pulls the view up. + await mountFixture(page, 60); + + await page.evaluate(() => { + ( + window as unknown as StickFixtureWindow + ).__BUZZ_E2E_STICK_FIXTURE__.scrollToTop(); + }); + + const afterScrollUp = await readState(page); + expect(afterScrollUp.isNearBottom).toBe(false); + expect(afterScrollUp.distanceFromBottom).toBeGreaterThan(100); + + // Pushing more items should NOT yank scrollTop back to the bottom + // — sticky is off because the user moved away. + await page.evaluate(() => { + const f = (window as unknown as StickFixtureWindow) + .__BUZZ_E2E_STICK_FIXTURE__; + for (let i = 0; i < 5; i += 1) f.push(); + }); + await page.waitForTimeout(400); + + const afterPushes = await readState(page); + expect(afterPushes.isNearBottom).toBe(false); + expect(afterPushes.distanceFromBottom).toBeGreaterThan(100); +});