peter
5051809940
UX-8: one Escape must close only the topmost layer, never both an open popover AND the player beneath it. The popover's Escape listener is on `document` and fires before the player's on `window`; closing the popover flushes its layer off the shared stack synchronously, so the player's `anyLayerAbove()` lookup read "nothing above" and closed too. Add a consumed-event flag to layerStack (`markEscapeHandled`/`escapeHandled`): the acting topmost layer stamps the shared KeyboardEvent, and both players check the stamp before the (now-mutated) stack. Immune to listener order and mid-dispatch stack mutation. Cover with layerStack.dom.test.ts. UX-7: the E4 `.reveal` sweep already covered the card action rows; two residual hover-only traps remained — invisible-but-tappable/Tab-focusable on touch. Add a group-agnostic `.reveal-hover` recipe (display:none under `(hover: none)`) paired with each site's own hover/focus-within reveal: - VideoCard play/Continue/Restart overlay: also reveal on group-focus-within (keyboard), and drop it on touch — the card tap already plays/resumes, so no permanent scrim over every mobile thumbnail. - Carousel prev/next arrows: gone on touch, where the strip swipes.
80 lines
3.2 KiB
TypeScript
80 lines
3.2 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,
|
|
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);
|
|
});
|
|
});
|