Files
siftlode/frontend/src/lib/layerStack.dom.test.ts
T
peter 4aaaaa7860 chore(comments): drop epic, sprint and review-round ids from the code
The code cited its plan instead of saying why: `E28 review J4`, `(R6 S3b)`, `S1 justified…`,
`deep-review round 5`. That plan lives in the private ops repo, so for every reader of this one the
citation was noise, and a sentence whose subject was the plan step ("round 2 showed…") hid its
lesson behind an id. Each one is gone and the "why" kept: a bare citation is deleted, a sentence
carrying a lesson is reworded around it ("a later attempt showed…").

Measured (`scripts/audit_metrics.py`, product code): 652 lines in 270 files carried an epic code by
the definition the next commit holds (507 / 241 by the old, two-digit-only one) -> 0. Across all
code, tests, scripts and configs: ~1100 epic-code lines plus ~320 sprint/round ids.

Behaviour unchanged, checked mechanically: every changed .py file has the same AST once docstrings
are blanked; css/sh/toml/yml/json files are identical once comments are stripped; the only
non-comment changes in TS/JS are test titles, one description string in a contract table, and five
ESLint messages, whose text is all the gates snapshot diff holds. Not touched: Markdown (docs and
the changelog keep their history), Alembic migrations, TV episode notation (`S2 · E5`), DESIGN.md
ids (`C1`, `B4`, `S3h`), tracker ids (`SEC-16`, `DX-17`).

One stale claim fixed on the way: test_hls_reaper's docstring said the player checkpoints "every
10 s whether or not it is playing", which plex/stream.py calls wrong; it now says what the code does.
2026-10-05 00:19:39 +02:00

119 lines
4.9 KiB
TypeScript

// @vitest-environment jsdom
//
// Guards the UX-8 fix: one Escape must close only the TOPMOST layer, never both an open popover AND
// the player beneath it. The bug was that the popover's Escape listener (on `document`) fires before
// the player's (on `window`), and closing the popover flushes its layer off the stack synchronously —
// so by the time the player's `anyLayerAbove()` lookup runs, the stack already reads "nothing above"
// and the player closes too. The fix stamps the shared KeyboardEvent when a layer acts; the player
// reads the stamp instead of the (now-mutated) stack. This test reproduces that exact ordering with
// the stack primitives — the layer removes itself between the two listeners, mimicking the flush.
import { describe, it, expect, vi, afterEach } from "vitest";
import {
pushLayer,
removeLayer,
anyLayerAbove,
anyBlockingLayer,
anyBlockingLayerAbove,
isTopmost,
markEscapeHandled,
escapeHandled,
} from "./layerStack";
// Keep the module-level stack clean between cases (ids never repeat, but membership must not leak).
const opened: number[] = [];
function open(): number {
const id = pushLayer();
opened.push(id);
return id;
}
afterEach(() => {
while (opened.length) removeLayer(opened.pop()!);
});
describe("layerStack Escape coordination (UX-8)", () => {
it("a player defers even when the acting layer is popped mid-dispatch", () => {
const playerId = open();
const popoverId = open();
const playerClose = vi.fn();
// Popover: the `document` listener from useLayer — topmost acts, stamps the event, and (the
// regression) its close synchronously removes the layer before the window listener runs.
const onDocumentKey = (e: KeyboardEvent) => {
if (e.key === "Escape" && isTopmost(popoverId)) {
markEscapeHandled(e);
removeLayer(popoverId); // the synchronous React flush the real close triggers
}
};
// Player: the `window` listener — fires AFTER the document one, reading an already-mutated stack.
const onWindowKey = (e: KeyboardEvent) => {
if (e.key !== "Escape") return;
if (escapeHandled(e)) return; // the fix
if (anyLayerAbove(playerId)) return; // the old, now-insufficient guard
playerClose();
};
const e = new KeyboardEvent("keydown", { key: "Escape" });
onDocumentKey(e);
onWindowKey(e);
// The stamp holds even though the stack no longer shows a layer above the player...
expect(escapeHandled(e)).toBe(true);
expect(anyLayerAbove(playerId)).toBe(false); // ...which is exactly why anyLayerAbove alone failed
expect(playerClose).not.toHaveBeenCalled();
});
it("a plain Escape with no layer above still closes the player", () => {
const playerId = open();
const playerClose = vi.fn();
const e = new KeyboardEvent("keydown", { key: "Escape" });
if (!escapeHandled(e) && !anyLayerAbove(playerId)) playerClose();
expect(playerClose).toHaveBeenCalledOnce();
});
it("stamps are per-event: a fresh Escape is not treated as already-handled", () => {
const first = new KeyboardEvent("keydown", { key: "Escape" });
markEscapeHandled(first);
const second = new KeyboardEvent("keydown", { key: "Escape" });
expect(escapeHandled(second)).toBe(false);
});
});
// A layer answers TWO questions, and a review found they are not the same one: "do the page's own
// keys stay quiet?" and "do I take the Tab trap from the dialog below?". The recorder's step prompt
// is the case that separates them — a field inside whatever dialog the pill is docked in, so it
// owns the keyboard but its two controls are no trap for that dialog's many.
describe("owning the keyboard and owning the Tab trap", () => {
it("an inline prompt silences the page and still leaves the dialog its trap", () => {
const dialog = open();
const prompt = pushLayer({ trapsTab: false });
opened.push(prompt);
expect(anyBlockingLayerAbove(dialog)).toBe(false); // the dialog keeps trapping Tab...
// ...and the page's own keys are quiet by the PROMPT's doing, not the stand-in dialog's: the
// dialog is non-passive too, so asserting this with it still on the stack asserted nothing.
removeLayer(dialog);
expect(anyBlockingLayer()).toBe(true); // `j`, `g h` and the rest stay off the page behind
});
it("a real dialog above does take the trap from the one below", () => {
const dialog = open();
const above = open();
expect(anyBlockingLayerAbove(dialog)).toBe(true);
expect(anyBlockingLayerAbove(above)).toBe(false);
});
it("a passive layer takes neither: a hover card must silence nothing", () => {
const card = pushLayer({ passive: true });
opened.push(card);
expect(anyBlockingLayer()).toBe(false); // `j` still moves the list under a resting pointer
const dialog = open();
const over = pushLayer({ passive: true });
opened.push(over);
expect(anyBlockingLayerAbove(dialog)).toBe(false); // ...and it is no trap either
});
});