fix(ui): every full-screen overlay hosts its own toasts, above the players' controls, without re-announcing them

Deep-review round 3 of bug hunt 2026-09 (the round-2 overlay toast host, finished).

- Inside an overlay the toast stack sits above the bottom band (bottom-24, the UpNextCard rung) and
  lets clicks between cards through: at the app's own spot it covered the Plex transport row and
  PlayerModal's docked bar and took their clicks while a toast was up (measured: play/pause, ±10,
  stop, prev/next, mute all hit the toast).
- ui/Modal and the onboarding wizard mount an overlay host too, so a toast raised under any dialog
  is readable and a click on it no longer closes the dialog; the Modal's Tab trap spans the whole
  dialog, so a toast's action and dismiss are keyboard-reachable. A test now enforces the
  convention: a component that covers the viewport at z-overlay must mount <Toaster overlay />.
- PlayerModal's host moves into the stage in true fullscreen (outside the element holding the lock it
  was painted under it).
- A host that takes over shows the toasts it inherits silently (no second role=alert, its region
  aria-live=off until they are in), and each countdown bar starts as far in as its toast already is
  instead of restarting while the real dismiss timer does not.
- The overlay-host registry uses lib/store instead of a hand-rolled listener triad.
This commit is contained in:
2026-09-11 18:04:54 +02:00
parent 5c5cabbe5a
commit 33fe895539
9 changed files with 228 additions and 41 deletions
@@ -7,6 +7,7 @@ import { api, type Me } from "../lib/api";
import { beginGrant, endOnboarding } from "../lib/onboarding";
import { Button } from "./ui/Button";
import Overlay from "./ui/Overlay";
import Toaster from "./ui/Toaster";
// A first-login wizard that walks the user through granting YouTube access one scope at a
// time, each with a plain explanation and an up-front heads-up about Google's "unverified
@@ -262,6 +263,9 @@ export default function OnboardingWizard({ me, onClose }: { me: Me; onClose: ()
<Dots index={stepIndex} total={3} />
</div>
</div>
{/* Toasts raised while the wizard is up (the connection notice among them) show above its
scrim, not under it — see ui/Toaster. */}
<Toaster overlay />
</div>
</Overlay>
);
@@ -270,4 +270,23 @@ describe("PlayerModal — toasts raised while it is open", () => {
act(() => void notify({ level: "error", message: "Couldn't remove the video." }));
expect(screen.getByTestId("player-modal").textContent).toContain("Couldn't remove the video.");
});
it("shows them inside the stage in true fullscreen — the element holding the lock", async () => {
// Anything outside the fullscreen element is painted under it. The player's host sat beside the
// card, so in fullscreen a lost connection was reported to nobody.
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
const A = vid("A", 0);
renderModal(
<PlayerModal video={A} queue={[A]} startIndex={0} onClose={() => {}} onState={() => {}} />,
);
const stage = document.querySelector(".player-stage")!;
Object.defineProperty(document, "fullscreenElement", { configurable: true, get: () => stage });
try {
act(() => void document.dispatchEvent(new Event("fullscreenchange")));
act(() => void notify({ level: "error", message: "Connection lost" }));
await waitFor(() => expect(stage.textContent).toContain("Connection lost"));
} finally {
delete (document as { fullscreenElement?: unknown }).fullscreenElement;
}
});
});
+4 -2
View File
@@ -586,6 +586,7 @@ export default function PlayerModal({
the exit button also re-arms our shortcuts. Add-to-playlist works in true fullscreen (its
popover portals INTO the fullscreen element); download stays hidden there because it opens
a Modal, which still portals to <body> — outside the fullscreen element. */}
{isFullscreen && <Toaster overlay />}
{(maximized || isFullscreen) && (
<div
onClick={(e) => e.stopPropagation()}
@@ -962,8 +963,9 @@ export default function PlayerModal({
)}
</div>
{/* Toasts raised while the player is open (a failed Remove, a lost connection) — the app's own
Toaster is underneath this scrim. See ui/Toaster. */}
<Toaster overlay />
Toaster is underneath this scrim. See ui/Toaster. In true fullscreen it moves into the
stage, the element holding the lock: out here it was painted under it, i.e. not at all. */}
{!isFullscreen && <Toaster overlay />}
</div>,
document.body,
);
@@ -20,7 +20,11 @@ vi.mock("react-i18next", async (importOriginal) => {
});
vi.mock("../lib/api", () => ({ api: { uploadDownload: h.uploadDownload } }));
vi.mock("../lib/downloads", () => ({ invalidateDownloads: vi.fn() }));
vi.mock("../lib/notifications", () => ({ notify: h.notify }));
// The rest of the module stays real: the dialog is a ui/Modal, which hosts its own toasts.
vi.mock("../lib/notifications", async (importOriginal) => ({
...(await importOriginal<typeof import("../lib/notifications")>()),
notify: h.notify,
}));
vi.mock("../lib/nav", () => ({ navigateTo: vi.fn() }));
import UploadDialog from "./UploadDialog";
@@ -20,6 +20,7 @@ vi.mock("react-i18next", async (importOriginal) => {
vi.mock("../../lib/history", () => ({ useBackToClose: () => {} }));
import Modal from "./Modal";
import { clearAll, notify } from "../../lib/notifications";
// Fire a keydown on window (where Modal installs its handler), inside act.
const press = (key: string, shiftKey = false) =>
@@ -91,6 +92,32 @@ describe("Modal focus trap (offsetParent stubbed 'visible')", () => {
expect(document.activeElement).toBe(screen.getByText("beta"));
});
it("Tab reaches a toast shown inside the dialog, and wraps from it", () => {
// The dialog's toast host sits beside the panel; trapped to the panel, a toast's Undo or dismiss
// could not be reached from the keyboard at all.
render(
<Modal title="t" onClose={() => {}}>
<Body />
</Modal>,
);
act(() => void notify({ level: "info", message: "Saved" }));
try {
const dismissToast = screen.getByTitle("notifications.dismiss");
// The panel's last control is no longer the trap's edge: Tab carries on (natively) to the toast.
screen.getByText("beta").focus();
press("Tab");
expect(document.activeElement).toBe(screen.getByText("beta")); // not wrapped back to the top
// The toast is the edge now: past it, Tab wraps to the first control, and Shift+Tab reaches it.
dismissToast.focus();
press("Tab");
expect(document.activeElement).toBe(closeBtn());
press("Tab", true);
expect(document.activeElement).toBe(dismissToast);
} finally {
act(() => clearAll());
}
});
it("Tab pulls focus back inside when it has escaped to the page behind", () => {
const outside = document.createElement("button");
document.body.appendChild(outside);
@@ -189,3 +216,22 @@ describe("Modal focus trap (raw jsdom — every offsetParent null)", () => {
expect(document.activeElement).toBe(panelEl());
});
});
describe("Modal — toasts raised while it is open", () => {
afterEach(() => {
cleanup();
act(() => clearAll());
});
it("shows them inside the dialog, above its scrim", () => {
// The app's Toaster is in a stacking context under this body-level portal: a failed share or
// rename was drawn under the scrim, and clicking it landed on the backdrop and closed the dialog.
render(
<Modal title="Share" onClose={() => {}}>
<Body />
</Modal>,
);
act(() => void notify({ level: "error", message: "Couldn't share it." }));
expect(screen.getByRole("dialog").textContent).toContain("Couldn't share it.");
});
});
+12 -2
View File
@@ -6,6 +6,7 @@ import { useBackToClose } from "../../lib/history";
import { pushLayer, removeLayer, isTopmost } from "../../lib/layerStack";
import { useFullscreenElement } from "../../lib/fullscreen";
import { Button } from "./Button";
import Toaster from "./Toaster";
// Tabbable elements inside the dialog, in DOM order. `:not([tabindex="-1"])` drops programmatically
// focusable-only nodes; the visibility filter (offsetParent) drops display:none ones (e.g. a
@@ -56,6 +57,7 @@ export default function Modal({
// drag that begins inside the dialog (e.g. selecting text in an input) and is released out on
// the backdrop wrongly dismisses — the click event bubbles to the common ancestor (the backdrop).
const pressedOnBackdrop = useRef(false);
const rootRef = useRef<HTMLDivElement>(null);
// Keep a stable handler across renders so the stack id is assigned once per mount.
const onCloseRef = useRef(onClose);
@@ -74,7 +76,10 @@ export default function Modal({
if (e.key === "Tab") {
const panel = panelRef.current;
if (!panel) return;
const items = focusablesIn(panel);
// The whole dialog, not just the panel: its toast host sits beside the panel (see ui/Toaster),
// and a toast's Undo or dismiss must be reachable from the keyboard like everything else.
const scope = rootRef.current ?? panel;
const items = focusablesIn(scope);
const first = items[0];
const last = items[items.length - 1];
if (!first || !last) {
@@ -82,7 +87,7 @@ export default function Modal({
return;
}
const active = document.activeElement;
if (!panel.contains(active) || active === panel) {
if (!scope.contains(active) || active === panel) {
// Focus escaped the dialog, OR sits on the panel container itself (tabIndex=-1, reached by
// clicking dialog dead space) — Node.contains(self) is true, so `active === panel` must be
// checked explicitly or Shift+Tab from the panel would leak to the background. Pull it back.
@@ -119,6 +124,7 @@ export default function Modal({
const top = placement === "top";
return createPortal(
<div
ref={rootRef}
className={`fixed inset-0 z-overlay flex bg-black/70 overlay-backdrop ${
drawer
? "justify-end"
@@ -161,6 +167,10 @@ export default function Modal({
</div>
<div className="px-5 pb-5">{children}</div>
</div>
{/* Toasts raised while a dialog is open show inside it: the app's own host is under this
scrim, where a failed share or rename was unreadable and a click on it closed the dialog
(see ui/Toaster). */}
<Toaster overlay />
</div>,
fsRoot ?? document.body,
);
@@ -20,7 +20,7 @@ afterEach(() => {
act(() => clearAll());
});
const hosts = () => Array.from(document.querySelectorAll("[aria-live='polite']"));
const hosts = () => Array.from(document.querySelectorAll("[aria-live]"));
describe("Toaster — one host shows the toasts", () => {
it("moves the toasts into an open overlay, and back when it closes", () => {
@@ -41,6 +41,7 @@ describe("Toaster — one host shows the toasts", () => {
expect(screen.getByTestId("app").textContent).not.toContain("Couldn't remove the video.");
// Both live regions stay mounted — a polite region has to pre-exist to be announced.
expect(hosts()).toHaveLength(2);
expect(hosts().map((h) => h.getAttribute("aria-live"))).toEqual(["off", "polite"]);
overlay.unmount();
expect(screen.getByTestId("app").textContent).toContain("Couldn't remove the video.");
@@ -59,3 +60,43 @@ describe("Toaster — one host shows the toasts", () => {
expect(backdrop).not.toHaveBeenCalled();
});
});
describe("Toaster — handing the toasts to another host", () => {
it("does not announce the toasts it inherits again, only the ones that come after", () => {
// The new host gets them as NEW nodes: a screen reader read every one out again, an error
// assertively, each time a player (or any dialog) opened or closed.
render(<Toaster />);
act(() => void notify({ level: "error", message: "old news" }));
render(<Toaster overlay />);
const old = screen.getByText("old news").closest(".toast-card")!;
expect(old.getAttribute("role")).toBeNull();
act(() => void notify({ level: "error", message: "fresh" }));
expect(screen.getByText("fresh").closest(".toast-card")!.getAttribute("role")).toBe("alert");
});
it("keeps the countdown where it was, instead of starting it again from full", () => {
// The bar is a CSS animation on the node; the dismiss timer was armed once, at notify time. A
// new node restarted the bar, and the toast vanished with it still two-thirds full.
const t0 = Date.now();
const now = vi.spyOn(Date, "now").mockReturnValue(t0);
render(<Toaster />);
act(() => void notify({ level: "info", message: "saved" }));
now.mockReturnValue(t0 + 4000);
render(<Toaster overlay />);
const bar = screen.getByText("saved").closest(".toast-card")!.lastElementChild as HTMLElement;
expect(bar.style.animation).toContain("-4000ms");
});
it("keeps the players' control band free inside an overlay", () => {
// At the app's own spot (bottom-4) the stack sat on the Plex transport row and PlayerModal's
// docked bar, above them at z-overlay, and took their clicks for as long as a toast was up.
render(<Toaster overlay />);
act(() => void notify({ level: "error", message: "boom" }));
const host = hosts()[0]!;
expect(host.className).toContain("bottom-24");
expect(host.className).toContain("pointer-events-none");
expect(screen.getByText("boom").closest(".toast-card")!.className).toContain(
"pointer-events-auto",
);
});
});
@@ -0,0 +1,33 @@
// Every full-screen overlay hosts its own toasts (see ui/Toaster).
//
// The app's Toaster lives in the content column, a stacking context: anything that covers the
// viewport at z-overlay paints over it, so a toast raised while that overlay is open is unreadable and
// a click on it lands on the overlay's backdrop. The fix is a convention — the overlay mounts
// `<Toaster overlay />` inside itself — and a convention stated in a comment is one the next overlay
// forgets (deep-review round 3 found every ui/Modal and the onboarding wizard without one). So it is
// checked here: a component that renders a viewport-covering z-overlay layer mounts an overlay host.
import { describe, it, expect } from "vitest";
const sources = import.meta.glob("../**/*.tsx", { query: "?raw", import: "default", eager: true });
// Covers the viewport for a moment with nothing a toast could be about (a lazy chunk's black
// placeholder while the player loads).
const EXEMPT = new Set(["../PlexBrowse.tsx"]);
describe("full-screen overlays host their own toasts", () => {
it("every one that covers the viewport at z-overlay mounts <Toaster overlay />", () => {
const missing = Object.entries(sources)
.filter(([path]) => !path.endsWith(".test.tsx") && !EXEMPT.has(path))
.filter(([, src]) => /fixed inset-0 z-overlay/.test(src as string))
.filter(([, src]) => !(src as string).includes("<Toaster overlay />"))
.map(([path]) => path);
expect(missing).toEqual([]);
});
it("finds the overlays it is meant to check", () => {
const covered = Object.entries(sources).filter(([, src]) =>
/fixed inset-0 z-overlay/.test(src as string),
);
expect(covered.length).toBeGreaterThanOrEqual(3); // Modal, PlayerModal, the wizard at least
});
});
+63 -35
View File
@@ -1,7 +1,8 @@
import { useEffect, useId, useSyncExternalStore } from "react";
import { useEffect, useId, useMemo, useState, useSyncExternalStore } from "react";
import { useTranslation } from "react-i18next";
import { AlertCircle, AlertTriangle, CheckCircle2, Info, X, XCircle } from "lucide-react";
import { dismiss, getActiveToasts, subscribe, type NotifLevel } from "../../lib/notifications";
import { createStore } from "../../lib/store";
export const LEVEL_STYLE: Record<NotifLevel, { icon: typeof Info; color: string; bar: string }> = {
info: { icon: Info, color: "text-info", bar: "bg-info" },
@@ -12,22 +13,17 @@ export const LEVEL_STYLE: Record<NotifLevel, { icon: typeof Info; color: string;
};
// Overlay hosts. The app's own Toaster sits in the content column, and that column is a stacking
// context (its `@container` implies layout containment) — so a full-screen player, portalled to
// <body> at the same z-overlay, painted over every toast: a failure raised while watching was
// drawn under an 80% black, blurred scrim, and a click on it closed the player. Such an overlay
// mounts its OWN `<Toaster overlay />` inside itself instead, and exactly one host shows the
// toasts: the most recently mounted overlay, else the app's. Same component, same look — and inside
// the overlay's aria-modal dialog, where a screen reader can still reach it.
let overlayHosts: string[] = [];
const hostListeners = new Set<() => void>();
const subscribeHosts = (l: () => void) => {
hostListeners.add(l);
return () => void hostListeners.delete(l);
};
const topHost = () => overlayHosts[overlayHosts.length - 1] ?? null;
const setHosts = (next: string[]) => {
overlayHosts = next;
hostListeners.forEach((l) => l());
// context (its `@container` implies layout containment) — so anything portalled to <body> at the
// same z-overlay painted over every toast: a failure raised while watching was drawn under an 80%
// black, blurred scrim, and a click on it closed the player. So every full-screen overlay mounts its
// OWN `<Toaster overlay />` inside itself — ui/Modal does it for every dialog, the players for
// themselves — and exactly one host shows the toasts: the most recently mounted overlay, else the
// app's. Same component, same look — and inside the overlay's aria-modal dialog, where a screen
// reader can still reach it.
const overlayHosts = createStore<readonly string[]>([]);
const topHost = () => {
const hosts = overlayHosts.get();
return hosts[hosts.length - 1] ?? null;
};
export default function Toaster({ overlay = false }: { overlay?: boolean }) {
@@ -35,23 +31,43 @@ export default function Toaster({ overlay = false }: { overlay?: boolean }) {
const id = useId();
useEffect(() => {
if (!overlay) return;
setHosts([...overlayHosts, id]);
return () => setHosts(overlayHosts.filter((h) => h !== id));
overlayHosts.set((hosts) => [...hosts, id]);
return () => overlayHosts.set((hosts) => hosts.filter((h) => h !== id));
}, [overlay, id]);
const top = useSyncExternalStore(subscribeHosts, topHost, topHost);
const top = useSyncExternalStore(overlayHosts.subscribe, topHost, topHost);
const active = useSyncExternalStore(subscribe, getActiveToasts, getActiveToasts);
const toasts = top === (overlay ? id : null) ? active : [];
const isTop = top === (overlay ? id : null);
const toasts = isTop ? active : [];
// A host that takes over (an overlay opening, or the app's own once it closes) receives the toasts
// that were already up as NEW nodes — and a screen reader announced every one of them again, an
// error assertively. So the ones it inherits are shown silently: no `role="alert"`, and the region
// stays `aria-live="off"` until they are in, turning polite (for the toasts that come after) once
// they have been rendered.
const inherited = useMemo(
() => new Set(isTop ? getActiveToasts().map((n) => n.id) : []),
[isTop],
);
const [armed, setArmed] = useState(false);
useEffect(() => setArmed(isTop), [isTop]);
return (
// Stable, always-mounted polite live region — a polite region must PRE-EXIST in the DOM for a
// screen reader to announce content later inserted into it (a region created together with its
// first message is unreliably announced). `aria-live` (not role="status") on purpose:
// role="status" implies aria-atomic="true", which would re-read the WHOLE stack on every toast;
// aria-live defaults atomic=false, so only the newly-added toast is spoken. Error/fatal toasts
// additionally carry role="alert", which announces assertively on insertion regardless.
// Stable, always-mounted live region — a polite region must PRE-EXIST in the DOM for a screen
// reader to announce content later inserted into it (a region created together with its first
// message is unreliably announced). `aria-live` (not role="status") on purpose: role="status"
// implies aria-atomic="true", which would re-read the WHOLE stack on every toast; aria-live
// defaults atomic=false, so only the newly-added toast is spoken. Error/fatal toasts additionally
// carry role="alert", which announces assertively on insertion regardless.
//
// Inside an overlay it sits ABOVE the bottom band: that is where the players keep their control
// bars (the Plex transport row, PlayerModal's docked bar), and the toast stack — on top of them at
// z-overlay, and catching the click — took play/pause, ±10, stop, prev/next and mute for as long
// as a toast was up (the connection notice: until the server answered). The same rung UpNextCard
// uses for the same reason. Clicks between the cards go through to what is underneath.
<div
aria-live="polite"
className="absolute bottom-4 left-4 z-overlay flex flex-col gap-2 w-80 max-w-[calc(100vw-2rem)]"
aria-live={isTop && armed ? "polite" : "off"}
className={`absolute left-4 z-overlay flex flex-col gap-2 w-80 max-w-[calc(100vw-2rem)] ${
overlay ? "bottom-24 pointer-events-none" : "bottom-4"
}`}
// Inside an overlay, a click on a toast is not a click on the overlay's backdrop.
onClick={overlay ? (e) => e.stopPropagation() : undefined}
>
@@ -61,8 +77,8 @@ export default function Toaster({ overlay = false }: { overlay?: boolean }) {
return (
<div
key={toast.id}
role={urgent ? "alert" : undefined}
className="toast-card glass relative overflow-hidden rounded-xl px-3 py-3 flex items-start gap-3 animate-[popIn_0.16s_ease]"
role={urgent && !inherited.has(toast.id) ? "alert" : undefined}
className="toast-card glass pointer-events-auto relative overflow-hidden rounded-xl px-3 py-3 flex items-start gap-3 animate-[popIn_0.16s_ease]"
>
<Icon className={`w-5 h-5 shrink-0 mt-0.5 ${color}`} />
<div className="min-w-0 flex-1">
@@ -95,10 +111,8 @@ export default function Toaster({ overlay = false }: { overlay?: boolean }) {
<X className="w-4 h-4" />
</button>
{toast.duration && (
<div
className={`absolute left-0 bottom-0 h-0.5 w-full origin-left ${bar}`}
style={{ animation: `toastbar ${toast.duration}ms linear forwards` }}
/>
// Keyed by `ts`: a coalesced repeat re-arms the dismiss timer, so its bar restarts too.
<CountdownBar key={toast.ts} since={toast.ts} duration={toast.duration} bar={bar} />
)}
</div>
);
@@ -106,3 +120,17 @@ export default function Toaster({ overlay = false }: { overlay?: boolean }) {
</div>
);
}
/** The auto-dismiss countdown, started as far in as the toast already is. A toast handed to another
* host is a new node, and its bar used to start again from full while the dismiss timer — armed
* once, at notify time — did not: the toast vanished with the bar still two-thirds full. The offset
* is taken once, at mount; recomputed on a later render it would jump the running animation ahead. */
function CountdownBar({ since, duration, bar }: { since: number; duration: number; bar: string }) {
const [elapsed] = useState(() => Math.min(duration, Math.max(0, Date.now() - since)));
return (
<div
className={`absolute left-0 bottom-0 h-0.5 w-full origin-left ${bar}`}
style={{ animation: `toastbar ${duration}ms linear -${elapsed}ms forwards` }}
/>
);
}