fix(ui): a toast host ranks by when its overlay opened, and an overlay keeps its Escape, its dialog and its navigation straight
Deep-review round 4, the overlays. - Toast hosts were ranked by mount time, and PlayerModal swaps its host on every fullscreen change: leaving true fullscreen with the download dialog open put the player's host back on top, so toasts were drawn under that dialog's scrim and a click on one closed the dialog. Each overlay takes a rank once, when it opens (`useOverlayOrder`), and hands it to every host it renders. - A dialog open while fullscreen is left keeps a stable portal container (moved, not re-created): React tore the portal down when `fsRoot` changed, resetting a half-filled form and dropping focus to <body>. Focus is put back after the move. - The onboarding wizard is a layer, not a raw document listener: one Escape used to dismiss the error dialog over it AND end onboarding, skipping the write-scope step. - `navigateTo` (a toast's "View", clicked inside the player's own host) goes over the overlays it unmounts, so the player's Back-to-close cleanup no longer pops the page it just opened — the same fix Shift+W already had. The access-requests toast action too. - The live region arms itself a task after it takes over inherited toasts: React flushes effects in the same task as that render, so the "silent inheritance" arming was a no-op. - The overlay-host guard matches class tokens in any order (PlexPlayer's root was never checked), globs `ui/` too (ui/Modal was not checked either), and requires the rank.
This commit is contained in:
@@ -279,7 +279,8 @@ export default function App() {
|
|||||||
label: t("common.accessRequestsReview"),
|
label: t("common.accessRequestsReview"),
|
||||||
onClick: () => {
|
onClick: () => {
|
||||||
focusAccessRequestsTab();
|
focusAccessRequestsTab();
|
||||||
setPage("users");
|
// Over whatever overlay the toast was clicked in — as navigateTo does (see lib/nav).
|
||||||
|
navigateOverOverlays(() => setPage("users"));
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -0,0 +1,63 @@
|
|||||||
|
// @vitest-environment jsdom
|
||||||
|
//
|
||||||
|
// Escape closes the topmost layer only — the wizard included (deep-review round 4). Its own Escape
|
||||||
|
// was a raw `document` listener with no layer check, and document listeners run before the window
|
||||||
|
// listener ui/Modal uses: a sync that failed inside the wizard opens the global error dialog, and one
|
||||||
|
// Esc dismissed the error AND ended onboarding, skipping the write-scope step.
|
||||||
|
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||||
|
import { render, cleanup, screen, fireEvent } from "@testing-library/react";
|
||||||
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||||
|
|
||||||
|
vi.mock("react-i18next", async (importOriginal) => {
|
||||||
|
const actual = await importOriginal<typeof import("react-i18next")>();
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }),
|
||||||
|
Trans: ({ i18nKey }: { i18nKey: string }) => i18nKey,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
vi.mock("../lib/onboarding", () => ({ beginGrant: vi.fn(), endOnboarding: vi.fn() }));
|
||||||
|
|
||||||
|
import OnboardingWizard from "./OnboardingWizard";
|
||||||
|
import Modal from "./ui/Modal";
|
||||||
|
import { api, type Me } from "../lib/api";
|
||||||
|
|
||||||
|
const me = { id: 1, email: "a@b", can_read: false, can_write: false } as unknown as Me;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.spyOn(api, "myStatus").mockResolvedValue({ channels_total: 0 } as never);
|
||||||
|
});
|
||||||
|
afterEach(() => {
|
||||||
|
cleanup();
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("the onboarding wizard's Escape", () => {
|
||||||
|
const mount = (onClose: () => void, dialog: boolean) =>
|
||||||
|
render(
|
||||||
|
<QueryClientProvider
|
||||||
|
client={new QueryClient({ defaultOptions: { queries: { retry: false } } })}
|
||||||
|
>
|
||||||
|
<OnboardingWizard me={me} onClose={onClose} />
|
||||||
|
{dialog && (
|
||||||
|
<Modal title="boom" onClose={() => {}}>
|
||||||
|
<p>the sync failed</p>
|
||||||
|
</Modal>
|
||||||
|
)}
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
|
||||||
|
it("closes the wizard when nothing is open above it", () => {
|
||||||
|
const onClose = vi.fn();
|
||||||
|
mount(onClose, false);
|
||||||
|
fireEvent.keyDown(document, { key: "Escape" });
|
||||||
|
expect(onClose).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves the wizard alone while a dialog is open over it", () => {
|
||||||
|
const onClose = vi.fn();
|
||||||
|
mount(onClose, true);
|
||||||
|
fireEvent.keyDown(screen.getByText("the sync failed"), { key: "Escape" });
|
||||||
|
expect(onClose).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -7,7 +7,8 @@ import { api, type Me } from "../lib/api";
|
|||||||
import { beginGrant, endOnboarding } from "../lib/onboarding";
|
import { beginGrant, endOnboarding } from "../lib/onboarding";
|
||||||
import { Button } from "./ui/Button";
|
import { Button } from "./ui/Button";
|
||||||
import Overlay from "./ui/Overlay";
|
import Overlay from "./ui/Overlay";
|
||||||
import Toaster from "./ui/Toaster";
|
import Toaster, { useOverlayOrder } from "./ui/Toaster";
|
||||||
|
import { useLayer } from "../lib/layerStack";
|
||||||
|
|
||||||
// A first-login wizard that walks the user through granting YouTube access one scope at a
|
// 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
|
// time, each with a plain explanation and an up-front heads-up about Google's "unverified
|
||||||
@@ -103,15 +104,12 @@ export default function OnboardingWizard({ me, onClose }: { me: Me; onClose: ()
|
|||||||
endOnboarding(!me.can_read);
|
endOnboarding(!me.can_read);
|
||||||
onClose();
|
onClose();
|
||||||
};
|
};
|
||||||
|
const overlayOrder = useOverlayOrder(); // this overlay's toast-host rank (see ui/Toaster)
|
||||||
|
|
||||||
useEffect(() => {
|
// A layer in the shared stack, so Escape closes the wizard only when it is on top: a raw document
|
||||||
function onKey(e: KeyboardEvent) {
|
// listener ran BEFORE the error dialog's (a sync that failed inside the wizard opens one), and one
|
||||||
if (e.key === "Escape") close();
|
// Esc closed the error AND ended onboarding, skipping the write-scope step (deep-review round 4).
|
||||||
}
|
useLayer(true, close);
|
||||||
document.addEventListener("keydown", onKey);
|
|
||||||
return () => document.removeEventListener("keydown", onKey);
|
|
||||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
|
||||||
}, [me.can_read]);
|
|
||||||
|
|
||||||
// Are we currently importing (or about to)? Show a progress screen instead of the
|
// Are we currently importing (or about to)? Show a progress screen instead of the
|
||||||
// "you're connected" step until the first import settles.
|
// "you're connected" step until the first import settles.
|
||||||
@@ -265,7 +263,7 @@ export default function OnboardingWizard({ me, onClose }: { me: Me; onClose: ()
|
|||||||
</div>
|
</div>
|
||||||
{/* Toasts raised while the wizard is up (the connection notice among them) show above its
|
{/* Toasts raised while the wizard is up (the connection notice among them) show above its
|
||||||
scrim, not under it — see ui/Toaster. */}
|
scrim, not under it — see ui/Toaster. */}
|
||||||
<Toaster overlay />
|
<Toaster overlay order={overlayOrder} />
|
||||||
</div>
|
</div>
|
||||||
</Overlay>
|
</Overlay>
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -48,7 +48,7 @@ import { fullscreenElement } from "../lib/fullscreen";
|
|||||||
import { dispatch, isActivationTarget, useKeyScope } from "../lib/keymap";
|
import { dispatch, isActivationTarget, useKeyScope } from "../lib/keymap";
|
||||||
import { closeThenNavigate, useBackToClose } from "../lib/history";
|
import { closeThenNavigate, useBackToClose } from "../lib/history";
|
||||||
import ChannelNameLink from "./ChannelNameLink";
|
import ChannelNameLink from "./ChannelNameLink";
|
||||||
import Toaster from "./ui/Toaster";
|
import Toaster, { useOverlayOrder } from "./ui/Toaster";
|
||||||
import { afterRemoval } from "../lib/playerQueue";
|
import { afterRemoval } from "../lib/playerQueue";
|
||||||
import {
|
import {
|
||||||
useNativeControlsYield,
|
useNativeControlsYield,
|
||||||
@@ -379,6 +379,10 @@ export default function PlayerModal({
|
|||||||
staleTime: 5 * 60_000,
|
staleTime: 5 * 60_000,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Its toast hosts' rank (see ui/Toaster): the player's two hosts — the stage one in true fullscreen,
|
||||||
|
// the outer one otherwise — swap on every fullscreen change, and must not outrank a dialog opened
|
||||||
|
// over the player by doing so.
|
||||||
|
const overlayOrder = useOverlayOrder();
|
||||||
// This player is a dismissable layer: register it so a Modal or popover opened ABOVE it (e.g. the
|
// This player is a dismissable layer: register it so a Modal or popover opened ABOVE it (e.g. the
|
||||||
// download dialog, or — after S4 — an anchored menu) takes Escape first and this player defers.
|
// download dialog, or — after S4 — an anchored menu) takes Escape first and this player defers.
|
||||||
const layerIdRef = useRef<number | null>(null);
|
const layerIdRef = useRef<number | null>(null);
|
||||||
@@ -583,10 +587,9 @@ export default function PlayerModal({
|
|||||||
bar — that stays reachable by hovering the video's bottom edge, just above ours. Always
|
bar — that stays reachable by hovering the video's bottom edge, just above ours. Always
|
||||||
visible (no auto-hide) so there's no guessing whether it will appear and no fight with the
|
visible (no auto-hide) so there's no guessing whether it will appear and no fight with the
|
||||||
seek bar over the same pixels. stopPropagation so a click here doesn't toggle playback;
|
seek bar over the same pixels. stopPropagation so a click here doesn't toggle playback;
|
||||||
the exit button also re-arms our shortcuts. Add-to-playlist works in true fullscreen (its
|
the exit button also re-arms our shortcuts. Both of its dialogs work in true fullscreen:
|
||||||
popover portals INTO the fullscreen element); download stays hidden there because it opens
|
add-to-playlist's popover and download's Modal portal INTO the fullscreen element. */}
|
||||||
a Modal, which still portals to <body> — outside the fullscreen element. */}
|
{isFullscreen && <Toaster overlay order={overlayOrder} />}
|
||||||
{isFullscreen && <Toaster overlay />}
|
|
||||||
{(maximized || isFullscreen) && (
|
{(maximized || isFullscreen) && (
|
||||||
<div
|
<div
|
||||||
onClick={(e) => e.stopPropagation()}
|
onClick={(e) => e.stopPropagation()}
|
||||||
@@ -965,7 +968,7 @@ export default function PlayerModal({
|
|||||||
{/* Toasts raised while the player is open (a failed Remove, a lost connection) — the app's own
|
{/* 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. In true fullscreen it moves into the
|
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. */}
|
stage, the element holding the lock: out here it was painted under it, i.e. not at all. */}
|
||||||
{!isFullscreen && <Toaster overlay />}
|
{!isFullscreen && <Toaster overlay order={overlayOrder} />}
|
||||||
</div>,
|
</div>,
|
||||||
document.body,
|
document.body,
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -43,7 +43,7 @@ import { PlexPlayerSettings } from "./PlexPlayerSettings";
|
|||||||
import { PlexStatsOverlay } from "./PlexStatsOverlay";
|
import { PlexStatsOverlay } from "./PlexStatsOverlay";
|
||||||
import { createFragMeter } from "../lib/plexStats";
|
import { createFragMeter } from "../lib/plexStats";
|
||||||
import { UpNextCard } from "./UpNextCard";
|
import { UpNextCard } from "./UpNextCard";
|
||||||
import Toaster from "./ui/Toaster";
|
import Toaster, { useOverlayOrder } from "./ui/Toaster";
|
||||||
import {
|
import {
|
||||||
playhead,
|
playhead,
|
||||||
useAutoSkip,
|
useAutoSkip,
|
||||||
@@ -793,6 +793,8 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) {
|
|||||||
useKeyScope("plexPlayer");
|
useKeyScope("plexPlayer");
|
||||||
useHelpConditions({ episode: detail?.kind === "episode" });
|
useHelpConditions({ episode: detail?.kind === "episode" });
|
||||||
|
|
||||||
|
// Its toast host's rank among open overlays (see ui/Toaster).
|
||||||
|
const overlayOrder = useOverlayOrder();
|
||||||
// This player is a dismissable layer: register it so a Modal or popover opened ABOVE it (its own
|
// This player is a dismissable layer: register it so a Modal or popover opened ABOVE it (its own
|
||||||
// settings/tracks menus included) takes Escape first and this player defers (R8 S3).
|
// settings/tracks menus included) takes Escape first and this player defers (R8 S3).
|
||||||
const layerIdRef = useRef<number | null>(null);
|
const layerIdRef = useRef<number | null>(null);
|
||||||
@@ -1515,7 +1517,7 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) {
|
|||||||
)}
|
)}
|
||||||
{/* The app's Toaster is underneath this full-screen player (and outside it in native
|
{/* The app's Toaster is underneath this full-screen player (and outside it in native
|
||||||
fullscreen), so toasts raised while watching show here instead. See ui/Toaster. */}
|
fullscreen), so toasts raised while watching show here instead. See ui/Toaster. */}
|
||||||
<Toaster overlay />
|
<Toaster overlay order={overlayOrder} />
|
||||||
</div>,
|
</div>,
|
||||||
document.body,
|
document.body,
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -0,0 +1,52 @@
|
|||||||
|
// @vitest-environment jsdom
|
||||||
|
//
|
||||||
|
// A dialog opened from a player in true fullscreen portals INTO the fullscreen element (a node under
|
||||||
|
// <body> is painted beneath it, i.e. invisible). Leaving fullscreen changes that container — and a
|
||||||
|
// React portal whose container changes is torn down and recreated, so everything the dialog held went
|
||||||
|
// with it: a half-filled download form reset to its defaults and focus fell to <body> (a verifier's
|
||||||
|
// side finding in deep-review round 4).
|
||||||
|
import { describe, it, expect, afterEach } from "vitest";
|
||||||
|
import { render, cleanup, screen, fireEvent, act } from "@testing-library/react";
|
||||||
|
import { vi } from "vitest";
|
||||||
|
|
||||||
|
vi.mock("react-i18next", async (importOriginal) => {
|
||||||
|
const actual = await importOriginal<typeof import("react-i18next")>();
|
||||||
|
return { ...actual, useTranslation: () => ({ t: (key: string) => key }) };
|
||||||
|
});
|
||||||
|
|
||||||
|
import Modal from "./Modal";
|
||||||
|
|
||||||
|
const setFullscreen = (el: Element | null) =>
|
||||||
|
act(() => {
|
||||||
|
Object.defineProperty(document, "fullscreenElement", { value: el, configurable: true });
|
||||||
|
document.dispatchEvent(new Event("fullscreenchange"));
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
cleanup();
|
||||||
|
setFullscreen(null);
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("a dialog open while fullscreen is left", () => {
|
||||||
|
it("keeps what the viewer had typed in it, and the focus inside it", async () => {
|
||||||
|
const stage = document.createElement("div");
|
||||||
|
document.body.appendChild(stage);
|
||||||
|
await setFullscreen(stage);
|
||||||
|
render(
|
||||||
|
<Modal title="Download" onClose={() => {}}>
|
||||||
|
<input aria-label="name" defaultValue="" />
|
||||||
|
</Modal>,
|
||||||
|
);
|
||||||
|
const field = screen.getByLabelText("name") as HTMLInputElement;
|
||||||
|
fireEvent.change(field, { target: { value: "my cut" } });
|
||||||
|
field.focus();
|
||||||
|
expect(stage.contains(field)).toBe(true);
|
||||||
|
|
||||||
|
await setFullscreen(null); // the browser's own Esc leaves fullscreen; the dialog stays open
|
||||||
|
|
||||||
|
const after = screen.getByLabelText("name") as HTMLInputElement;
|
||||||
|
expect(after.value).toBe("my cut");
|
||||||
|
expect(document.activeElement).toBe(after);
|
||||||
|
stage.remove();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,4 +1,4 @@
|
|||||||
import { useEffect, useId, useRef, useState, type ReactNode } from "react";
|
import { useEffect, useId, useLayoutEffect, useRef, useState, type ReactNode } from "react";
|
||||||
import { createPortal } from "react-dom";
|
import { createPortal } from "react-dom";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import { X } from "lucide-react";
|
import { X } from "lucide-react";
|
||||||
@@ -6,7 +6,7 @@ import { useBackToClose } from "../../lib/history";
|
|||||||
import { pushLayer, removeLayer, isTopmost } from "../../lib/layerStack";
|
import { pushLayer, removeLayer, isTopmost } from "../../lib/layerStack";
|
||||||
import { useFullscreenElement } from "../../lib/fullscreen";
|
import { useFullscreenElement } from "../../lib/fullscreen";
|
||||||
import { Button } from "./Button";
|
import { Button } from "./Button";
|
||||||
import Toaster from "./Toaster";
|
import Toaster, { useOverlayOrder } from "./Toaster";
|
||||||
|
|
||||||
// Tabbable elements inside the dialog, in DOM order. `:not([tabindex="-1"])` drops programmatically
|
// 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
|
// focusable-only nodes; the visibility filter (offsetParent) drops display:none ones (e.g. a
|
||||||
@@ -45,6 +45,23 @@ export default function Modal({
|
|||||||
// Browser/mouse Back closes the topmost modal instead of navigating away.
|
// Browser/mouse Back closes the topmost modal instead of navigating away.
|
||||||
useBackToClose(onClose);
|
useBackToClose(onClose);
|
||||||
const fsRoot = useFullscreenElement();
|
const fsRoot = useFullscreenElement();
|
||||||
|
// ONE container element for the dialog's whole life, MOVED between <body> and the fullscreen
|
||||||
|
// element rather than handed to `createPortal` as a changing container: React tears a portal down
|
||||||
|
// and builds it again when its container changes, and the dialog's contents went with it — a
|
||||||
|
// half-filled download form reset to its defaults, and focus fell to <body>, the moment the viewer
|
||||||
|
// left true fullscreen (deep-review round 4).
|
||||||
|
const [host] = useState(() => document.createElement("div"));
|
||||||
|
useLayoutEffect(() => {
|
||||||
|
const parent = fsRoot ?? document.body;
|
||||||
|
if (host.parentNode === parent) return;
|
||||||
|
// appendChild MOVES the node: the browser blurs whatever was focused inside it on the way out.
|
||||||
|
const focused = host.contains(document.activeElement)
|
||||||
|
? (document.activeElement as HTMLElement)
|
||||||
|
: null;
|
||||||
|
parent.appendChild(host);
|
||||||
|
focused?.focus();
|
||||||
|
}, [fsRoot, host]);
|
||||||
|
useEffect(() => () => host.remove(), [host]);
|
||||||
const panelRef = useRef<HTMLDivElement>(null);
|
const panelRef = useRef<HTMLDivElement>(null);
|
||||||
const titleId = useId();
|
const titleId = useId();
|
||||||
// Capture the trigger to restore focus to, during THIS render — i.e. before commit runs the
|
// Capture the trigger to restore focus to, during THIS render — i.e. before commit runs the
|
||||||
@@ -52,6 +69,7 @@ export default function Modal({
|
|||||||
// it later (in the effect) would capture a now-inside element that unmounts on close, stranding
|
// it later (in the effect) would capture a now-inside element that unmounts on close, stranding
|
||||||
// focus on <body>. useState initializer runs exactly once, on first render.
|
// focus on <body>. useState initializer runs exactly once, on first render.
|
||||||
const [restoreTo] = useState(() => document.activeElement as HTMLElement | null);
|
const [restoreTo] = useState(() => document.activeElement as HTMLElement | null);
|
||||||
|
const overlayOrder = useOverlayOrder();
|
||||||
|
|
||||||
// Backdrop-click close must only fire when the press STARTED on the backdrop too. Otherwise a
|
// Backdrop-click close must only fire when the press STARTED on the backdrop too. Otherwise a
|
||||||
// drag that begins inside the dialog (e.g. selecting text in an input) and is released out on
|
// drag that begins inside the dialog (e.g. selecting text in an input) and is released out on
|
||||||
@@ -170,8 +188,8 @@ export default function Modal({
|
|||||||
{/* Toasts raised while a dialog is open show inside it: the app's own host is under this
|
{/* 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
|
scrim, where a failed share or rename was unreadable and a click on it closed the dialog
|
||||||
(see ui/Toaster). */}
|
(see ui/Toaster). */}
|
||||||
<Toaster overlay />
|
<Toaster overlay order={overlayOrder} />
|
||||||
</div>,
|
</div>,
|
||||||
fsRoot ?? document.body,
|
host,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -41,7 +41,8 @@ describe("Toaster — one host shows the toasts", () => {
|
|||||||
expect(screen.getByTestId("app").textContent).not.toContain("Couldn't remove the video.");
|
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.
|
// Both live regions stay mounted — a polite region has to pre-exist to be announced.
|
||||||
expect(hosts()).toHaveLength(2);
|
expect(hosts()).toHaveLength(2);
|
||||||
expect(hosts().map((h) => h.getAttribute("aria-live"))).toEqual(["off", "polite"]);
|
// The taking-over host arms itself a task later (see the "inherits" test below).
|
||||||
|
expect(hosts().map((h) => h.getAttribute("aria-live"))).toEqual(["off", "off"]);
|
||||||
|
|
||||||
overlay.unmount();
|
overlay.unmount();
|
||||||
expect(screen.getByTestId("app").textContent).toContain("Couldn't remove the video.");
|
expect(screen.getByTestId("app").textContent).toContain("Couldn't remove the video.");
|
||||||
@@ -74,6 +75,47 @@ describe("Toaster — handing the toasts to another host", () => {
|
|||||||
expect(screen.getByText("fresh").closest(".toast-card")!.getAttribute("role")).toBe("alert");
|
expect(screen.getByText("fresh").closest(".toast-card")!.getAttribute("role")).toBe("alert");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("keeps its live region off while the inherited toasts go in, and arms it a task later", async () => {
|
||||||
|
// The other half of "silently": a region that is already polite when the inherited nodes are
|
||||||
|
// inserted announces them anyway. React flushes effects in the SAME task as that render, so the
|
||||||
|
// arming has to wait for the next one — a screen reader reads the region's state after the task.
|
||||||
|
render(<Toaster />);
|
||||||
|
act(() => void notify({ level: "info", message: "saved" }));
|
||||||
|
const overlay = render(<Toaster overlay />);
|
||||||
|
const region = overlay.container.firstElementChild!;
|
||||||
|
expect(region.textContent).toContain("saved"); // it took the toast over…
|
||||||
|
expect(region.getAttribute("aria-live")).toBe("off"); // …while still off
|
||||||
|
await act(async () => void (await new Promise((r) => setTimeout(r, 0))));
|
||||||
|
expect(region.getAttribute("aria-live")).toBe("polite"); // and polite for what comes next
|
||||||
|
});
|
||||||
|
|
||||||
|
it("ranks hosts by when their OVERLAY opened, not by when the host mounted", async () => {
|
||||||
|
// PlayerModal's host moves into the fullscreen stage and back: remounted, it used to jump over a
|
||||||
|
// dialog opened above the player, and the toasts went back under that dialog's scrim.
|
||||||
|
const player = render(
|
||||||
|
<div data-testid="player">
|
||||||
|
<Toaster overlay order={1} />
|
||||||
|
</div>,
|
||||||
|
);
|
||||||
|
render(
|
||||||
|
<div data-testid="dialog">
|
||||||
|
<Toaster overlay order={2} />
|
||||||
|
</div>,
|
||||||
|
);
|
||||||
|
act(() => void notify({ level: "error", message: "boom" }));
|
||||||
|
expect(screen.getByTestId("dialog").textContent).toContain("boom");
|
||||||
|
// The player swaps its host (leaving true fullscreen) while the dialog stays open.
|
||||||
|
player.rerender(
|
||||||
|
<div data-testid="player">
|
||||||
|
<span />
|
||||||
|
<Toaster overlay order={1} />
|
||||||
|
</div>,
|
||||||
|
);
|
||||||
|
await act(async () => void (await new Promise((r) => setTimeout(r, 0))));
|
||||||
|
expect(screen.getByTestId("dialog").textContent).toContain("boom");
|
||||||
|
expect(screen.getByTestId("player").textContent).not.toContain("boom");
|
||||||
|
});
|
||||||
|
|
||||||
it("keeps the countdown where it was, instead of starting it again from full", () => {
|
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
|
// 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.
|
// new node restarted the bar, and the toast vanished with it still two-thirds full.
|
||||||
|
|||||||
@@ -3,31 +3,77 @@
|
|||||||
// The app's Toaster lives in the content column, a stacking context: anything that covers the
|
// 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
|
// 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
|
// 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
|
// `<Toaster overlay order={useOverlayOrder()} />` inside itself — and a convention stated in a comment
|
||||||
// forgets (deep-review round 3 found every ui/Modal and the onboarding wizard without one). So it is
|
// is one the next overlay forgets (deep-review round 3 found every ui/Modal and the onboarding wizard
|
||||||
// checked here: a component that renders a viewport-covering z-overlay layer mounts an overlay host.
|
// without one). So it is checked here: a component that renders a viewport-covering layer at
|
||||||
|
// z-overlay or above mounts an overlay host, ranked by when the overlay opened.
|
||||||
|
//
|
||||||
|
// The class TOKENS are matched in any order, and the layer need not be `inset-0`: the first version
|
||||||
|
// matched the exact string "fixed inset-0 z-overlay", so PlexPlayer's own root (`fixed left-0 top-0
|
||||||
|
// z-overlay`) was never checked at all, and a new viewer copying it would have slipped past
|
||||||
|
// (deep-review round 4).
|
||||||
import { describe, it, expect } from "vitest";
|
import { describe, it, expect } from "vitest";
|
||||||
|
|
||||||
const sources = import.meta.glob("../**/*.tsx", { query: "?raw", import: "default", eager: true });
|
// BOTH patterns: `../**/*.tsx` from this directory does not match the siblings in `ui/` itself, so
|
||||||
|
// ui/Modal.tsx — the overlay behind every dialog in the app — was never checked by this guard at all
|
||||||
|
// (deep-review round 4).
|
||||||
|
const sources = import.meta.glob(["../**/*.tsx", "./*.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
|
// Overlays with nothing a toast could be about: a lazy chunk's black placeholder while the player
|
||||||
// placeholder while the player loads).
|
// loads, and the signed-out landing page's screenshot zoom (the app's Toaster is not even mounted
|
||||||
const EXEMPT = new Set(["../PlexBrowse.tsx"]);
|
// there — App renders Welcome instead of the shell).
|
||||||
|
const EXEMPT = new Set(["../PlexBrowse.tsx", "../Welcome.tsx"]);
|
||||||
|
|
||||||
|
/** Every class string in the file (quoted or templated), including multi-line ones. */
|
||||||
|
const classStrings = (src: string): string[] =>
|
||||||
|
[...src.matchAll(/className=\{?[`"']([^`"']*)[`"']/g)].map((m) => m[1]!);
|
||||||
|
|
||||||
|
const coversTheViewport = (src: string) =>
|
||||||
|
classStrings(src).some(
|
||||||
|
(cls) =>
|
||||||
|
/(^|\s)fixed(\s|$)/.test(cls) &&
|
||||||
|
/(^|\s)z-(overlay|popover|tooltip)(\s|$)/.test(cls) &&
|
||||||
|
(/(^|\s)inset-0(\s|$)/.test(cls) ||
|
||||||
|
(/(^|\s)left-0(\s|$)/.test(cls) && /(^|\s)top-0(\s|$)/.test(cls))),
|
||||||
|
);
|
||||||
|
|
||||||
|
const overlays = () =>
|
||||||
|
Object.entries(sources)
|
||||||
|
.filter(([path]) => !path.endsWith(".test.tsx") && !EXEMPT.has(path))
|
||||||
|
.filter(([, src]) => coversTheViewport(src as string));
|
||||||
|
|
||||||
describe("full-screen overlays host their own toasts", () => {
|
describe("full-screen overlays host their own toasts", () => {
|
||||||
it("every one that covers the viewport at z-overlay mounts <Toaster overlay />", () => {
|
it("every one that covers the viewport mounts an overlay host", () => {
|
||||||
const missing = Object.entries(sources)
|
const missing = overlays()
|
||||||
.filter(([path]) => !path.endsWith(".test.tsx") && !EXEMPT.has(path))
|
.filter(([, src]) => !(src as string).includes("<Toaster overlay"))
|
||||||
.filter(([, src]) => /fixed inset-0 z-overlay/.test(src as string))
|
|
||||||
.filter(([, src]) => !(src as string).includes("<Toaster overlay />"))
|
|
||||||
.map(([path]) => path);
|
.map(([path]) => path);
|
||||||
expect(missing).toEqual([]);
|
expect(missing).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("and ranks it by when the overlay opened, not by when the host mounted", () => {
|
||||||
|
// A host that takes its own rank jumps to the top whenever it remounts — which PlayerModal's does
|
||||||
|
// on every fullscreen change, putting it back over a dialog opened above the player.
|
||||||
|
const unranked = overlays()
|
||||||
|
.filter(([, src]) => !(src as string).includes("useOverlayOrder()"))
|
||||||
|
.map(([path]) => path);
|
||||||
|
expect(unranked).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
it("finds the overlays it is meant to check", () => {
|
it("finds the overlays it is meant to check", () => {
|
||||||
const covered = Object.entries(sources).filter(([, src]) =>
|
// Modal, PlayerModal, PlexPlayer and the wizard at least — PlexPlayer is the one the exact-string
|
||||||
/fixed inset-0 z-overlay/.test(src as string),
|
// check used to miss.
|
||||||
|
const paths = overlays().map(([p]) => p);
|
||||||
|
expect(paths).toEqual(
|
||||||
|
expect.arrayContaining([
|
||||||
|
"./Modal.tsx",
|
||||||
|
"../PlayerModal.tsx",
|
||||||
|
"../PlexPlayer.tsx",
|
||||||
|
"../OnboardingWizard.tsx",
|
||||||
|
]),
|
||||||
);
|
);
|
||||||
expect(covered.length).toBeGreaterThanOrEqual(3); // Modal, PlayerModal, the wizard at least
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -16,24 +16,48 @@ export const LEVEL_STYLE: Record<NotifLevel, { icon: typeof Info; color: string;
|
|||||||
// context (its `@container` implies layout containment) — so anything portalled to <body> at the
|
// 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%
|
// 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
|
// 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
|
// OWN `<Toaster overlay order={…} />` inside itself — ui/Modal does it for every dialog, the players
|
||||||
// themselves — and exactly one host shows the toasts: the most recently mounted overlay, else the
|
// for themselves — and exactly one host shows the toasts: the one of the overlay OPENED last, else
|
||||||
// app's. Same component, same look — and inside the overlay's aria-modal dialog, where a screen
|
// the app's. Same component, same look — and inside the overlay's aria-modal dialog, where a screen
|
||||||
// reader can still reach it.
|
// reader can still reach it.
|
||||||
const overlayHosts = createStore<readonly string[]>([]);
|
//
|
||||||
|
// "Opened last", not "host mounted last": a host can remount while its overlay stays open —
|
||||||
|
// PlayerModal's moves into the fullscreen stage and back — and ranked by mount time it came back on
|
||||||
|
// top of a dialog opened over the player (the download dialog after leaving true fullscreen), so the
|
||||||
|
// toasts were drawn under that dialog's scrim again (deep-review round 4). So the OVERLAY takes its
|
||||||
|
// rank once, when it opens (`useOverlayOrder`), and hands it to every host it renders.
|
||||||
|
let overlaySeq = 0;
|
||||||
|
/** This overlay's rank among open overlays — taken once, when it mounts. Call it in the overlay
|
||||||
|
* itself, never in a component that can remount while the overlay stays open. */
|
||||||
|
export function useOverlayOrder(): number {
|
||||||
|
const [order] = useState(() => ++overlaySeq);
|
||||||
|
return order;
|
||||||
|
}
|
||||||
|
type Host = { id: string; order: number };
|
||||||
|
const overlayHosts = createStore<readonly Host[]>([]);
|
||||||
const topHost = () => {
|
const topHost = () => {
|
||||||
const hosts = overlayHosts.get();
|
let top: Host | null = null;
|
||||||
return hosts[hosts.length - 1] ?? null;
|
for (const h of overlayHosts.get()) if (!top || h.order >= top.order) top = h;
|
||||||
|
return top?.id ?? null;
|
||||||
};
|
};
|
||||||
|
|
||||||
export default function Toaster({ overlay = false }: { overlay?: boolean }) {
|
export default function Toaster({
|
||||||
|
overlay = false,
|
||||||
|
order,
|
||||||
|
}: {
|
||||||
|
overlay?: boolean;
|
||||||
|
/** The owning overlay's `useOverlayOrder()`. Without one, the host ranks by its own mount. */
|
||||||
|
order?: number;
|
||||||
|
}) {
|
||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
const id = useId();
|
const id = useId();
|
||||||
|
const ownOrder = useOverlayOrder();
|
||||||
|
const rank = order ?? ownOrder;
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!overlay) return;
|
if (!overlay) return;
|
||||||
overlayHosts.set((hosts) => [...hosts, id]);
|
overlayHosts.set((hosts) => [...hosts, { id, order: rank }]);
|
||||||
return () => overlayHosts.set((hosts) => hosts.filter((h) => h !== id));
|
return () => overlayHosts.set((hosts) => hosts.filter((h) => h.id !== id));
|
||||||
}, [overlay, id]);
|
}, [overlay, id, rank]);
|
||||||
const top = useSyncExternalStore(overlayHosts.subscribe, topHost, topHost);
|
const top = useSyncExternalStore(overlayHosts.subscribe, topHost, topHost);
|
||||||
const active = useSyncExternalStore(subscribe, getActiveToasts, getActiveToasts);
|
const active = useSyncExternalStore(subscribe, getActiveToasts, getActiveToasts);
|
||||||
const isTop = top === (overlay ? id : null);
|
const isTop = top === (overlay ? id : null);
|
||||||
@@ -41,14 +65,23 @@ export default function Toaster({ overlay = false }: { overlay?: boolean }) {
|
|||||||
// A host that takes over (an overlay opening, or the app's own once it closes) receives the toasts
|
// 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
|
// 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
|
// 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
|
// stays `aria-live="off"` until they are in, turning polite (for the toasts that come after) in a
|
||||||
// they have been rendered.
|
// LATER task. Not in an effect: React flushes those in the same task as the render that inserted the
|
||||||
|
// toasts, and a screen reader reads the region's state after the task — by which time it was polite
|
||||||
|
// again, i.e. the arming did nothing.
|
||||||
const inherited = useMemo(
|
const inherited = useMemo(
|
||||||
() => new Set(isTop ? getActiveToasts().map((n) => n.id) : []),
|
() => new Set(isTop ? getActiveToasts().map((n) => n.id) : []),
|
||||||
[isTop],
|
[isTop],
|
||||||
);
|
);
|
||||||
const [armed, setArmed] = useState(false);
|
const [armed, setArmed] = useState(false);
|
||||||
useEffect(() => setArmed(isTop), [isTop]);
|
useEffect(() => {
|
||||||
|
if (!isTop) {
|
||||||
|
setArmed(false);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const timer = window.setTimeout(() => setArmed(true), 0);
|
||||||
|
return () => window.clearTimeout(timer);
|
||||||
|
}, [isTop]);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
// Stable, always-mounted live region — a polite region must PRE-EXIST in the DOM for a screen
|
// Stable, always-mounted live region — a polite region must PRE-EXIST in the DOM for a screen
|
||||||
|
|||||||
@@ -0,0 +1,40 @@
|
|||||||
|
// @vitest-environment jsdom
|
||||||
|
//
|
||||||
|
// `navigateTo` is what a toast's action calls ("Download added — View"), and that click happens
|
||||||
|
// INSIDE an overlay's toast host. The page change unmounts the page-owned overlay (the player), whose
|
||||||
|
// Back-to-close cleanup then reclaimed the history entry the navigation had just pushed — so the user
|
||||||
|
// landed back where they started with the player closed instead of on the Downloads page
|
||||||
|
// (deep-review round 4). Same trap, and the same fix, as the global Shift+W shortcut.
|
||||||
|
//
|
||||||
|
// Its own file: `lib/history`'s depth bookkeeping is module state.
|
||||||
|
import { describe, it, expect, vi } from "vitest";
|
||||||
|
import { renderHook, act } from "@testing-library/react";
|
||||||
|
import { useBackToClose } from "./history";
|
||||||
|
import { navigateTo, setNavigator } from "./nav";
|
||||||
|
|
||||||
|
const settle = () =>
|
||||||
|
act(async () => {
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("navigateTo from inside an overlay", () => {
|
||||||
|
it("buries the overlay's entry, so closing it does not pop the page just opened", async () => {
|
||||||
|
const overlay = renderHook(() => useBackToClose(() => {}));
|
||||||
|
await settle();
|
||||||
|
const back = vi.spyOn(window.history, "back");
|
||||||
|
try {
|
||||||
|
setNavigator(() => {
|
||||||
|
window.history.pushState({ sfPage: "downloads" }, "");
|
||||||
|
return true; // as App's setPage reports: it really pushed
|
||||||
|
});
|
||||||
|
act(() => navigateTo("downloads"));
|
||||||
|
overlay.unmount(); // the page change unmounts the player
|
||||||
|
await settle();
|
||||||
|
expect(back).not.toHaveBeenCalled();
|
||||||
|
expect(window.history.state).toMatchObject({ sfPage: "downloads" });
|
||||||
|
} finally {
|
||||||
|
back.mockRestore();
|
||||||
|
setNavigator(() => {});
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,16 +1,21 @@
|
|||||||
|
import { navigateOverOverlays } from "./history";
|
||||||
import type { Page } from "./urlState";
|
import type { Page } from "./urlState";
|
||||||
|
|
||||||
// Tiny decoupled navigator so deep components (a feed card's download toast, the Download
|
// Tiny decoupled navigator so deep components (a feed card's download toast, the Download
|
||||||
// Center) can switch pages without threading setPage through the whole tree. App registers
|
// Center) can switch pages without threading setPage through the whole tree. App registers
|
||||||
// its setPage on mount; navigateTo is a no-op until then.
|
// its setPage on mount; navigateTo is a no-op until then.
|
||||||
let _navigate: ((p: Page) => void) | null = null;
|
let _navigate: ((p: Page) => boolean | void) | null = null;
|
||||||
|
|
||||||
export function setNavigator(fn: (p: Page) => void): void {
|
export function setNavigator(fn: (p: Page) => boolean | void): void {
|
||||||
_navigate = fn;
|
_navigate = fn;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Over whatever overlay is open: the "View" on a "Download added" toast is clicked INSIDE the player
|
||||||
|
* (its toast host), and the page change unmounts the page-owned player, whose Back-to-close cleanup
|
||||||
|
* then popped the entry the navigation had just pushed — the user landed back on the feed with the
|
||||||
|
* player gone (deep-review round 4). Same trap, same fix as Shift+W (see `navigateOverOverlays`). */
|
||||||
export function navigateTo(p: Page): void {
|
export function navigateTo(p: Page): void {
|
||||||
_navigate?.(p);
|
navigateOverOverlays(() => _navigate?.(p));
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Should this click be handled IN-APP, or left to the browser?
|
/** Should this click be handled IN-APP, or left to the browser?
|
||||||
|
|||||||
Reference in New Issue
Block a user