diff --git a/frontend/e2e/channel.spec.ts b/frontend/e2e/channel.spec.ts index 9a4313a..74dd3d7 100644 --- a/frontend/e2e/channel.spec.ts +++ b/frontend/e2e/channel.spec.ts @@ -29,4 +29,30 @@ test.describe("channel view", () => { await expect(page.getByTestId("channel-title")).toHaveCount(0); await expect(page.getByTestId("video-card").first()).toBeVisible(); }); + + // Bug 1 (Platform S4): opening a channel and coming Back restores the feed's scroll position + // rather than dumping you at the top. The shell owns the scroller and remembers each view's offset. + test("Back restores the feed scroll position", async ({ page }) => { + await openApp(page); + await expect(page.getByTestId("video-card").first()).toBeVisible(); + + const main = page.getByRole("main"); + await main.evaluate((el) => el.scrollBy(0, 3000)); + // Let any page-fetch triggered by the scroll settle, so the same content is cached for the + // return trip (a fetch still in flight at navigation would leave a shorter list to restore into). + await page.waitForTimeout(600); + const saved = await main.evaluate((el) => el.scrollTop); + expect(saved).toBeGreaterThan(1500); + + await page.getByTestId("video-card-channel").first().click(); + await expect(page.getByTestId("channel-title")).toBeVisible(); + await page.getByTestId("channel-back").click(); + await expect(page.getByTestId("video-card").first()).toBeVisible(); + + // Restored to roughly where we were, not reset to the top. Tolerance absorbs the virtualizer's + // estimate-vs-measured row heights differing slightly between the two mounts. + await expect + .poll(() => page.getByRole("main").evaluate((el) => el.scrollTop), { timeout: 5000 }) + .toBeGreaterThan(saved - 500); + }); }); diff --git a/frontend/e2e/feed.spec.ts b/frontend/e2e/feed.spec.ts index 95de5ce..724652c 100644 --- a/frontend/e2e/feed.spec.ts +++ b/frontend/e2e/feed.spec.ts @@ -47,4 +47,35 @@ test.describe("feed", () => { .poll(async () => (await renderedIds()).some((id) => id && !before.includes(id))) .toBe(true); }); + + // Bug 4 (Platform S4): the toolbar (show/content chips, count, sort, view) is fixed chrome — it + // pins in the shell's toolbar band above the scroller, so it stays put while the cards scroll. + test("the feed toolbar stays pinned while cards scroll", async ({ page }) => { + await openApp(page); + const count = page.getByTestId("feed-result-count"); + await expect(count).toBeVisible(); + const topBefore = await count.evaluate((el) => el.getBoundingClientRect().top); + + await page.getByRole("main").evaluate((el) => el.scrollBy(0, 4000)); + + await expect(count).toBeInViewport(); + const topAfter = await count.evaluate((el) => el.getBoundingClientRect().top); + expect(Math.abs(topAfter - topBefore)).toBeLessThan(4); + }); + + // Bug 3 (Platform S4): changing a filter is a fresh result set, so the scroll jumps back to the + // top rather than stranding you deep in the old list's offset. + test("a new filter resets the scroll to the top", async ({ page }) => { + await openApp(page); + await expect(page.getByTestId("video-card").first()).toBeVisible(); + + await page.getByRole("main").evaluate((el) => el.scrollBy(0, 4000)); + expect(await page.getByRole("main").evaluate((el) => el.scrollTop)).toBeGreaterThan(500); + + // Switch the show filter → a different result set → back to the top. + await page.getByTestId("feed-show-all").click(); + await expect + .poll(() => page.getByRole("main").evaluate((el) => el.scrollTop)) + .toBeLessThan(50); + }); }); diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index be880ca..8b6b7e0 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -632,6 +632,17 @@ export default function App() {
{t("common.loading")}
); + // Scroll restore/reset key for the shared normal-page scroller (see PageScroller): the same + // page/feed-filters returns you to where you were (bug 1); a changed feed filter is a new key, so + // the fresh result set starts at the top (bug 3). `q` is normalised out so typing in the search + // box doesn't reset the scroll on every keystroke; the live-YouTube sub-view gets its own key. + const normalScrollKey = + page !== "feed" + ? `page:${page}` + : ytSearch != null + ? `yt:${ytSearch}` + : `feed:${JSON.stringify({ ...filters, q: "" })}`; + return (
{channelView ? ( - + ) : ( (); + +// Put the scroller back to `top`. The feed is virtualized, so on a remount the list needs a frame +// or two to render enough rows to reach its full height; an immediate `scrollTop` would otherwise +// clamp short. Re-apply over a few frames until it sticks (or the content is simply shorter now). +function restoreScroll(el: HTMLElement, top: number): () => void { + if (top <= 0) { + el.scrollTop = 0; + return () => {}; + } + let raf = 0; + let frames = 0; + let cancelled = false; + const tick = () => { + if (cancelled) return; + if (el.scrollHeight - el.clientHeight >= top) el.scrollTop = top; + if (el.scrollTop >= top - 1 || frames++ > 90) return; // reached, or gave up after ~1.5s + raf = requestAnimationFrame(tick); + }; + raf = requestAnimationFrame(tick); + return () => { + cancelled = true; + cancelAnimationFrame(raf); + }; +} + +// The page's scroll container — the ONE scroller in the app, owned by PageShell. The feed, the +// channel page and Plex browse all scroll inside it rather than owning a scroller of their own, +// which is why the edge-fade and the scroll restore/reset policy live here once instead of three +// times. // // Two invariants this element carries, both learned the hard way: // @@ -19,13 +48,61 @@ import { useScrollFade } from "../lib/useScrollFade"; // the gutter and re-open that loop. Unlike the rail and the side panels, this scroller wears the // fade next to its scrollbar, not instead of it. // +// Scroll restore/reset policy (`scrollKey` identifies the logical view): +// - returning to a view with the SAME key — feed → open a channel → Back, feed filters unchanged — +// restores where you were (bug 1). The channel page is a different scroller, so the feed's +// scroller unmounts and remounts; the saved offset survives in `savedScroll` and is re-applied. +// - a NEW key — the feed's filters changed, i.e. a fresh result set — starts at the top (bug 3). +// The offset is recorded on every scroll rather than at unmount: by the time an unmount cleanup +// runs the scroller's scrollTop has already been zeroed by the teardown, so a cleanup-time read +// saves 0 and the restore is lost. Recording live sidesteps that (and StrictMode's synthetic +// unmount, which would otherwise persist a 0 too). +// // The fade state lives in here rather than in App so that a flip re-renders this component alone — // `children` arrives as an already-built element, so React skips the page subtree underneath. -export default function PageScroller({ children }: { children: ReactNode }) { +export default function PageScroller({ + children, + scrollKey, +}: { + children: ReactNode; + scrollKey?: string; +}) { const fade = useScrollFade(); + const fadeRef = fade.ref; + const elRef = useRef(null); + // Stable identity: an inline ref callback changes every render, which makes React detach and + // re-attach the ref each commit — and since fade.ref is a state setter, that re-attach loops it + // into "Maximum update depth exceeded". useScrollFade's ref setter is itself stable, so binding + // once here is safe. + const setRef = useCallback( + (node: HTMLElement | null) => { + elRef.current = node; + fadeRef(node); + }, + [fadeRef], + ); + + useLayoutEffect(() => { + const el = elRef.current; + if (!el || scrollKey == null) return; + const key = scrollKey; + const want = savedScroll.get(key) ?? 0; + const cancelRestore = restoreScroll(el, want); + // Record position continuously (not at unmount): the scroller's scrollTop is unreliable by the + // time an unmount cleanup runs, and a save-on-scroll survives whatever tears down last. + const onScroll = () => { + savedScroll.set(key, el.scrollTop); + }; + el.addEventListener("scroll", onScroll, { passive: true }); + return () => { + cancelRestore(); + el.removeEventListener("scroll", onScroll); + }; + }, [scrollKey]); + return (
diff --git a/frontend/src/components/PageShell.tsx b/frontend/src/components/PageShell.tsx index a526cc6..1ff0b2f 100644 --- a/frontend/src/components/PageShell.tsx +++ b/frontend/src/components/PageShell.tsx @@ -38,10 +38,13 @@ export default function PageShell({ header, banners, children, + scrollKey, }: { header?: ReactNode; banners?: ReactNode; children: ReactNode; + // Identifies the logical view for the scroll restore/reset policy — see PageScroller. + scrollKey?: string; }) { const [toolbarSlot, setToolbarSlot] = useState(null); return ( @@ -50,7 +53,7 @@ export default function PageShell({
{banners}
- {children} + {children}
);