refactor(nav): drop redundant sub-view markDirty; add useFilterState re-hydrate tests
Addresses the 2 findings from /code-review high on 726c382:
- NavigationProvider: openChannel/enterYtSearch no longer markDirty. A sub-view is an
overlay, not a page change — the page re-hydrate's replaceState already spreads the
_chan/_yt marker, so the overlay survives on top of the RESTORED page. Removing the
markDirty lets a fresh tab still restore the stored landing page under a pre-pin overlay
(previously it stuck on the default). setPage keeps markDirty (a real navigation).
- useFilterState.dom.test.ts: model a mutable pinned account and add 3 integration tests —
null->id re-hydrate adopts the stored map, the migrate seed carries a legacy value forward
on re-hydrate when the key is empty, and a pre-pin edit is preserved (markDirty guard).
gate-green: vite build + vitest 120 + eslint-0 + knip + prettier. Fresh-tab restore re-E2E'd
(lands on Plex with full state, zero console errors).
This commit is contained in:
@@ -115,8 +115,8 @@ export function NavigationProvider({ children }: { children: ReactNode }) {
|
|||||||
setPageState(stored);
|
setPageState(stored);
|
||||||
setAccountRaw(PAGE_KEY, stored); // no-op re-write, keeps storage authoritative
|
setAccountRaw(PAGE_KEY, stored); // no-op re-write, keeps storage authoritative
|
||||||
// Correct the CURRENT entry (replace, not push) and PRESERVE any markers already on it — a channel
|
// Correct the CURRENT entry (replace, not push) and PRESERVE any markers already on it — a channel
|
||||||
// (_chan) or YT search (_yt) opened in the pre-pin window must survive, or its overlay is orphaned
|
// (_chan) or YT search (_yt) opened in the pre-pin window survives on top of the restored page,
|
||||||
// from history and Back can't close it. (Those setters also markDirty, so this usually no-ops then.)
|
// rather than being orphaned from history so Back can't close it.
|
||||||
window.history.replaceState({ ...(window.history.state || {}), sfPage: stored }, "");
|
window.history.replaceState({ ...(window.history.state || {}), sfPage: stored }, "");
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -137,35 +137,31 @@ export function NavigationProvider({ children }: { children: ReactNode }) {
|
|||||||
[markDirty],
|
[markDirty],
|
||||||
);
|
);
|
||||||
|
|
||||||
const openChannel = useCallback(
|
// A sub-view (channel / YT search) is an OVERLAY, not a page change, so it does NOT markDirty: a
|
||||||
(id: string, name?: string) => {
|
// pre-pin account re-hydrate should still restore the stored page underneath — the re-hydrate's
|
||||||
markDirty(); // opening a sub-view is a navigation — don't let a late account-pin re-hydrate override the page
|
// replaceState spreads the _chan/_yt markers, so the overlay survives on top of the restored page.
|
||||||
setChannelView({ id, name });
|
const openChannel = useCallback((id: string, name?: string) => {
|
||||||
const st = window.history.state || {};
|
setChannelView({ id, name });
|
||||||
if (st._chan) window.history.replaceState({ ...st, _chan: id, _chanName: name ?? null }, "");
|
const st = window.history.state || {};
|
||||||
else window.history.pushState({ ...st, _chan: id, _chanName: name ?? null }, "");
|
if (st._chan) window.history.replaceState({ ...st, _chan: id, _chanName: name ?? null }, "");
|
||||||
},
|
else window.history.pushState({ ...st, _chan: id, _chanName: name ?? null }, "");
|
||||||
[markDirty],
|
}, []);
|
||||||
);
|
|
||||||
|
|
||||||
const closeChannel = useCallback(() => {
|
const closeChannel = useCallback(() => {
|
||||||
if (window.history.state?._chan) window.history.back();
|
if (window.history.state?._chan) window.history.back();
|
||||||
else setChannelView(null);
|
else setChannelView(null);
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
const enterYtSearch = useCallback(
|
const enterYtSearch = useCallback((q: string) => {
|
||||||
(q: string) => {
|
// Overlay, not a page change — see openChannel: no markDirty, the re-hydrate preserves _yt.
|
||||||
markDirty(); // a sub-view navigation — see openChannel
|
setYtSearch(q);
|
||||||
setYtSearch(q);
|
// Drop any prior _ytScrape marker — the new search's source isn't known yet; Feed re-stamps it
|
||||||
// Drop any prior _ytScrape marker — the new search's source isn't known yet; Feed re-stamps it
|
// once the results resolve, so a reload only restores a confirmed scrape-sourced search.
|
||||||
// once the results resolve, so a reload only restores a confirmed scrape-sourced search.
|
const { _ytScrape: _drop, ...st } = window.history.state || {};
|
||||||
const { _ytScrape: _drop, ...st } = window.history.state || {};
|
if (st._yt)
|
||||||
if (st._yt)
|
window.history.replaceState({ ...st, _yt: q }, ""); // refine current search
|
||||||
window.history.replaceState({ ...st, _yt: q }, ""); // refine current search
|
else window.history.pushState({ ...st, sfPage: "feed", _yt: q }, ""); // new sub-view entry
|
||||||
else window.history.pushState({ ...st, sfPage: "feed", _yt: q }, ""); // new sub-view entry
|
}, []);
|
||||||
},
|
|
||||||
[markDirty],
|
|
||||||
);
|
|
||||||
|
|
||||||
const exitYtSearch = useCallback(() => {
|
const exitYtSearch = useCallback(() => {
|
||||||
// Pop our search entry (so Forward still works); fall back to a plain clear if we don't own one.
|
// Pop our search entry (so Forward still works); fall back to a plain clear if we don't own one.
|
||||||
|
|||||||
@@ -6,21 +6,27 @@
|
|||||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||||
import { renderHook, act } from "@testing-library/react";
|
import { renderHook, act } from "@testing-library/react";
|
||||||
|
|
||||||
const { store } = vi.hoisted(() => ({ store: new Map<string, string>() }));
|
// `ctl.account` models the tab's pinned account: getAccountRaw returns null while it's null (mirrors the
|
||||||
|
// real accountKey → no key → null), so a fresh-tab mount reads defaults until the account is pinned.
|
||||||
|
const { store, ctl } = vi.hoisted(() => ({
|
||||||
|
store: new Map<string, string>(),
|
||||||
|
ctl: { account: 1 as number | null },
|
||||||
|
}));
|
||||||
vi.mock("./storage", () => ({
|
vi.mock("./storage", () => ({
|
||||||
getAccountRaw: (k: string) => store.get(k) ?? null,
|
getAccountRaw: (k: string) => (ctl.account == null ? null : (store.get(k) ?? null)),
|
||||||
setAccountRaw: (k: string, v: string) => {
|
setAccountRaw: (k: string, v: string) => {
|
||||||
store.set(k, v);
|
if (ctl.account != null) store.set(k, v);
|
||||||
},
|
},
|
||||||
}));
|
}));
|
||||||
// A stable account id keeps useAccountRehydrate a no-op here (no null→id transition); the re-hydrate
|
vi.mock("./activeAccount", () => ({ useActiveAccountId: () => ctl.account }));
|
||||||
// path itself is covered in useAccountRehydrate.dom.test.ts + useSortState.dom.test.ts.
|
|
||||||
vi.mock("./activeAccount", () => ({ useActiveAccountId: () => 1 }));
|
|
||||||
|
|
||||||
import { useFilterState } from "./useFilterState";
|
import { useFilterState } from "./useFilterState";
|
||||||
|
|
||||||
describe("useFilterState", () => {
|
describe("useFilterState", () => {
|
||||||
beforeEach(() => store.clear());
|
beforeEach(() => {
|
||||||
|
store.clear();
|
||||||
|
ctl.account = 1;
|
||||||
|
});
|
||||||
|
|
||||||
it("hydrates to an empty map when storage is empty", () => {
|
it("hydrates to an empty map when storage is empty", () => {
|
||||||
const { result } = renderHook(() => useFilterState("k"));
|
const { result } = renderHook(() => useFilterState("k"));
|
||||||
@@ -71,4 +77,42 @@ describe("useFilterState", () => {
|
|||||||
// A seed that returns null falls back to an empty map.
|
// A seed that returns null falls back to an empty map.
|
||||||
expect(renderHook(() => useFilterState("k3", () => null)).result.current[0]).toEqual({});
|
expect(renderHook(() => useFilterState("k3", () => null)).result.current[0]).toEqual({});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The account-pin re-hydrate (via useAccountRehydrate) integrated into useFilterState.
|
||||||
|
it("re-hydrates the stored map once the account is pinned after mount", () => {
|
||||||
|
ctl.account = null; // fresh tab: no account at mount
|
||||||
|
store.set("k", JSON.stringify({ sync: "fully", tags: ["7"] }));
|
||||||
|
const { result, rerender } = renderHook(() => useFilterState("k"));
|
||||||
|
expect(result.current[0]).toEqual({}); // mount read with no account → empty
|
||||||
|
act(() => {
|
||||||
|
ctl.account = 1; // <App> pins the account
|
||||||
|
rerender();
|
||||||
|
});
|
||||||
|
expect(result.current[0]).toEqual({ sync: "fully", tags: ["7"] });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("applies the migrate seed on the account-pin re-hydrate when the key is still empty", () => {
|
||||||
|
ctl.account = null;
|
||||||
|
// Mirrors migrateSyncChip: reads an account-scoped legacy key, so it yields null until pinned.
|
||||||
|
const migrate = () => (ctl.account == null ? null : { sync: "needsFull" });
|
||||||
|
const { result, rerender } = renderHook(() => useFilterState("k", migrate));
|
||||||
|
expect(result.current[0]).toEqual({}); // migrate can't read the legacy key with no account yet
|
||||||
|
act(() => {
|
||||||
|
ctl.account = 1;
|
||||||
|
rerender();
|
||||||
|
});
|
||||||
|
expect(result.current[0]).toEqual({ sync: "needsFull" }); // migrate carries the legacy value forward
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does NOT clobber a pre-pin user edit when the account is then pinned (markDirty guard)", () => {
|
||||||
|
ctl.account = null;
|
||||||
|
store.set("k", JSON.stringify({ sync: "fully" })); // the stored value
|
||||||
|
const { result, rerender } = renderHook(() => useFilterState("k"));
|
||||||
|
act(() => result.current[1]({ sync: "needsFull" })); // user edits before the pin
|
||||||
|
act(() => {
|
||||||
|
ctl.account = 1;
|
||||||
|
rerender();
|
||||||
|
});
|
||||||
|
expect(result.current[0]).toEqual({ sync: "needsFull" }); // the edit survives, not "fully"
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user