From 91d36b3357af566aab595e50b3d26c89306753eb Mon Sep 17 00:00:00 2001 From: npeter83 Date: Mon, 26 Oct 2026 00:19:02 +0100 Subject: [PATCH] fix(channel): the channel page's address names the channel and its own view, not the page under it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening a channel kept every key of the page under it (`?q=music&sort=oldest&chan=…`, a sort the channel did not show), and the channel's own tab, sort and content types lived in its state alone, so a reload or a bookmark lost them. Now the channel's address is `channelUrl`: the shell's keys and `chan`, shared by `openChannel` and the channel link's href. The page writes its view (`chtab`, `chsort`, `chtypes`, lib/channelPageUrlState) and reopens it; these are shell keys, cleared by a page switch and by closing it in place. While a channel stands over a page, no module store writes its keys (`urlPage`); Back finds them on the page's own entry. A link written by an older build opens without the page's keys. --- .../NavigationProvider.dom.test.tsx | 53 +++++++ .../navigation/NavigationProvider.tsx | 34 ++++- .../navigation/channelOverPage.dom.test.tsx | 100 ++++++++++++++ frontend/src/features/navigation/urlPage.ts | 9 ++ .../video/ChannelNameLink.dom.test.tsx | 18 ++- .../src/features/video/ChannelNameLink.tsx | 8 +- frontend/src/lib/channelPageUrlState.test.ts | 69 ++++++++++ frontend/src/lib/channelPageUrlState.ts | 57 ++++++++ frontend/src/lib/urlState.ts | 21 ++- frontend/src/moduleUrl.contract.test.ts | 19 ++- .../src/modules/channels/ChannelsProvider.tsx | 9 +- frontend/src/modules/feed/ChannelPage.tsx | 42 +++++- .../modules/feed/ChannelPage.url.dom.test.tsx | 130 ++++++++++++++++++ .../src/modules/feed/FeedFiltersProvider.tsx | 13 +- frontend/src/modules/files/FilesProvider.tsx | 9 +- .../modules/playlists/PlaylistsProvider.tsx | 11 +- frontend/src/modules/plex/PlexProvider.tsx | 9 +- 17 files changed, 565 insertions(+), 46 deletions(-) create mode 100644 frontend/src/features/navigation/channelOverPage.dom.test.tsx create mode 100644 frontend/src/features/navigation/urlPage.ts create mode 100644 frontend/src/lib/channelPageUrlState.test.ts create mode 100644 frontend/src/lib/channelPageUrlState.ts create mode 100644 frontend/src/modules/feed/ChannelPage.url.dom.test.tsx diff --git a/frontend/src/features/navigation/NavigationProvider.dom.test.tsx b/frontend/src/features/navigation/NavigationProvider.dom.test.tsx index d21893eb..aa74bceb 100644 --- a/frontend/src/features/navigation/NavigationProvider.dom.test.tsx +++ b/frontend/src/features/navigation/NavigationProvider.dom.test.tsx @@ -27,6 +27,7 @@ vi.mock("react-i18next", async (importOriginal) => { import { NavigationProvider } from "./NavigationProvider"; import { useNavigation, useNavigationActions } from "./navigationContext"; +import { urlPage } from "./urlPage"; import { useBackToClose } from "../../lib/history"; import { PageHeading } from "../../components/ui/Headings"; @@ -225,6 +226,58 @@ describe("NavigationProvider · a link carrying another page's keys", () => { expect(window.location.search).toBe("?page=stats&chan=UC1"); expect(result.current.channelView?.id).toBe("UC1"); }); + + it("opens a channel link with only the channel page's keys, even over the page they belong to", () => { + window.history.replaceState({}, "", "/?q=music&sort=oldest&chan=UC1&chtab=about&chtypes=ns"); + render(); + expect(window.location.search).toBe("?chan=UC1&chtab=about&chtypes=ns"); + }); +}); + +// The channel page has an address of its own (the owner, 2026-10-05): it used to take the feed's +// filters along (`?q=music&sort=oldest&chan=…`), naming a sort the channel did not show. +describe("NavigationProvider · the channel page's address", () => { + it("names only the channel; the page under it keeps its own address for Back", async () => { + window.history.replaceState({ sfPage: "feed" }, "", "/?q=music&sort=oldest"); + const { result } = render(); + act(() => result.current.openChannel("UC1", "One")); + expect(window.location.search).toBe("?chan=UC1"); + act(() => window.history.back()); + await vi.waitFor(() => expect(result.current.channelView).toBeNull()); + expect(window.location.search).toBe("?q=music&sort=oldest"); + }); + + it("says no page owns the address while the channel stands over it", () => { + window.history.replaceState({ sfPage: "plex" }, "", "/?page=plex"); + const { result } = render(); + expect(result.current.page).toBe("plex"); + expect(urlPage(result.current)).toBe("plex"); + act(() => result.current.openChannel("UC1", "One")); + expect(urlPage(result.current)).toBeNull(); + }); + + it("another channel opened from it starts fresh", () => { + const { result } = render(); + act(() => result.current.openChannel("UC1", "One")); + window.history.replaceState(window.history.state, "", "/?chan=UC1&chtab=about&chsort=oldest"); + act(() => result.current.openChannel("UC2", "Two")); + expect(window.location.search).toBe("?chan=UC2"); + }); + + it("a page switch takes the channel page's keys with it", () => { + window.history.replaceState({}, "", "/?chan=UC1&chtab=about&chsort=oldest&chtypes=n"); + const { result } = render(); + act(() => result.current.setPage("stats")); + expect(window.location.search).toBe("?page=stats"); + }); + + it("closing it in place takes its keys with it", () => { + window.history.replaceState({}, "", "/?chan=UC1&chtab=about&chtypes=n"); + const { result } = render(); + stubNavigation([false]); + act(() => result.current.closeChannel()); + expect(window.location.search).toBe(""); + }); }); describe("NavigationProvider · Back", () => { diff --git a/frontend/src/features/navigation/NavigationProvider.tsx b/frontend/src/features/navigation/NavigationProvider.tsx index a09de092..e9200276 100644 --- a/frontend/src/features/navigation/NavigationProvider.tsx +++ b/frontend/src/features/navigation/NavigationProvider.tsx @@ -2,7 +2,13 @@ import { useCallback, useEffect, useMemo, useState, type ReactNode } from "react import { useLatest } from "../../lib/useLatest"; import { useTranslation } from "react-i18next"; import { useMe } from "../../lib/useMe"; -import { resolvePage, retiredParams, urlWithParams, type Page } from "../../lib/urlState"; +import { + channelUrl, + resolvePage, + retiredParams, + urlWithParams, + type Page, +} from "../../lib/urlState"; import { useDocumentObjectTitle } from "../../lib/documentTitle"; import { getAccountRaw, setAccountRaw } from "../../lib/storage"; import { @@ -13,6 +19,7 @@ import { rememberNavPosition, } from "../../lib/history"; import { foreignUrlKeys, moduleUrlKeys, pageTitleKey, reachablePage } from "./moduleRegistry"; +import { CHANNEL_PAGE_KEYS } from "../../lib/channelPageUrlState"; import { channelViewFrom, loadInitialPage, @@ -41,11 +48,20 @@ import { function pageRootParams(page: Page): Record { return { page: page === "feed" ? null : page, - chan: null, - ...Object.fromEntries(moduleUrlKeys().map((k) => [k, null])), + ...closedChannelParams(), + ...nulls(moduleUrlKeys()), }; } +const nulls = (keys: readonly string[]) => Object.fromEntries(keys.map((k) => [k, null])); + +/** No channel page: neither `chan` nor its view's keys, which the page itself writes (ChannelPage) — + * a closed channel leaves none behind. (An opened one gets its address from `channelUrl`.) */ +const closedChannelParams = (): Record => ({ + chan: null, + ...nulls(CHANNEL_PAGE_KEYS), +}); + export function NavigationProvider({ children }: { children: ReactNode }) { const { t, i18n } = useTranslation(); // Held here as well as in Settings: this is the instance mounted on every boot, so a value only @@ -128,7 +144,9 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // inside one. const openChannel = useCallback((id: string, name?: string) => { closeOverlaysThenNavigate(() => { - const url = urlWithParams({ chan: id }); + // The channel's address names the channel alone: the page under it keeps its own keys on its + // own entry, where Back finds them, and writes none while the channel stands over it (urlPage). + const url = channelUrl(id); if (window.history.state?._chan) { const st = window.history.state; window.history.replaceState({ ...st, _chan: id, _chanName: name ?? null }, "", url); @@ -148,7 +166,7 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // entry's channel marks and the ?chan param. setChannelView(null); const { _chan: _id, _chanName: _name, ...st } = window.history.state ?? {}; - window.history.replaceState(st, "", urlWithParams({ chan: null })); + window.history.replaceState(st, "", urlWithParams(closedChannelParams())); } }, []); @@ -234,9 +252,11 @@ export function NavigationProvider({ children }: { children: ReactNode }) { const linked = new URLSearchParams(window.location.search).get("page"); const remembered = getAccountRaw(PAGE_KEY); // A link can carry another page's keys (hand-edited, or written by an older build): the page - // opens without them, so its address names only what it shows. + // opens without them, so its address names only what it shows — and a channel page open over it + // shows none of the page's own (an older build's channel links carried the feed's filters). + const shown = channelViewFrom(st, window.location.search); const url = urlWithParams({ - ...Object.fromEntries(foreignUrlKeys(mountPage).map((k) => [k, null])), + ...nulls(shown ? moduleUrlKeys() : foreignUrlKeys(mountPage)), ...retiredParams(linked ?? (resolvePage(remembered) === mountPage ? remembered : null)), page: mountPage === "feed" ? null : mountPage, }); diff --git a/frontend/src/features/navigation/channelOverPage.dom.test.tsx b/frontend/src/features/navigation/channelOverPage.dom.test.tsx new file mode 100644 index 00000000..7b76ea1a --- /dev/null +++ b/frontend/src/features/navigation/channelOverPage.dom.test.tsx @@ -0,0 +1,100 @@ +// @vitest-environment jsdom +// +// The channel page opens over any page, and its address names the channel alone (the owner, 2026-10-05: +// it used to carry the feed's filters, naming a sort the channel did not show). The page under it is +// still mounted — its store lives at the root — and a change in it while the channel stands over it +// (a background refetch, the header search) must not write its keys onto the channel's address. Closing +// the channel brings them back. The real provider and the real stores; only the network is absent. +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, cleanup, render, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { useEffect } from "react"; + +vi.mock("react-i18next", async (importOriginal) => ({ + ...(await importOriginal()), + useTranslation: () => ({ t: (k: string) => k, i18n: { language: "en" } }), +})); + +import { NavigationProvider } from "./NavigationProvider"; +import { ModuleStores } from "./ModuleStores"; +import { useNavigationActions, type NavigationActions } from "./navigationContext"; +import { useFeedFilters, useFeedFiltersActions } from "../../modules/feed"; +import { usePlexActions } from "../../modules/plex"; +import { useChannelsActions } from "../../modules/channels"; +import { useFilesActions } from "../../modules/files"; +import { usePlaylistsActions } from "../../modules/playlists"; +import { claimLocalData } from "../../lib/storage"; + +type Probe = { + nav: NavigationActions; + change: Record void>; +}; +const probe: { current: Probe | null } = { current: null }; + +function Probe() { + const nav = useNavigationActions(); + const filters = useFeedFilters(); + const { setFilters } = useFeedFiltersActions(); + const plex = usePlexActions(); + const channels = useChannelsActions(); + const files = useFilesActions(); + const playlists = usePlaylistsActions(); + // Handed out after each render (an effect, not the render itself, may write outside it). + useEffect(() => { + probe.current = { + nav, + change: { + feed: () => setFilters({ ...filters, sort: "oldest" }), + plex: () => plex.setScope("movie"), + channels: () => channels.setView("hidden"), + files: () => files.setView({ kind: "folder", id: 3 }), + playlists: () => playlists.setSelectedId(5), + }, + }; + }); + return null; +} + +// What each page's change above writes, once the page is what the address shows. +const WRITTEN: Record = { + feed: ["sort", "oldest"], + plex: ["pscope", "movie"], + channels: ["ctab", "hidden"], + files: ["folder", "3"], + playlists: ["pl", "5"], +}; + +beforeEach(() => { + localStorage.clear(); + claimLocalData(3); +}); +afterEach(() => { + cleanup(); + window.history.replaceState(null, "", "/"); +}); + +const search = () => new URLSearchParams(window.location.search); + +describe.each(Object.keys(WRITTEN))("a channel page over %s", (page) => { + it("keeps the page's keys off its address, and they come back when it closes", async () => { + // The page named even for the feed: a bare address opens on Home (the account's default). + window.history.replaceState({ sfPage: page }, "", `/?page=${page}`); + render( + + + + + + + , + ); + act(() => probe.current!.nav.openChannel("UC1", "One")); + act(() => probe.current!.change[page]!()); + const [key, value] = WRITTEN[page]!; + expect(search().get(key)).toBeNull(); + expect(search().get("chan")).toBe("UC1"); + act(() => probe.current!.nav.closeChannel()); + await waitFor(() => expect(search().get("chan")).toBeNull()); + await waitFor(() => expect(search().get(key)).toBe(value)); + }); +}); diff --git a/frontend/src/features/navigation/urlPage.ts b/frontend/src/features/navigation/urlPage.ts new file mode 100644 index 00000000..b63ffbb8 --- /dev/null +++ b/frontend/src/features/navigation/urlPage.ts @@ -0,0 +1,9 @@ +import type { Page } from "../../lib/urlState"; +import type { NavigationState } from "./navigationContext"; + +/** The page whose own keys the address bar carries: none while the channel page stands over it, so + * every module store writes its keys (`useUrlMirror`) only while its page is what the screen shows. + * A plain function beside the context, not a hook in it: the stores' tests mock that module. */ +export function urlPage(nav: Pick): Page | null { + return nav.channelView ? null : nav.page; +} diff --git a/frontend/src/features/video/ChannelNameLink.dom.test.tsx b/frontend/src/features/video/ChannelNameLink.dom.test.tsx index 481d7a48..d4e43911 100644 --- a/frontend/src/features/video/ChannelNameLink.dom.test.tsx +++ b/frontend/src/features/video/ChannelNameLink.dom.test.tsx @@ -48,8 +48,9 @@ describe("ChannelNameLink", () => { it("is an anchor whose href opens this channel on the current page", () => { const { link } = renderLink(); expect(link.tagName).toBe("A"); - // The same URL openChannel pushes: the current page's query plus ?chan=. - expect(link.getAttribute("href")).toBe(`/?page=feed&show=unwatched&chan=${CH}`); + // The same URL openChannel pushes: the current page plus ?chan= — not the page's own keys, + // which named a view the channel does not show (`show=unwatched` here). + expect(link.getAttribute("href")).toBe(`/?page=feed&chan=${CH}`); }); it("takes the in-app path on a plain left click, and suppresses the browser navigation", () => { @@ -73,11 +74,16 @@ describe("ChannelNameLink", () => { expect(onOpenInApp).not.toHaveBeenCalled(); }); - it("does not lose the rest of the address bar", () => { - window.history.replaceState({}, "", "/?page=channels&q=abc"); + // The address names only what the screen shows: the channel over the page, not the page's filters + // (a link's new tab once opened the channel under the feed's search and sort). + it("keeps the page and the shell's keys, and leaves the page's and another channel's behind", () => { + window.history.replaceState( + {}, + "", + "/?page=channels&q=abc&ctab=hidden&chtab=about&m=signup#x=1", + ); const { link } = renderLink(); - expect(link.getAttribute("href")).toContain("page=channels"); - expect(link.getAttribute("href")).toContain("q=abc"); + expect(link.getAttribute("href")).toBe(`/?page=channels&chan=${CH}&m=signup#x=1`); }); it("gives a channel with no title yet an accessible name", () => { diff --git a/frontend/src/features/video/ChannelNameLink.tsx b/frontend/src/features/video/ChannelNameLink.tsx index 1d129e5e..7270b8b1 100644 --- a/frontend/src/features/video/ChannelNameLink.tsx +++ b/frontend/src/features/video/ChannelNameLink.tsx @@ -1,15 +1,15 @@ import { forwardRef, type AnchorHTMLAttributes } from "react"; import { useTranslation } from "react-i18next"; import { isPlainLeftClick } from "../../lib/nav"; -import { urlWithParams } from "../../lib/urlState"; +import { channelUrl } from "../../lib/urlState"; // A channel name that is a REAL link. The player showed it as a `