From cca7a61c4fc5e0d7255a306e8de889ce76e9ec7c Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 9 Jun 2026 15:52:34 -0700 Subject: [PATCH] FN-6142: fix mobile visibility restore scroll anchoring Restore pinned mobile chat scroll position when visibility-driven refetch hooks fire. - re-anchor mobile chat to the bottom during visibility/pageshow refetch capture when the user was pinned - tighten ChatView visibility restore regression tests to assert the synchronous re-anchor path directly - remove the ChatView rooms test quarantine and restore the dashboard include list entry Files changed: packages/dashboard/app/components/ChatView.tsx | 6 +++++- .../app/components/__tests__/ChatView.rooms.test.tsx | 17 ++++++++--------- .../app/components/__tests__/ChatView.test.tsx | 7 ++++--- packages/dashboard/vitest.config.ts | 2 +- scripts/lib/test-quarantine.json | 8 +------- 5 files changed, 19 insertions(+), 21 deletions(-) Fusion-Task-Id: FN-6142 Fusion-Task-Lineage: 2974cf0d-5512-4b24-ab0c-2d2fb63fd41a --- packages/dashboard/app/components/ChatView.tsx | 6 +++++- .../__tests__/ChatView.rooms.test.tsx | 17 ++++++++--------- .../app/components/__tests__/ChatView.test.tsx | 7 ++++--- packages/dashboard/vitest.config.ts | 2 +- scripts/lib/test-quarantine.json | 8 +------- 5 files changed, 19 insertions(+), 21 deletions(-) diff --git a/packages/dashboard/app/components/ChatView.tsx b/packages/dashboard/app/components/ChatView.tsx index 53ef468ecc..b471e771b4 100644 --- a/packages/dashboard/app/components/ChatView.tsx +++ b/packages/dashboard/app/components/ChatView.tsx @@ -1708,7 +1708,11 @@ export function ChatView({ projectId, addToast, experimentalFeatures }: ChatView } const captureForRefetch = () => { + const wasPinnedBefore = !isUserScrollingRef.current; captureScrollSnapshot(); + if (wasPinnedBefore && isMobile && messagesContainerRef.current) { + scrollToBottom("visibility-restore"); + } }; const onVisibilityChange = () => { @@ -1725,7 +1729,7 @@ export function ChatView({ projectId, addToast, experimentalFeatures }: ChatView document.removeEventListener("visibilitychange", onVisibilityChange); window.removeEventListener("pageshow", captureForRefetch); }; - }, [isMobile, activeSession, roomThreadActive, captureScrollSnapshot]); + }, [isMobile, activeSession, roomThreadActive, captureScrollSnapshot, scrollToBottom]); useEffect(() => { if (roomThreadActive) { diff --git a/packages/dashboard/app/components/__tests__/ChatView.rooms.test.tsx b/packages/dashboard/app/components/__tests__/ChatView.rooms.test.tsx index debb0e49cf..bd2a9cf1f7 100644 --- a/packages/dashboard/app/components/__tests__/ChatView.rooms.test.tsx +++ b/packages/dashboard/app/components/__tests__/ChatView.rooms.test.tsx @@ -801,9 +801,10 @@ describe("ChatView — rooms (FN-3805..FN-3811 contract)", () => { Object.defineProperty(document, "visibilityState", { configurable: true, value: "visible" }); fireEvent(document, new Event("visibilitychange")); - await waitFor(() => { - expect(metrics.getScrollTop()).toBe(1180); - }); + // Regression guard: visibility restore must explicitly re-anchor when pinned. + // This previously passed only when an old anchorToBottom rAF happened to run + // after the visibility event and masked that the handler only captured a snapshot. + expect(metrics.getScrollTop()).toBe(1180); } finally { metrics.restore(); restoreMatchMedia.mockRestore(); @@ -826,9 +827,9 @@ describe("ChatView — rooms (FN-3805..FN-3811 contract)", () => { metrics.setScrollTop(300); fireEvent(window, new Event("pageshow")); - await waitFor(() => { - expect(metrics.getScrollTop()).toBe(1180); - }); + // Regression guard: pageshow shares the visibility restore path and must not + // depend on leftover rAF callbacks from the initial mount anchor. + expect(metrics.getScrollTop()).toBe(1180); } finally { metrics.restore(); restoreMatchMedia.mockRestore(); @@ -851,9 +852,7 @@ describe("ChatView — rooms (FN-3805..FN-3811 contract)", () => { Object.defineProperty(document, "visibilityState", { configurable: true, value: "visible" }); fireEvent(document, new Event("visibilitychange")); - await waitFor(() => { - expect(metrics.getScrollTop()).toBe(300); - }); + expect(metrics.getScrollTop()).toBe(300); } finally { metrics.restore(); restoreMatchMedia.mockRestore(); diff --git a/packages/dashboard/app/components/__tests__/ChatView.test.tsx b/packages/dashboard/app/components/__tests__/ChatView.test.tsx index aed6ed1f41..2ac69b93f4 100644 --- a/packages/dashboard/app/components/__tests__/ChatView.test.tsx +++ b/packages/dashboard/app/components/__tests__/ChatView.test.tsx @@ -4577,9 +4577,10 @@ describe("ChatView mobile behavior", () => { Object.defineProperty(document, "visibilityState", { configurable: true, value: "visible" }); fireEvent(document, new Event("visibilitychange")); - await waitFor(() => { - expect(scrollTopValue).toBe(1180); - }); + // Regression guard: visibility restore must explicitly re-anchor when pinned. + // Without that, this only passed when leftover anchorToBottom rAF callbacks + // happened to run after the visibility event. + expect(scrollTopValue).toBe(1180); } finally { restoreMatchMedia.mockRestore(); Object.defineProperty(document, "visibilityState", { configurable: true, value: "visible" }); diff --git a/packages/dashboard/vitest.config.ts b/packages/dashboard/vitest.config.ts index 590de66c4c..676ff87fa5 100644 --- a/packages/dashboard/vitest.config.ts +++ b/packages/dashboard/vitest.config.ts @@ -231,7 +231,7 @@ const qualityAppComponentBatchBTests = buildComponentQualityInclude(batchedQuali const qualityAppAppOnlyTests = ["app/components/__tests__/App.test.tsx"]; const qualityAppChatOnlyTests = ["app/components/__tests__/ChatView.test.tsx"]; const qualityAppSettingsOnlyTests = ["app/components/__tests__/SettingsModal.test.tsx"]; -const quarantinedDashboardTests = ["app/components/__tests__/ChatView.rooms.test.tsx"]; +const quarantinedDashboardTests: string[] = []; const qualityApiTests = [ // Critical HTTP/server behavior: auth, task/project/settings mutation, diff --git a/scripts/lib/test-quarantine.json b/scripts/lib/test-quarantine.json index 89f8774fdc..39eac9c428 100644 --- a/scripts/lib/test-quarantine.json +++ b/scripts/lib/test-quarantine.json @@ -1,10 +1,4 @@ { "$comment": "Flaky-test quarantine ledger (deletion ratchet — see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date — the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). There is deliberately no loader module and no automation around this file: it is a dated record, the vitest config exclude is the mechanism, and the sweep is policy executed by whoever touches the suite.", - "entries": [ - { - "file": "packages/dashboard/app/components/__tests__/ChatView.rooms.test.tsx", - "reason": "FN-6103 workspace pnpm test observed flaky mobile visibility restore assertion: expected scrollTop 1180, received 300 in ChatView.rooms.test.tsx:805; unrelated to CSS-only composer button sizing change.", - "quarantinedAt": "2026-06-09" - } - ] + "entries": [] }