From ac80c099e22b2721e47f1cfb184f3ad179d0e427 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= <mbombeck@gmail.com>
Date: Sat, 16 May 2026 00:31:05 +0200
Subject: [PATCH] fix(insights): unstick scroll on tab-strip and mother-page
 navigation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`/insights` landed at the wrong vertical position when the user
returned from a sub-page, and vertical swipes that began on the
sticky `<InsightsTabStrip>` were eaten by the strip's
`overflow-x-auto` scroll container so the page "felt stuck"
mid-gesture. Two roots, two fixes:

1. Move the horizontal pill scroll from the outer `<nav>` onto the
   inner pill-row `<div>` (the inner row already declared
   `overflow-x-auto`; the outer one is redundant once it's gone).
   Add `touch-action: pan-y` to the outer `<nav>` so vertical
   swipes pass through.

2. Defer the `<SubPageShell>` scroll-reset to the next animation
   frame so the sub-page's first paint settles before the scroll
   lands. Mirror the same deferred reset on the `/insights` mother
   page so a back-navigation lands cleanly at the top instead of
   inheriting the sub-page's vertical position through cached
   scroll state.

A Playwright spec at `e2e/insights-scroll-restoration.spec.ts`
locks both regressions: (a) /insights → /insights/<sub-page> → back
lands within 50 px of the top, (b) the outer `<nav>` declares
`touch-action: pan-y` and no longer owns the horizontal scroll.

iOS contract: none — both surfaces are web-only.
---
 e2e/insights-scroll-restoration.spec.ts       | 90 +++++++++++++++++++
 src/app/insights/page.tsx                     | 16 ++++
 .../insights/insights-tab-strip.tsx           | 12 ++-
 src/components/insights/sub-page-shell.tsx    | 28 +++---
 4 files changed, 133 insertions(+), 13 deletions(-)
 create mode 100644 e2e/insights-scroll-restoration.spec.ts

diff --git a/e2e/insights-scroll-restoration.spec.ts b/e2e/insights-scroll-restoration.spec.ts
new file mode 100644
index 000000000..9010c7399
--- /dev/null
+++ b/e2e/insights-scroll-restoration.spec.ts
@@ -0,0 +1,90 @@
+import { expect, test } from "@playwright/test";
+
+import { STORAGE_STATE_PATH } from "./setup/global-setup";
+
+/**
+ * v1.4.28 R3a FB-D3 — `/insights` scroll restoration + tab-strip
+ * gesture handling.
+ *
+ * Two regressions the maintainer hit walking through v1.4.27:
+ *
+ *   (a) Returning from a sub-page to `/insights` landed the viewport
+ *       partway down the page because the mother page had no scroll
+ *       reset on mount and the sub-page's `SubPageShell` had already
+ *       reset it for the sub-page only.
+ *
+ *   (b) Vertical swipes that started on the sticky `<InsightsTabStrip>`
+ *       were eaten by the strip's `overflow-x-auto` scroll container,
+ *       so the page "felt stuck" until the gesture lifted.
+ *
+ * Both fixes land in the same commit. This spec is the regression
+ * lock — failure here means a future refactor re-introduced one of
+ * the two roots.
+ */
+test.describe("v1.4.28 — insights scroll restoration", () => {
+  test.use({ storageState: STORAGE_STATE_PATH });
+
+  test("returning to /insights from a sub-page lands at the top", async ({
+    page,
+  }) => {
+    await page.goto("/insights", { waitUntil: "domcontentloaded" });
+    // Force the mother page tall enough that scrolling has somewhere
+    // to go even on a sparse seed; the rendered Insights overview
+    // already runs ~1500 px tall on the demo seed.
+    await page.evaluate(() => window.scrollTo({ top: 600, behavior: "auto" }));
+
+    // Click the first available sub-page pill.
+    const subPagePill = page
+      .locator("[data-slot='insights-tab-strip-pill']")
+      .nth(1);
+    await expect(subPagePill).toBeVisible();
+    await subPagePill.click();
+    await page.waitForURL(/\/insights\//);
+
+    // Back to the mother page via the overview pill (pill index 0).
+    await page
+      .locator("[data-slot='insights-tab-strip-pill']")
+      .first()
+      .click();
+    await page.waitForURL(/\/insights\/?$/);
+
+    // The mother page's mount-effect deferred a `scrollTo(0)` to the
+    // next animation frame. Wait a tick for it to land.
+    await page.waitForTimeout(50);
+    const y = await page.evaluate(() => window.scrollY);
+    expect(y).toBeLessThan(50);
+  });
+
+  test("the sticky tab strip declares touch-action pan-y so vertical swipes pass through", async ({
+    page,
+  }) => {
+    await page.goto("/insights", { waitUntil: "domcontentloaded" });
+    const nav = page.locator("[data-slot='insights-tab-strip']");
+    await expect(nav).toBeVisible();
+    const touchAction = await nav.evaluate(
+      (el) => (el as HTMLElement).style.touchAction || getComputedStyle(el).touchAction,
+    );
+    expect(touchAction).toContain("pan-y");
+  });
+
+  test("the tab strip's horizontal scroll lives on an inner row, not on the outer nav", async ({
+    page,
+  }) => {
+    await page.goto("/insights", { waitUntil: "domcontentloaded" });
+    const nav = page.locator("[data-slot='insights-tab-strip']");
+    const navOverflowX = await nav.evaluate(
+      (el) => getComputedStyle(el).overflowX,
+    );
+    // The outer `<nav>` no longer scrolls horizontally — the inner row
+    // owns that responsibility now so vertical swipes outside the row
+    // reach the document.
+    expect(navOverflowX).not.toBe("auto");
+    expect(navOverflowX).not.toBe("scroll");
+
+    const innerScroller = nav.locator("> div > div").first();
+    const innerOverflowX = await innerScroller.evaluate(
+      (el) => getComputedStyle(el).overflowX,
+    );
+    expect(innerOverflowX).toMatch(/auto|scroll/);
+  });
+});
diff --git a/src/app/insights/page.tsx b/src/app/insights/page.tsx
index e563ddbe6..c2f96b5ff 100644
--- a/src/app/insights/page.tsx
+++ b/src/app/insights/page.tsx
@@ -1,5 +1,6 @@
 "use client";
 
+import { useEffect } from "react";
 import { useQuery } from "@tanstack/react-query";
 import Link from "next/link";
 import { Loader2, TrendingUp } from "lucide-react";
@@ -95,6 +96,21 @@ export default function InsightsPage() {
   const { isAuthenticated, user } = useAuth();
   const { t } = useTranslations();
 
+  // v1.4.28 FB-D3 — mirror `<SubPageShell>`'s deferred scroll-reset on
+  // the mother page so a back-navigation from a sub-page (where
+  // `SubPageShell` already reset the scroll on mount) lands cleanly
+  // at the top of the overview instead of inheriting the sub-page's
+  // vertical position through the cached scroll state. Single
+  // mount-only effect; uses `requestAnimationFrame` so the reset
+  // settles after first paint, same as the sub-page shell.
+  useEffect(() => {
+    if (typeof window === "undefined") return;
+    const handle = window.requestAnimationFrame(() => {
+      window.scrollTo({ top: 0, behavior: "auto" });
+    });
+    return () => window.cancelAnimationFrame(handle);
+  }, []);
+
   // v1.4.27 R3d MB4 — Coach drawer state lives in the layout-level
   // `<CoachLaunchProvider>` so every routed sub-page can reach it.
   // The hero strip + suggested-prompt chips call `askCoach(prefill)`
diff --git a/src/components/insights/insights-tab-strip.tsx b/src/components/insights/insights-tab-strip.tsx
index f9be8acfb..6d94a6366 100644
--- a/src/components/insights/insights-tab-strip.tsx
+++ b/src/components/insights/insights-tab-strip.tsx
@@ -136,6 +136,15 @@ export function InsightsTabStrip({
     <nav
       data-slot="insights-tab-strip"
       aria-label={t("insights.navAriaLabel")}
+      // v1.4.28 FB-D3 — `touch-action: pan-y` lets vertical swipes that
+      // begin on the sticky strip scroll the page through it. The
+      // legacy `overflow-x-auto` on the outer `<nav>` claimed the
+      // entire touch surface as a horizontal scroll container, so
+      // vertical drags that started on the pill row never reached the
+      // document. The horizontal pill scroll now lives on the inner
+      // `<div>` below; the outer `<nav>` stays sticky but no longer
+      // owns the gesture.
+      style={{ touchAction: "pan-y" }}
       className={cn(
         // v1.4.27 MB7 / CF-72 — relative wrapper hosts the right-edge
         // fade overlay below so the user reads "there's more to
@@ -144,8 +153,7 @@ export function InsightsTabStrip({
         // strip via a sibling div; it sits only on `<sm` because the
         // wider viewport above already shows every pill.
         "relative",
-        "bg-background/95 sticky top-0 z-30 overflow-x-auto border-b py-2 backdrop-blur",
-        "[scrollbar-width:none] [&::-webkit-scrollbar]:hidden",
+        "bg-background/95 sticky top-0 z-30 border-b py-2 backdrop-blur",
       )}
     >
       <div className="flex items-center gap-2">
diff --git a/src/components/insights/sub-page-shell.tsx b/src/components/insights/sub-page-shell.tsx
index 41dc1fc58..e4856dd29 100644
--- a/src/components/insights/sub-page-shell.tsx
+++ b/src/components/insights/sub-page-shell.tsx
@@ -60,20 +60,26 @@ export function SubPageShell({
 }: SubPageShellProps) {
   // a11y: focus the heading on mount so a tab-strip navigation actually
   // moves screen-reader focus into the sub-page body. v1.4.27 MB7 /
-  // CF-35 made the call opt-in via `focusOnMount` — the default-on
-  // version was stealing focus from soft-keyboards on mobile and
-  // breaking mid-flow form entries. The `scrollTo` still fires
-  // unconditionally; both honour `prefers-reduced-motion: reduce`
-  // implicitly because `scrollTo({ behavior: "auto" })` does not
-  // animate, and the focus call is a single discrete event.
+  // CF-35 made the focus call opt-in via `focusOnMount` — the
+  // default-on version stole focus from soft-keyboards on mobile.
+  //
+  // v1.4.28 FB-D3 — defer the `scrollTo` to the next animation frame
+  // so the sub-page's first paint settles before the scroll lands.
+  // The legacy unconditional in-effect scroll fired before the chart
+  // skeleton had a height; the viewport snapped to 0, then the chart
+  // popped in and the scroll position visibly jumped again. Wrapping
+  // in `requestAnimationFrame` runs the scroll after the layout pass
+  // so the user sees one smooth landing.
   const headingRef = useRef<HTMLHeadingElement | null>(null);
   useEffect(() => {
-    if (typeof window !== "undefined") {
+    if (typeof window === "undefined") return;
+    const handle = window.requestAnimationFrame(() => {
       window.scrollTo({ top: 0, behavior: "auto" });
-    }
-    if (focusOnMount) {
-      headingRef.current?.focus({ preventScroll: true });
-    }
+      if (focusOnMount) {
+        headingRef.current?.focus({ preventScroll: true });
+      }
+    });
+    return () => window.cancelAnimationFrame(handle);
   }, [focusOnMount]);
 
   return (