test(views,layer): close the two polish-tail deep-review test gaps
UX-8 layerStack: add an integration test wired through the real useLayer hook (popover document-listener + stamp) and a PlayerModal-shaped window listener, driven by a real dispatched keydown. The popover close is wrapped in flushSync so its layer is popped synchronously between the two listeners -- the production mid-dispatch race -- making the escapeHandled stamp the only thing that keeps the player from also closing. Mutation-checked: removing the guard fails exactly this case. SavedViews: cover feedCanSeedDefault's "yield to a bookmarked filter URL" branch (a filter-bearing search string -> no seed) and the per-source seededRef guard against the 60s refetch (a changed-list refetch re-runs the seed effect; the guard blocks re-seed). The refetch test waits for the changed list to reach the component so the effect has its chance to wrongly re-fire. Both mutation-checked. Frontend gate green: 464 vitest (+4). Backend untouched.
This commit is contained in:
@@ -46,6 +46,7 @@ import { SavedViewsProvider, useSavedViews } from "./SavedViewsProvider";
|
||||
import { api, setActiveAccount, type SavedView } from "../lib/api";
|
||||
import { LS, accountKey } from "../lib/storage";
|
||||
import { ACTIVE_ACCOUNT_KEY } from "../lib/activeAccount";
|
||||
import { qk } from "../lib/queryKeys";
|
||||
|
||||
const ID = 7;
|
||||
const view = (over: Partial<SavedView>): SavedView => ({
|
||||
@@ -58,12 +59,19 @@ const view = (over: Partial<SavedView>): SavedView => ({
|
||||
});
|
||||
|
||||
function Probe() {
|
||||
const { loading } = useSavedViews();
|
||||
return <div data-testid="loading">{String(loading)}</div>;
|
||||
const { loading, views } = useSavedViews();
|
||||
return (
|
||||
<div data-testid="loading" data-count={views.length}>
|
||||
{String(loading)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
async function mountResolved(views: SavedView[]) {
|
||||
vi.spyOn(api, "savedViews").mockResolvedValue(views);
|
||||
// Fresh array + object copies per call, like a real fetch — so a refetch changes viewsQuery.data's
|
||||
// identity and re-runs the seed effect (which is what makes the per-source guard actually load-bearing
|
||||
// in the re-seed test; a shared reference would skip the effect and hide a missing guard).
|
||||
vi.spyOn(api, "savedViews").mockImplementation(async () => views.map((v) => ({ ...v })));
|
||||
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
render(
|
||||
<QueryClientProvider client={qc}>
|
||||
@@ -74,11 +82,15 @@ async function mountResolved(views: SavedView[]) {
|
||||
);
|
||||
// Wait for the query to resolve so the first-resolve effect has run.
|
||||
await waitFor(() => expect(screen.getByTestId("loading").textContent).toBe("false"));
|
||||
return qc;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
localStorage.clear();
|
||||
sessionStorage.setItem(ACTIVE_ACCOUNT_KEY, String(ID));
|
||||
// Reset the URL: feedCanSeedDefault reads window.location.search, and a stale ?sort= from a prior
|
||||
// case would suppress seeding in the others.
|
||||
window.history.replaceState({}, "", "/");
|
||||
h.page = "feed";
|
||||
h.setFeedFilters.mockClear();
|
||||
h.setPlexFilters.mockClear();
|
||||
@@ -123,6 +135,46 @@ describe("SavedViewsProvider — default-view seeding on first resolve", () => {
|
||||
expect(h.setFeedFilters).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("feed: does NOT seed when the URL carries filter params (a shared/bookmarked view wins)", async () => {
|
||||
// A link like ?sort=oldest is an explicit filter the user arrived on; the starred default must not
|
||||
// stomp it. feedCanSeedDefault() bails on hasFilterParams — plex ignores the URL, so this is feed-only.
|
||||
window.history.replaceState({}, "", "/?sort=oldest");
|
||||
await mountResolved([
|
||||
view({ id: 2, is_default: true, filters: { scope: "all", show: "watched" } }),
|
||||
]);
|
||||
expect(h.setFeedFilters).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does NOT re-seed on a background refetch that returns a changed list (per-source one-shot guard)", async () => {
|
||||
// The 60s refetch re-resolves the query; when it returns a CHANGED list (e.g. the user just added a
|
||||
// view elsewhere) react-query hands down a new reference and the seed effect re-runs. seededRef must
|
||||
// stop it re-applying the default over filters the user has since edited. (An identical refetch is
|
||||
// deduped by structural sharing and never reaches the effect, so the changed list is the real test.)
|
||||
const def = view({ id: 2, is_default: true, filters: { scope: "all", show: "watched" } });
|
||||
let calls = 0;
|
||||
vi.spyOn(api, "savedViews").mockImplementation(async () => {
|
||||
calls += 1;
|
||||
return calls === 1 ? [def] : [def, view({ id: 9, name: "added-later" })];
|
||||
});
|
||||
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
render(
|
||||
<QueryClientProvider client={qc}>
|
||||
<SavedViewsProvider>
|
||||
<Probe />
|
||||
</SavedViewsProvider>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
await waitFor(() => expect(screen.getByTestId("loading").textContent).toBe("false"));
|
||||
expect(h.setFeedFilters).toHaveBeenCalledTimes(1);
|
||||
await act(async () => {
|
||||
await qc.refetchQueries({ queryKey: qk.savedViews("feed") });
|
||||
});
|
||||
// Wait until the changed list has actually reached the component, so the seed effect has had its
|
||||
// chance to (wrongly) re-fire — otherwise the assertion could win by racing ahead of the re-render.
|
||||
await waitFor(() => expect(screen.getByTestId("loading").dataset.count).toBe("2"));
|
||||
expect(h.setFeedFilters).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("plex: seeds the starred default into a fresh module (parity with the feed)", async () => {
|
||||
h.page = "plex";
|
||||
await mountResolved([view({ id: 3, is_default: true, filters: { genres: ["Action"] } })]);
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// Integration guard for the UX-8 single-Escape fix — the sibling of layerStack.dom.test.ts, but wired
|
||||
// through the REAL `useLayer` hook and a REAL dispatched keydown instead of hand-rolled listeners. It
|
||||
// mounts the two production listener shapes as React components:
|
||||
// - the POPOVER exactly as AnchoredPopover uses it: `useLayer(open, onEscape)` installs the real
|
||||
// `document` keydown listener that stamps the event and closes on topmost;
|
||||
// - the PLAYER exactly as PlayerModal reads it: a `window` keydown listener that bails on
|
||||
// `escapeHandled(e)` first, then on `anyAbove()`.
|
||||
// The load-bearing subtlety the primitives test can only fake: the popover's close is wrapped in
|
||||
// `flushSync`, so React tears the popover down (and `useLayer` cleanup pops its layer) SYNCHRONOUSLY
|
||||
// between the `document` and `window` listeners — the real mid-dispatch flush. That makes the stack read
|
||||
// "nothing above the player" by the time the player's listener runs, so ONLY the event stamp can stop
|
||||
// the player from also closing. If the stamp regressed, test 1 would see the player close on the first
|
||||
// Escape. floating-ui isn't involved here (we drive `useLayer` directly), so no observer stubs needed.
|
||||
import { describe, it, expect, vi, afterEach } from "vitest";
|
||||
import { useEffect, useState } from "react";
|
||||
import { flushSync } from "react-dom";
|
||||
import { render, act, cleanup } from "@testing-library/react";
|
||||
import { useLayer, escapeHandled } from "./layerStack";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
// The player: registers a layer (no onEscape — it owns a richer handler) and a `window` listener that
|
||||
// mirrors PlayerModal's guard order (consumed-stamp first, then anyAbove).
|
||||
function Player({ onClose }: { onClose: () => void }) {
|
||||
const { anyAbove } = useLayer(true);
|
||||
useEffect(() => {
|
||||
const onKey = (e: KeyboardEvent) => {
|
||||
if (e.key !== "Escape") return;
|
||||
if (escapeHandled(e)) return; // the UX-8 fix
|
||||
if (anyAbove()) return; // the old, race-prone guard
|
||||
onClose();
|
||||
};
|
||||
window.addEventListener("keydown", onKey);
|
||||
return () => window.removeEventListener("keydown", onKey);
|
||||
}, [anyAbove, onClose]);
|
||||
return <div data-testid="player" />;
|
||||
}
|
||||
|
||||
// The popover: the real AnchoredPopover usage. flushSync makes the unmount (and its layer removal) run
|
||||
// synchronously inside the `document` listener — the production mid-dispatch flush the fix must survive.
|
||||
function Popover({ onClosed }: { onClosed: () => void }) {
|
||||
const [open, setOpen] = useState(true);
|
||||
useLayer(open, () => {
|
||||
flushSync(() => setOpen(false));
|
||||
onClosed();
|
||||
});
|
||||
return open ? <div data-testid="popover" /> : null;
|
||||
}
|
||||
|
||||
function dispatchEscape(): KeyboardEvent {
|
||||
const e = new KeyboardEvent("keydown", { key: "Escape", bubbles: true });
|
||||
act(() => {
|
||||
document.dispatchEvent(e);
|
||||
});
|
||||
return e;
|
||||
}
|
||||
|
||||
describe("layerStack Escape coordination — live useLayer + real dispatch (UX-8)", () => {
|
||||
it("one Escape closes only the topmost popover; a second closes the player", () => {
|
||||
const playerClose = vi.fn();
|
||||
const popoverClosed = vi.fn();
|
||||
const { queryByTestId } = render(
|
||||
<>
|
||||
<Player onClose={playerClose} />
|
||||
<Popover onClosed={popoverClosed} />
|
||||
</>,
|
||||
);
|
||||
// Player registers first, popover second → popover is topmost.
|
||||
expect(queryByTestId("popover")).not.toBeNull();
|
||||
|
||||
// First Escape: popover acts (stamps + flushSync-closes, removing its layer synchronously), player
|
||||
// defers on the stamp even though the stack now shows nothing above it.
|
||||
const first = dispatchEscape();
|
||||
expect(escapeHandled(first)).toBe(true);
|
||||
expect(popoverClosed).toHaveBeenCalledOnce();
|
||||
expect(queryByTestId("popover")).toBeNull();
|
||||
expect(playerClose).not.toHaveBeenCalled();
|
||||
|
||||
// Second Escape: nothing above the player, unstamped → the player closes.
|
||||
const second = dispatchEscape();
|
||||
expect(escapeHandled(second)).toBe(false);
|
||||
expect(playerClose).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("with no layer above, a lone player closes on the first Escape", () => {
|
||||
const playerClose = vi.fn();
|
||||
render(<Player onClose={playerClose} />);
|
||||
dispatchEscape();
|
||||
expect(playerClose).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user