Files
siftlode/frontend/src/lib/history.dom.test.ts
T
peter 20e17c0ccb fix(e10): review round 1 — the boundary that did not bind, and 14 more
The epic-end review found 32 verified issues. This is the first fix round:
the three highs, the user-facing mediums, and the tests that were passing
for the wrong reason. Docs and the remaining low-severity tail are next.

The boundary rule was the worst of them. Both `no-restricted-imports`
regexes were written in plain "..." literals, where `\.` is an unknown
escape that collapses to the any-char `.` — so the rule read like a
boundary and matched far less than one. `../../lib/api` (the barrel every
feature component imports) and `../../components/Feed` (the same file
`../Feed` names) both sailed through a green gate. String.raw, a third
pattern for the second spelling, and a vitest over ESLint.lintText so a
rule that silently matches nothing can never look like a clean codebase
again.

closeThenNavigate waited for exactly one reclaim popstate. With a second
Back-closable layer still mounted — a palette opened over the player —
the navigation unmounted that layer too, and its reconcile backed over
the entry the navigation had just pushed: "go to Channels" closed the
player and landed on the feed. It now waits for the LAST programmatic
back, then adopts the entries the navigation buried, and falls back after
250ms rather than hanging if no popstate is ever coming.

The rail's inline rename died on arrival. With no domReference, floating-
ui's returnFocus falls through to a module-global stack and restores some
unrelated element from an earlier popover — blurring the field into its
own commit-and-close, from the second popover of a session onward. The
context menu now owns the restore: back to the row, but only if the row
is still there and nothing else has claimed focus.

Also:
- Backspace under the cheat sheet stopped Plex playback and dropped to
  the grid; `back` now defers to any layer above, like `escape` beside it.
- A move invalidated only the source list, so the target kept serving
  pre-move contents for 30s while the rail already counted N+1.
- Every move error a user can reach is a 404, which `req` leaves to the
  caller — so the row snapped back in silence and the shipped moveErrors
  strings were unreachable.
- swallowClick armed on long-press but was disarmed only by the click it
  was armed for; any press that produced no click ate the next tap on the
  whole list.
- The context menu stayed parked over whatever row scrolled under it.
- overflow-clip on the chip lanes ate the focus ring's top and bottom.
- "Why is this off" rode on disabled rows, which show no tooltip and take
  no focus: a new `inert` item keeps the reason readable.
- Shift+F10 was advertised app-wide but wired only on Playlists; it has
  its own scope now, and useContextMenu matches through the shared parser
  instead of its own literals.
- Mod+K could not close the palette, `h` machine-gunned on auto-repeat,
  and both were bound above the branch that mounts their overlays.
- Palette options were in the tab order under aria-activedescendant, and
  a scroll-synthesised mousemove stole the keyboard selection.
- The OSD live region was created together with its own text.
- Ending a rename stranded focus on <body>.
- The Button variants' hover halves are `enabled:` — the one migrated
  site that had guarded its own lost the guard on the way in.
- HU builtinLocked named the list "Watch later" in a Hungarian sentence.

Tests: the chip-lane contract counted per FILE, so a second hand-rolled
lane in an already-compliant file was invisible; the closeThenNavigate
ordering test only passed on a popstate left behind by its neighbours and
failed alone, so it moved to its own file.
2026-09-05 23:52:29 +02:00

166 lines
6.6 KiB
TypeScript

// @vitest-environment jsdom
//
// Component-test lane pilot: the useBackToExit "back-ladder". This is the first jsdom/renderHook
// test in the repo (the pure-node lane can't touch window.history / popstate). It exercises the
// module-level overlay stack in history.ts through its only observable effect — which onExit fires.
//
// A GENUINE user Back is simulated by dispatching `popstate` directly: the real-Back branch in
// history.ts does NOT call history.back() (it only decrements its own historyDepth to match the
// entry the browser already dropped), so a raw popstate reproduces it deterministically without
// depending on jsdom's async back() semantics. Each test closes its layers via genuine Backs, so
// no reconcile-driven history.back() runs and the shared module state stays clean between tests.
import { describe, it, expect, vi, afterEach } from "vitest";
import { renderHook, act } from "@testing-library/react";
import { closeThenNavigate, useBackToClose, useBackToExit } from "./history";
// Let the mount effect's coalesced reconcile microtask settle.
const settle = () =>
act(async () => {
await Promise.resolve();
});
// A genuine user Back (browser already dropped the entry; popstate arrives with no pending back()).
const userBack = () =>
act(() => {
window.dispatchEvent(new PopStateEvent("popstate", { state: window.history.state }));
});
afterEach(() => {
vi.clearAllMocks();
});
describe("useBackToExit back-ladder", () => {
it("nested active layers peel off one genuine Back at a time, topmost first (LIFO)", async () => {
const outer = vi.fn();
const inner = vi.fn();
const ro = renderHook(() => useBackToExit(true, outer));
await settle();
const ri = renderHook(() => useBackToExit(true, inner));
await settle();
await userBack(); // peels the topmost (inner) only
expect(inner).toHaveBeenCalledTimes(1);
expect(outer).not.toHaveBeenCalled();
await userBack(); // now peels the outer
expect(outer).toHaveBeenCalledTimes(1);
expect(inner).toHaveBeenCalledTimes(1);
ro.unmount();
ri.unmount();
});
it("stacks above a useBackToClose layer — Back closes the exit layer before the close layer", async () => {
const close = vi.fn();
const exit = vi.fn();
const rc = renderHook(() => useBackToClose(close));
await settle();
const re = renderHook(() => useBackToExit(true, exit));
await settle();
await userBack();
expect(exit).toHaveBeenCalledTimes(1);
expect(close).not.toHaveBeenCalled();
await userBack();
expect(close).toHaveBeenCalledTimes(1);
rc.unmount();
re.unmount();
});
it("an inactive layer registers nothing — Back falls straight through to the layer below it", async () => {
const below = vi.fn();
const gated = vi.fn();
const rb = renderHook(() => useBackToExit(true, below));
await settle();
const rg = renderHook(() => useBackToExit(false, gated)); // gated: not active → no layer
await settle();
await userBack();
expect(gated).not.toHaveBeenCalled();
expect(below).toHaveBeenCalledTimes(1);
rb.unmount();
rg.unmount();
});
});
describe("closeThenNavigate", () => {
// The bug it exists for: an overlay's unmount reclaims its Back-to-close entry with a programmatic
// history.back(). A pushState made before that lands is undone by it, so a dialog command that
// navigates does nothing but close. PlayerModal hit it once; the command palette hit it again.
it("closes first and navigates only after the reclaim lands", () => {
const order: string[] = [];
closeThenNavigate(
() => order.push("close"),
() => order.push("navigate"),
);
expect(order).toEqual(["close"]);
window.dispatchEvent(new PopStateEvent("popstate"));
expect(order).toEqual(["close", "navigate"]);
});
it("navigates exactly once, however many pops follow", () => {
const navigate = vi.fn();
closeThenNavigate(() => {}, navigate);
window.dispatchEvent(new PopStateEvent("popstate"));
window.dispatchEvent(new PopStateEvent("popstate"));
expect(navigate).toHaveBeenCalledTimes(1);
});
// E10 review round 1. Waiting for ONE reclaim is not enough when a SECOND Back-closable layer is
// still mounted and the navigation is what unmounts it: that layer's own reconcile fired another
// history.back(), which popped the entry the navigation had just pushed and bounced the user back
// where they started (palette opened over the player → "go to Channels" landed on the feed).
//
// NOTE this one may NOT use the raw-popstate shortcut the rest of the file uses: a synthetic
// popstate is read as a GENUINE Back, which pops the lower layer off the stack and decrements
// historyDepth by itself — so the test would pass with or without the fix. The bug only appears
// on the programmatic path, so back() is stubbed to land its own popstate instead.
it("adopts the entries of layers the navigation itself unmounts", async () => {
const back = vi.spyOn(window.history, "back").mockImplementation(() => {
window.dispatchEvent(new PopStateEvent("popstate", { state: window.history.state }));
});
try {
const player = renderHook(() => useBackToClose(() => {}));
await settle();
const palette = renderHook(() => useBackToClose(() => {}));
await settle();
expect(back).not.toHaveBeenCalled(); // both layers hold an entry; nothing reclaimed yet
const navigate = vi.fn(() => window.history.pushState({ sfPage: "channels" }, ""));
await act(async () => {
closeThenNavigate(() => palette.unmount(), navigate);
await Promise.resolve(); // let the palette's own reclaim reconcile run
});
expect(navigate).toHaveBeenCalledTimes(1);
expect(back).toHaveBeenCalledTimes(1); // the palette's entry, and only that
player.unmount(); // …which is what the navigation does to the layer below
await settle();
expect(back).toHaveBeenCalledTimes(1); // must NOT back over the entry navigate just pushed
} finally {
back.mockRestore();
}
});
// Without this the command is not late, it never happens: an overlay closed in the same tick it
// opened never pushed an entry, so no reclaim popstate is ever coming.
it("navigates anyway when no reclaim popstate ever arrives", () => {
vi.useFakeTimers();
try {
const navigate = vi.fn();
closeThenNavigate(() => {}, navigate);
expect(navigate).not.toHaveBeenCalled();
vi.advanceTimersByTime(300);
expect(navigate).toHaveBeenCalledTimes(1);
} finally {
vi.useRealTimers();
}
});
});