diff --git a/backend/tests/test_download_failure_catalog.py b/backend/tests/test_download_failure_catalog.py index ec241e26..af661721 100644 --- a/backend/tests/test_download_failure_catalog.py +++ b/backend/tests/test_download_failure_catalog.py @@ -400,3 +400,45 @@ def test_an_mp3_download_is_stored_as_an_mp3(db, monkeypatch, tmp_path): asset = db.get(MediaAsset, a.id) assert (asset.container, asset.acodec, asset.media_kind) == ("mp3", "mp3", "audio") assert asset.rel_path.endswith(".mp3") + + +@pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available") +@pytest.mark.parametrize("mode", ["av", "v"]) +def test_a_video_download_is_still_made_browser_playable(db, monkeypatch, tmp_path, mode): + # The mp3 fix skips the browser-playable pass for AUDIO only: a video that arrives in a container + # browsers won't play (an .mkv) is still remuxed into an .mp4. + u = _user(db, f"fb-mkv-{mode}@example.com") + a, j = _asset_and_job( + db, u, sig=f"mkv-{mode}", asset_status="downloading", job_status="running" + ) + monkeypatch.setattr(settings, "download_root", str(tmp_path)) + monkeypatch.setattr(pipeline.urlguard, "check_url", lambda url: None) + + def extract(job_id, asset_id, url, spec, staging, adapter, **kwargs): + media = staging / "1736360027626346.mkv" + audio = ["-f", "lavfi", "-i", "sine=frequency=440", "-c:a", "aac"] if mode == "av" else [] + subprocess.run( + [ + "ffmpeg", + "-y", + "-loglevel", + "error", + "-f", + "lavfi", + "-i", + "testsrc=size=64x64:rate=10", + ] + + audio + + ["-t", "1", "-c:v", "libx264", "-pix_fmt", "yuv420p", str(media)], + check=True, + ) + return {"id": "1736360027626346", "ext": "mkv", "title": "Reel", "filepath": str(media)} + + monkeypatch.setattr(pipeline, "_extract_with_fallback", extract) + + pipeline._download(j.id, a.id, "url", _REEL, formats.normalize({"mode": mode}), u.id) + + db.expire_all() + asset = db.get(MediaAsset, a.id) + assert (asset.container, asset.vcodec) == ("mp4", "h264") + assert asset.rel_path.endswith(".mp4") diff --git a/backend/tests/test_formats.py b/backend/tests/test_formats.py index 5eec3e51..c49cb97b 100644 --- a/backend/tests/test_formats.py +++ b/backend/tests/test_formats.py @@ -170,3 +170,12 @@ def test_audio_versions_its_cache_identity_apart_from_video(monkeypatch): assert formats.format_sig(video) == before[1] monkeypatch.setattr(formats, "_SIG_VERSION", formats._SIG_VERSION + 1) assert formats.format_sig(video) != before[1] + + +def test_the_mp3_fix_moved_the_audio_cache_identity(monkeypatch): + # Before the fix every mode hashed with version 4, so an audio signature still at 4 would find the + # cached mp3-remuxed-into-mp4 assets iPad refuses (prod job 268) instead of re-deriving them. + audio = {"mode": "a", "audio_format": "mp3"} + current = formats.format_sig(audio) + monkeypatch.setattr(formats, "_AUDIO_SIG_VERSION", 4) + assert formats.format_sig(audio) != current diff --git a/frontend/src/components/BootGate.dom.test.tsx b/frontend/src/components/BootGate.dom.test.tsx index 59e608cb..22e76133 100644 --- a/frontend/src/components/BootGate.dom.test.tsx +++ b/frontend/src/components/BootGate.dom.test.tsx @@ -9,20 +9,26 @@ // Real NavigationProvider, real storage and i18n; only the two boot probes are stubbed. The probe // records EVERY render, so "never showed the wrong page, not even for one render" is what is asserted. import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { render, screen, cleanup, act } from "@testing-library/react"; +import { render, screen, cleanup, act, fireEvent, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { useEffect } from "react"; import { useTranslation } from "react-i18next"; import BootGate from "./BootGate"; import { NavigationProvider, useNavigation } from "./NavigationProvider"; import { api, type Me } from "../lib/api"; import { ACTIVE_ACCOUNT_KEY } from "../lib/activeAccount"; +import { qk } from "../lib/queryKeys"; import i18n from "../i18n"; const renders: { page: string; lang: string }[] = []; +let mounts = 0; function Probe() { const { page } = useNavigation(); const { i18n: inst } = useTranslation(); renders.push({ page, lang: inst.language }); + useEffect(() => { + mounts++; + }, []); return
{page}
; } @@ -37,20 +43,24 @@ function mount() { , ); + return qc; } const account = (over: Partial = {}) => ({ id: 5, preferences: {}, ...over }) as Me; +const retryButton = () => screen.findByRole("button", { name: i18n.t("common.retry") }); beforeEach(async () => { localStorage.clear(); sessionStorage.clear(); // a NEW tab: nothing pinned yet window.history.replaceState(null, "", "/"); renders.length = 0; + mounts = 0; await i18n.changeLanguage("en"); vi.spyOn(api, "setupStatus").mockResolvedValue({ configured: true }); }); afterEach(() => { cleanup(); vi.restoreAllMocks(); + vi.unstubAllGlobals(); }); describe("BootGate", () => { @@ -90,4 +100,77 @@ describe("BootGate", () => { await screen.findByTestId("app"); expect(sessionStorage.getItem(ACTIVE_ACCOUNT_KEY)).toBeNull(); }); + + it("setup mode: mounts the app (App shows the wizard) without asking for the account", async () => { + vi.mocked(api.setupStatus).mockResolvedValue({ configured: false }); + const me = vi.spyOn(api, "me").mockResolvedValue(account()); + mount(); + + await screen.findByTestId("app"); + expect(me).not.toHaveBeenCalled(); + expect(sessionStorage.getItem(ACTIVE_ACCOUNT_KEY)).toBeNull(); + }); + + it("an unanswered setup probe is an error with a retry — never the signed-out landing", async () => { + vi.mocked(api.setupStatus) + .mockRejectedValueOnce(new Error("502")) + .mockResolvedValue({ configured: true }); + vi.spyOn(api, "me").mockResolvedValue(account()); + mount(); + + fireEvent.click(await retryButton()); + await screen.findByTestId("app"); + expect(renders.length).toBeGreaterThan(0); + expect(sessionStorage.getItem(ACTIVE_ACCOUNT_KEY)).toBe("5"); // it mounted for the account + }); + + it("an unanswered first account probe is an error with a retry", async () => { + vi.spyOn(api, "me").mockRejectedValueOnce(new Error("502")).mockResolvedValue(account()); + mount(); + + fireEvent.click(await retryButton()); + await screen.findByTestId("app"); + expect(sessionStorage.getItem(ACTIVE_ACCOUNT_KEY)).toBe("5"); + }); + + it("a background refetch that fails keeps the app mounted, as it was", async () => { + const me = vi.spyOn(api, "me").mockResolvedValue(account()); + const qc = mount(); + await screen.findByTestId("app"); + + me.mockRejectedValue(new Error("502")); + await act(() => qc.refetchQueries({ queryKey: qk.me() })); + // react-query hands the error to its observers on a timer: let that render land before asserting. + await act(() => new Promise((r) => setTimeout(r, 20))); + + expect(qc.getQueryState(qk.me())?.status).toBe("error"); + expect(screen.getByTestId("app")).toBeTruthy(); + expect(mounts).toBe(1); + }); + + it("a pin naming another account than the server answered as is corrected before the app reads it", async () => { + // Pinned to 9, but 9 was signed out in another tab: the server answers as the browser's default, 5. + sessionStorage.setItem(ACTIVE_ACCOUNT_KEY, "9"); + localStorage.setItem("siftlode.startOnHome.5", "0"); + localStorage.setItem("siftlode.page.5", "downloads"); + vi.spyOn(api, "me").mockResolvedValue(account()); + mount(); + + expect((await screen.findByTestId("app")).textContent).toBe("downloads"); + expect(renders.map((r) => r.page)).toEqual(renders.map(() => "downloads")); + expect(sessionStorage.getItem(ACTIVE_ACCOUNT_KEY)).toBe("5"); + }); + + it("a mounted tab whose identity changes reloads rather than keep the previous account's tree", async () => { + const me = vi.spyOn(api, "me").mockResolvedValue(account()); + const qc = mount(); + await screen.findByTestId("app"); + const reload = vi.fn(); + vi.stubGlobal("location", { ...window.location, reload }); + + me.mockResolvedValue(null); // the session ended + await act(() => qc.refetchQueries({ queryKey: qk.me() })); + + await waitFor(() => expect(reload).toHaveBeenCalled()); + }); }); diff --git a/frontend/src/components/BootGate.tsx b/frontend/src/components/BootGate.tsx index 666dafdf..0946c585 100644 --- a/frontend/src/components/BootGate.tsx +++ b/frontend/src/components/BootGate.tsx @@ -1,10 +1,10 @@ -import { Fragment, useLayoutEffect, useState, type ReactNode } from "react"; +import { useLayoutEffect, useState, type ReactNode } from "react"; import { useQuery } from "@tanstack/react-query"; import { useTranslation } from "react-i18next"; import { api, getActiveAccount, setActiveAccount } from "../lib/api"; import { qk } from "../lib/queryKeys"; import i18n, { isSupported, setLanguage } from "../i18n"; -import { StateMessage } from "./ui/QueryState"; +import { LoadingState, StateMessage } from "./ui/QueryState"; // The boot sequence, in ONE place: nothing of the app renders until this tab knows WHO it is. // @@ -18,27 +18,23 @@ import { StateMessage } from "./ui/QueryState"; // settle the identity, adopt what must be adopted before a single pixel of the app is drawn, THEN mount // the provider tree — whose initializers are then simply right, with nothing to re-read. // -// The tree is keyed on the account id, which enforces that in code rather than in a comment: if a tab's -// identity ever changed without a reload, every per-account initializer would run again for the new -// account instead of holding the previous one's values. (Today every switch reloads — NavSidebar.) +// The per-account initializers read the tab's PIN (sessionStorage), not `me`, so the pin is made to +// match the account the server answered as before they run. And the tree is only ever built for one +// identity: if a mounted tab's identity changes without a reload (its session ended, or the account it +// was pinned to was signed out in another tab and the server now answers as the browser's default), +// the tab reloads, like every account switch does (NavSidebar) — which also drops the previous +// account's query cache, whose keys are not per account. -/** The quiet screen shown while the tab's identity (or the app chunk) is still on its way. On a normal - * connection that is a few hundred milliseconds, and a "Loading…" label flashed for that long reads as - * a flicker, not as information — so the label only fades in once the wait is long enough to need one. - * The delay counts from navigation start, not from mount, so the gate's screen and the Suspense - * fallback that may follow it keep one clock instead of each restarting the fade. */ +/** The quiet screen shown while the tab's identity (or the app chunk) is still on its way — the shared + * loading visual, full screen. Its reveal counts from navigation start, so the gate's screen, the + * Suspense fallback after it and the first page's own loading state keep one clock (see LoadingState). */ export function BootScreen() { - const { t } = useTranslation(); - const [wait] = useState(() => LABEL_DELAY_MS - performance.now()); return ( -
- 0 ? { animation: `fadeIn 0.2s ease ${wait}ms both` } : undefined}> - {t("common.loading")} - +
+
); } -const LABEL_DELAY_MS = 400; export default function BootGate({ children }: { children: ReactNode }) { const { t } = useTranslation(); @@ -52,9 +48,10 @@ export default function BootGate({ children }: { children: ReactNode }) { }); const needsSetup = setupQuery.data?.configured === false; // THE fetching `me` query; every other reader is a passive (enabled:false) observer of its cache. - // Polled while signed in so an account an admin suspends or deletes is noticed with no interaction: - // the refetch 401s and App's handler drops to the login page. Not polled signed out — that refetch - // is pointless and used to flicker the public landing. + // Polled while signed in so an ended session is noticed with no interaction: /api/me answers a + // signed-out (or suspended) caller with 200 and no account, the identity drops to "nobody", and the + // tab reloads to the login page. Not polled signed out — that refetch is pointless and used to + // flicker the public landing. const meQuery = useQuery({ queryKey: qk.me(), queryFn: api.me, @@ -63,13 +60,13 @@ export default function BootGate({ children }: { children: ReactNode }) { }); const me = meQuery.data; - // Who this tab is, once known: an account id, `null` for "nobody" (signed out, setup mode, or no - // setup answer at all — App renders the landing / the wizard for those), `undefined` while unknown. + // Who this tab is, once known: an account id, `null` for "nobody" (signed out, or setup mode — App + // renders the landing / the wizard for those), `undefined` while unknown or unanswerable. // Keyed on the DATA, not the status: a background refetch that fails keeps the last answer, so a // server blip never tears the whole app down to this screen. - const identity: number | null | undefined = setupQuery.isPending + const identity: number | null | undefined = !setupQuery.data ? undefined - : !setupQuery.isSuccess || needsSetup + : needsSetup ? null : me === undefined ? undefined @@ -77,16 +74,22 @@ export default function BootGate({ children }: { children: ReactNode }) { // Adopt the identity before the tree mounts. A layout effect, so the re-render it triggers happens // before the browser paints: the frame that first shows the app already has the account pinned. - // • Pin this tab to the account it loaded as. Without the pin, a tab riding the session's default - // account would silently swap identity when ANOTHER tab changes that default (adds or switches - // an account); once pinned it sends its own account header. An explicit switch on THIS tab sets - // the pin itself, so this is a no-op then. + // • Pin this tab to the account the server answered as. Without the pin, a tab riding the + // session's default account would silently swap identity when ANOTHER tab changes that default + // (adds or switches an account); once pinned it sends its own account header. The server honours + // that header only for an account still signed in to this browser, so a pin naming one that is + // not is corrected here — otherwise the tab would run as the default account on the server while + // reading and writing the gone account's local state. // • Adopt the account's language, which follows the account across devices — the local copy only // knows what this browser last showed. const [readyFor, setReadyFor] = useState(undefined); useLayoutEffect(() => { if (identity === undefined) return; - if (me && getActiveAccount() == null) setActiveAccount(me.id); + if (me && getActiveAccount() !== me.id) setActiveAccount(me.id); + if (readyFor !== undefined) { + location.reload(); // the tree was built for another identity — see the header + return; + } const lang = me?.preferences?.language; if (typeof lang === "string" && isSupported(lang) && lang !== i18n.language) setLanguage(lang); setReadyFor(identity); @@ -95,16 +98,24 @@ export default function BootGate({ children }: { children: ReactNode }) { // eslint-disable-next-line react-hooks/exhaustive-deps }, [identity]); - // No answer at all (a network or 5xx failure on the very first probe): say so, with a retry. After - // a retry the identity lands and the tree mounts from scratch, like any first load. - if (me === undefined && meQuery.isError) + // No answer at all (a network or 5xx failure on the very first setup or account probe — past the + // one retry `req` already makes): say so, with a retry. An unanswered setup probe is NOT "signed + // out": rendering the login page to a signed-in user would be a lie. After a retry the identity + // lands and the tree mounts from scratch, like any first load. + const failed = + !setupQuery.data && setupQuery.isError + ? setupQuery + : me === undefined && meQuery.isError + ? meQuery + : null; + if (failed) return (
- meQuery.refetch()}> + failed.refetch()}> {t("common.somethingWrong")}
); if (identity === undefined || readyFor !== identity) return ; - return {children}; + return <>{children}; } diff --git a/frontend/src/components/FeedViewProvider.tsx b/frontend/src/components/FeedViewProvider.tsx index 61ce301d..a07ad2f4 100644 --- a/frontend/src/components/FeedViewProvider.tsx +++ b/frontend/src/components/FeedViewProvider.tsx @@ -7,9 +7,9 @@ import { usePref } from "../lib/usePref"; // page and the channel page read it via hooks instead of prop-drilling `view`/`setView` through App. // Split State + Actions contexts, mirroring the other providers; composed in main.tsx around . // -// The view mode is an account PREFERENCE, not a filter: it persists to the server on pick and is -// adopted from server prefs on login. usePref carries that server-sync (plus the localStorage mirror -// that seeds the first render synchronously) so this provider is just the two contexts over it. +// The view mode is an account PREFERENCE, not a filter: it persists to the server on pick and follows +// the account across devices. usePref carries that server-sync (seeded from the server pref, with the +// localStorage mirror as the fallback) so this provider is just the two contexts over it. interface FeedViewActions { setView: (v: FeedView) => void; diff --git a/frontend/src/components/PlexProvider.dom.test.tsx b/frontend/src/components/PlexProvider.dom.test.tsx new file mode 100644 index 00000000..2ce759ae --- /dev/null +++ b/frontend/src/components/PlexProvider.dom.test.tsx @@ -0,0 +1,45 @@ +// @vitest-environment jsdom +// +// A shared Plex link is persisted to the account, but only the dimensions it CARRIES. The bug it +// replaced: a link with only a scope also wrote the (empty) default filters, and a stored Plex filters +// value at all is what tells SavedViewsProvider the account has picked filters — so the account's +// default saved view was never applied again. +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { render, cleanup } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { NavigationProvider } from "./NavigationProvider"; +import { PlexProvider } from "./PlexProvider"; +import { ACTIVE_ACCOUNT_KEY } from "../lib/activeAccount"; + +function mount() { + render( + + + +
+ + + , + ); +} + +beforeEach(() => { + localStorage.clear(); + sessionStorage.setItem(ACTIVE_ACCOUNT_KEY, "5"); +}); +afterEach(() => { + cleanup(); + window.history.replaceState(null, "", "/"); +}); + +describe("PlexProvider shared link", () => { + it("persists the scope a link carries and leaves the dimensions it omits unwritten", () => { + window.history.replaceState(null, "", "/?page=plex&pscope=movie"); + mount(); + + expect(localStorage.getItem("siftlode.plexLibrary.5")).toBe("movie"); + expect(localStorage.getItem("siftlode.plexFilters.5")).toBeNull(); + expect(localStorage.getItem("siftlode.plexSort.5")).toBeNull(); + expect(localStorage.getItem("siftlode.plexShow.5")).toBeNull(); + }); +}); diff --git a/frontend/src/components/PlexProvider.tsx b/frontend/src/components/PlexProvider.tsx index dd9984cf..b15a8fab 100644 --- a/frontend/src/components/PlexProvider.tsx +++ b/frontend/src/components/PlexProvider.tsx @@ -151,15 +151,25 @@ function readPlexFilters(): PlexFilters { } // The base is this account's stored view (BootGate pins the account before this provider mounts), so a // PARTIAL link — one carrying only the scope, say — keeps the stored sort, show and filters for the rest. -function readPlexUrlSeed(): PlexUrlState | null { +// `present` records which dimensions the link actually carried: only those are persisted (below). +type PlexSeedPresent = { scope: boolean; sort: boolean; show: boolean; filters: boolean }; +type PlexUrlSeed = PlexUrlState & { present: PlexSeedPresent }; +function readPlexUrlSeed(): PlexUrlSeed | null { const params = new URLSearchParams(window.location.search); if (!hasPlexParams(params)) return null; - return plexParamsToState(params, { + const state = plexParamsToState(params, { scope: readPlexScope(), sort: readPlexSort(), show: readPlexShow(), filters: readPlexFilters(), }); + const present = { + scope: params.has("pscope"), + sort: params.has("psort"), + show: params.has("pshow"), + filters: params.has("pf"), + }; + return { ...state, present }; } export function PlexProvider({ children }: { children: ReactNode }) { @@ -195,14 +205,17 @@ export function PlexProvider({ children }: { children: ReactNode }) { const [jumpTo, setJumpTo] = useState(null); // Persist a shared link's view to the account once, so it wins over the stored view and survives a - // later plain reload. The seed's base was the stored view, so a dimension the link omitted is written - // back unchanged. Mirrors FeedFiltersProvider's initialHadUrlFilters branch. + // later plain reload. Mirrors FeedFiltersProvider's initialHadUrlFilters branch. Only the dimensions + // the link CARRIED are written: an omitted one already reads from storage, and writing its default + // would not be a no-op — a stored Plex filters value at all is what tells SavedViewsProvider the + // account has picked filters, so the account's default saved view would never be applied again. useEffect(() => { if (!urlSeed) return; - setAccountRaw(LS.plexLibrary, urlSeed.scope); - setAccountRaw(LS.plexShow, urlSeed.show); - setAccountRaw(LS.plexSort, JSON.stringify(urlSeed.sort)); - setAccountRaw(LS.plexFilters, JSON.stringify(urlSeed.filters)); + const { present } = urlSeed; + if (present.scope) setAccountRaw(LS.plexLibrary, urlSeed.scope); + if (present.show) setAccountRaw(LS.plexShow, urlSeed.show); + if (present.sort) setAccountRaw(LS.plexSort, JSON.stringify(urlSeed.sort)); + if (present.filters) setAccountRaw(LS.plexFilters, JSON.stringify(urlSeed.filters)); }, [urlSeed]); const setScope = useCallback((v: string) => { diff --git a/frontend/src/components/PrefsProvider.tsx b/frontend/src/components/PrefsProvider.tsx index 65112227..37689927 100644 --- a/frontend/src/components/PrefsProvider.tsx +++ b/frontend/src/components/PrefsProvider.tsx @@ -28,8 +28,8 @@ const PERF_KEY = LS.perfMode; // The four Settings-page preferences. They apply locally for instant preview and auto-save (debounced) // to the server — there is no draft or Save button. This provider owns them (lifted out of the App god -// component) plus the last-saved "baseline" used to suppress the echo when a change came from adopting -// server prefs on login. Consumers (SettingsPanel, GlassTuner) read the PrefsController via usePrefs(). +// component) plus the last-saved "baseline" (the server values it was seeded with, then each successful +// save) used to suppress saving what the server already holds. Consumers (SettingsPanel, GlassTuner) read the PrefsController via usePrefs(). type EditablePrefs = { theme: ThemePrefs; performanceMode: boolean; @@ -102,8 +102,8 @@ export function PrefsProvider({ children }: { children: ReactNode }) { useEffect(() => configureNotifications(notif), [notif]); // Auto-save on change (debounced ~500ms so a slider drag coalesces into one PUT). Live-apply is the - // effects above; this is the only persistence path. `baselineRef` suppresses the echo when the - // change came from adopting server prefs on login (`/me` merges, so a full payload is fine). + // effects above; this is the only persistence path. `baselineRef` keeps the mount seed from being + // saved straight back (`/me` merges, so a full payload is fine). const editablePrefs = useMemo( () => ({ theme, performanceMode: perf, hints, notifications: notif }), [theme, perf, hints, notif], @@ -111,7 +111,7 @@ export function PrefsProvider({ children }: { children: ReactNode }) { useEffect(() => { window.clearTimeout(saveTimer.current); if (JSON.stringify(editablePrefs) === JSON.stringify(baselineRef.current)) { - // Back at the last-saved value (initial load, server adopt, or an edit undone) — nothing to + // Back at the last-saved value (the mount seed, or an edit undone) — nothing to // save; clear any "Saving…" left from a change that was reverted before it flushed. setSaveState((s) => (s === "saving" ? "idle" : s)); return; diff --git a/frontend/src/components/WatchPage.tsx b/frontend/src/components/WatchPage.tsx index e4b34faf..8e15ca63 100644 --- a/frontend/src/components/WatchPage.tsx +++ b/frontend/src/components/WatchPage.tsx @@ -3,7 +3,7 @@ import { Download, File as FileIcon, Link2, Lock, PlayCircle, Repeat } from "luc import { Button } from "./ui/Button"; import { formatBytes } from "../lib/format"; import { exposeVideoToBackgroundPlay } from "../lib/browserBackgroundPlay"; -import { LS } from "../lib/storage"; +import { LS, readJSON, writeJSON } from "../lib/storage"; // Standalone, login-free video page for a shared /watch/ link. Rendered OUTSIDE the app // tree (no auth, no providers) — like the /privacy and /terms pages. Uses plain fetch and a tiny @@ -137,21 +137,11 @@ export default function WatchPage() { // Repeat the shared media when it ends — on unless the viewer turned it off. Remembered per browser // (this page has no account); an unreadable storage just means the default. - const [repeat, setRepeat] = useState(() => { - try { - return localStorage.getItem(LS.watchRepeat) !== "0"; - } catch { - return true; - } - }); + const [repeat, setRepeat] = useState(() => readJSON(LS.watchRepeat, true) !== false); const toggleRepeat = () => { const next = !repeat; setRepeat(next); - try { - localStorage.setItem(LS.watchRepeat, next ? "1" : "0"); - } catch { - /* storage unavailable — the toggle still applies to this visit */ - } + writeJSON(LS.watchRepeat, next); }; useEffect(() => { @@ -310,9 +300,13 @@ export default function WatchPage() { />
)} -
-
-

{meta.title || "Video"}

+ {/* The actions wrap under the title when the row is too narrow for both (a phone), rather + than squeezing the title into a column a word wide. */} +
+
+

+ {meta.title || "Video"} +

{meta.uploader && (meta.uploader_url ? ( {meta.uploader}
))}
- - {meta.allow_download && ( - + {/* An on/off toggle, so the shared pressed-pill paint (a solid fill when on) — not a + Button, whose variants differ only in colour. */} + + {meta.allow_download && ( + + {T.download} + + )} +
{(meta.source_url || (meta.extra_links && meta.extra_links.length > 0)) && (
diff --git a/frontend/src/components/ui/QueryState.dom.test.tsx b/frontend/src/components/ui/QueryState.dom.test.tsx new file mode 100644 index 00000000..cdfc420e --- /dev/null +++ b/frontend/src/components/ui/QueryState.dom.test.tsx @@ -0,0 +1,64 @@ +// @vitest-environment jsdom +// +// A loading state fades in only after a delay, and loading states that hand over to each other share +// ONE clock. The bug it replaced: every LoadingState restarted its own 300 ms CSS delay, so a slow +// first load went spinner → blank → spinner when the page chunk's Suspense fallback handed over to +// the page's own "fetching" state. The clock is module state, so each test runs at its own time. +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { render, cleanup } from "@testing-library/react"; +import { LoadingState } from "./QueryState"; + +let now = 0; +beforeEach(() => { + vi.spyOn(performance, "now").mockImplementation(() => now); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** The reveal delay the rendered loading state waits, in ms (0 = shown at once). */ +function revealDelay(container: HTMLElement): number { + const el = container.querySelector("[role=status]")!; + const m = /ease (\d+)ms/.exec(el.style.animation); + return m ? Number(m[1]) : 0; +} + +describe("LoadingState reveal", () => { + it("a loading state on its own waits the full delay", () => { + now = 10_000; + const { container } = render(); + expect(revealDelay(container)).toBe(300); + }); + + it("one that takes over from a loading state on screen keeps that one's clock", () => { + now = 20_000; + const { container, rerender } = render(); + now = 20_200; + rerender(); // a new element: the fallback leaves as it mounts + expect(revealDelay(container)).toBe(100); + }); + + it("once the shared clock has run out, the next one in the hand-off shows at once", () => { + now = 30_000; + const { container, rerender } = render(); + now = 30_400; + rerender(); + expect(revealDelay(container)).toBe(0); + }); + + it("a loading state after a pause starts a new clock", () => { + now = 40_000; + const first = render(); + first.unmount(); + now = 40_500; + const { container } = render(); + expect(revealDelay(container)).toBe(300); + }); + + it("`since` counts the wait from an earlier start (the boot screen: navigation start)", () => { + now = 50_250; + const { container } = render(); + expect(revealDelay(container)).toBe(150); + }); +}); diff --git a/frontend/src/components/ui/QueryState.tsx b/frontend/src/components/ui/QueryState.tsx index cfdd1062..d93334bc 100644 --- a/frontend/src/components/ui/QueryState.tsx +++ b/frontend/src/components/ui/QueryState.tsx @@ -1,22 +1,55 @@ -import type { ReactNode } from "react"; +import { useEffect, useState, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; import { AlertTriangle, Loader2 } from "lucide-react"; import { Button } from "./Button"; +// A loading state fades in only after a delay: most loads here answer in tens of milliseconds, and a +// spinner shown for two frames before the content replaces it reads as a flicker, not as feedback. A +// load that really takes a while still gets the spinner, a third of a second in. +// +// The delay is ONE clock across loading states that hand over to each other, not a per-element CSS +// delay. One wait is often several elements in a row — the boot screen, then the page chunk's +// Suspense fallback, then the page's own "fetching" state — and a delay restarted by each would blank +// an already-visible spinner for another 300 ms at every hand-off. A loading state that mounts while +// another is still on screen (or has just left it) continues that one's clock instead. +const LOADING_REVEAL_MS = 300; +const HANDOFF_MS = 100; +const reveal = { live: 0, at: 0, lastGone: -Infinity }; +function revealWait(since: number | undefined, delayMs: number): number { + const now = performance.now(); + if (reveal.live === 0 && now - reveal.lastGone > HANDOFF_MS) reveal.at = (since ?? now) + delayMs; + return reveal.at - now; +} + // Centered loading state: a spinning accent ring + an optional label. The single shared loading // visual for a page/section's first load, so all of them read the same (and can be swapped for // skeletons later in one place). `role=status`/`aria-live` announces it to screen readers. -// -const LOADING_REVEAL_DELAY = "300ms"; -// It fades in only after LOADING_REVEAL_DELAY: most first loads here answer in tens of milliseconds, -// and a spinner shown for two frames before the content replaces it reads as a flicker, not as -// feedback. A load that really takes a while still gets the spinner, a third of a second in. -export function LoadingState({ label, className = "" }: { label?: ReactNode; className?: string }) { +export function LoadingState({ + label, + className = "", + since, + delayMs = LOADING_REVEAL_MS, +}: { + label?: ReactNode; + className?: string; + /** When the wait began (a `performance.now()` time), if earlier than this mount — the boot screen + * passes 0, navigation start. Only used when this state starts a new clock. */ + since?: number; + delayMs?: number; +}) { const { t } = useTranslation(); + const [wait] = useState(() => revealWait(since, delayMs)); + useEffect(() => { + reveal.live++; + return () => { + reveal.live--; + reveal.lastGone = performance.now(); + }; + }, []); return (
0 ? { animation: `fadeIn 0.2s ease ${Math.round(wait)}ms both` } : undefined} role="status" aria-live="polite" > diff --git a/frontend/src/lib/storage.ts b/frontend/src/lib/storage.ts index 71398cea..adc1b7b6 100644 --- a/frontend/src/lib/storage.ts +++ b/frontend/src/lib/storage.ts @@ -65,7 +65,7 @@ export const LS = { notifSettings: "siftlode.notifSettings", onboardingDismissed: "siftlode.onboarding.dismissed", seenVersion: "siftlode.seenVersion", - // The public /watch page's repeat toggle ("0" = off; on by default). Browser-wide, NOT per account: + // The public /watch page's repeat toggle (JSON boolean; on by default). Browser-wide, NOT per account: // that page runs outside the app, with no account at all. watchRepeat: "siftlode.watchRepeat", chatDock: (userId: number) => `siftlode.chatDock.${userId}`, diff --git a/frontend/src/lib/usePref.ts b/frontend/src/lib/usePref.ts index 8b0bc7d0..cab9c27b 100644 --- a/frontend/src/lib/usePref.ts +++ b/frontend/src/lib/usePref.ts @@ -20,8 +20,8 @@ const reportPrefSaveFailure = (err: unknown) => // server-synced pref is a one-liner. // // It seeds straight from the `me` cache, which is always populated when it mounts: BootGate mounts the -// app tree only once /api/me has answered, keyed on the account, so there is nothing to adopt after -// mount. The source of truth is the server; the `me` cache is kept in sync optimistically so other +// app tree only once /api/me has answered, and a tab whose account changes reloads, so there is nothing +// to adopt after mount. The source of truth is the server; the `me` cache is kept in sync optimistically so other // consumers reading the same key (e.g. a second player instance) see the change at once. export interface UsePrefOptions { @@ -75,8 +75,8 @@ export function usePref( const { localKey, debounceMs, parse } = options; const qc = useQueryClient(); - // Latest parse in a ref so the seed/adopt/setter don't depend on a fresh closure each render (which - // would churn the memoized setter and the adopt effect). parse must be idempotent anyway. + // Latest parse in a ref so the seed and the setter don't depend on a fresh closure each render (which + // would churn the memoized setter). parse must be idempotent anyway. const parseRef = useRef(parse); parseRef.current = parse; const norm = useCallback((raw: unknown): T => { @@ -98,7 +98,7 @@ export function usePref( const saveTimer = useRef(undefined); // The pending debounced server write, if any — flushed (NOT dropped) on unmount so the last change // isn't lost. Dropping it would leave the server stale while local/cache/localStorage hold the new - // value, and a later adopt could then overwrite the newer local value with the stale server one. + // value, and the next load — which seeds from the server first — would bring the old value back. const pendingSave = useRef<(() => void) | null>(null); useEffect( () => () => { @@ -113,8 +113,8 @@ export function usePref( (next: T) => { setValue(next); if (localKey) writeAccount(localKey, next); - // Keep the `me` cache in step so other readers of this key (and this hook's own adopt effect) - // see the new value without a refetch. Merges into preferences, mirroring the server PUT. + // Keep the `me` cache in step so other readers of this key (and a later mount of this hook, + // which seeds from the cache) see the new value without a refetch. Merges into preferences, mirroring the server PUT. const payload = { [serverKey]: next }; mergePrefsCache(qc, payload); // savePrefs is fire-and-forget (idempotent PUT, so `req` retries a transient blip once); a