mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 22:02:16 +00:00
fix(insights): unstick scroll on tab-strip and mother-page navigation
`/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.
This commit is contained in:
1 parent
0d591ac928
commit
ac80c099e2
4 files changed
+133
-13
No files matched your search
@@ -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/);
|
||||
});
|
||||
});
|
||||
@@ -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)`
|
||||
|
||||
@@ -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">
|
||||
|
||||
@@ -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 (
|
||||
|
||||
Reference in new issue
Block a user