refactor(ui): the popover engine and every render-phase ref write compile under the React Compiler

E32 S3. The compiler skipped 93 functions; it now skips 29 (try/finally shapes, two suppressions,
VirtualGrid). react-hooks/refs hits 245 -> 31.

- Popover engine: `ref={pop.refs.setReference}` made the compiler treat the whole popover handle as
  a ref, so every component holding one was skipped (21 call sites, the engine and its surface
  among them). The setter is destructured now; the same for useScrollFade and useReturnFocus.
- Render-phase `ref.current = x` writes moved to `useEffectEvent` where only effects read them,
  else to the new `useLatest` (written in a layout effect) / `useStableCallback`.
- FocusSafeButton (under every Button) read the focused element during render; it reads it through
  useSyncExternalStore, which React asks on every render instead of a cached memo.

Bugs this surfaced, fixed:
- Combobox: a chip could keep showing its raw id. The label cache was a ref an effect filled after
  the render that needed it; it is state merged during render now (test seen RED).
- useUndoable rendered from a ref plus a forced re-render; compiled, that memoizes a stale value.
  The history is state now, mirrored in a ref for the stable callbacks.
- ConfigPanel and the saved-views editor re-seeded their drafts in an effect, painting the stale
  draft for a frame; both adjust during render against the remembered key.
- Toaster's handover is written after commit instead of during render; a new test pins that the
  inherited set is taken once, at the takeover (seen RED on a per-render re-read).
- IssueComposer mutated a ref prop as a one-shot focus token; it is plain state.
- PlexBrowse wrote its info-scroll memory from an effect declared before the ref, past its own
  "one writer" rule; the writer comes first now and the effect goes through it.

The compiler ratchet drops to the measured 29 and now also requires FocusSafeButton and the popover
engine to compile.
This commit is contained in:
2026-10-03 23:18:26 +02:00
parent dc00b55feb
commit 76358a0d77
68 changed files with 606 additions and 545 deletions
+5 -3
View File
@@ -164,10 +164,11 @@ function SubMenu({ item, onClose }: { item: SubmenuItem; onClose: () => void })
// running off the bottom at 768px.
fitHeight: true,
});
const { setReference } = pop.refs;
return (
<>
<PopoverItem
elementRef={pop.refs.setReference}
elementRef={setReference}
{...pop.getReferenceProps()}
role="menuitem"
disabled={item.disabled}
@@ -235,12 +236,13 @@ export default function ActionMenu({
// PopoverItem renders it as a native `disabled` button, which useListNavigation reads and never
// focuses. No explicit disabledIndices needed. See ActionMenu.dom.test.tsx for the guard.
const pop = useAnchoredPopover({ open, onOpenChange: setOpen, placement: "bottom-end" });
const { setReference } = pop.refs;
return (
<>
{/* A caller-shaped trigger (a labelled pill) is a surface; the default "…" is a glyph. */}
{className ? (
<Pressable
ref={pop.refs.setReference}
ref={setReference}
{...pop.getReferenceProps()}
aria-label={ariaLabel}
title={ariaLabel}
@@ -252,7 +254,7 @@ export default function ActionMenu({
<IconButton
variant="glass"
size="md"
ref={pop.refs.setReference}
ref={setReference}
{...pop.getReferenceProps()}
label={ariaLabel}
>
@@ -43,6 +43,10 @@ import type { useAnchoredPopover } from "./anchoredPopoverHooks";
// free-text input (the tag creator) must stopPropagation non-Escape keys on that input so ↑/↓ and
// typeahead act on the text, not the list — see ChannelTags.
//
// The trigger takes its ref DESTRUCTURED: `const { setReference } = pop.refs;` then
// `ref={setReference} {...pop.getReferenceProps()}`. Written `ref={pop.refs.setReference}`, the React
// Compiler treats the whole `pop` as a ref and skips the component (E32 S3 — 21 call sites did).
//
// S? adds a "combobox" mode — the ONE right way to build a typeable combobox (a search/filter input
// with a listbox of options below), so a hand-rolled floating-ui engine (the old MultiSelectChips /
// PlexSearchBox pattern) is no longer needed. It differs from menu/listbox in exactly the focus model:
@@ -294,6 +298,7 @@ function PopoverSurfaceInner({
elementsRef,
labelsRef,
} = popover;
const { setFloating } = refs;
// A node portaled to <body> is painted UNDERNEATH whatever holds the fullscreen lock, so a popover
// opened from a player's control bar in fullscreen would be invisible; it must portal INTO the
@@ -369,7 +374,7 @@ function PopoverSurfaceInner({
const surface = (
<div
ref={refs.setFloating}
ref={setFloating}
{...floatingProps}
aria-label={listProps ? undefined : ariaLabel}
className={zClass}
@@ -206,6 +206,30 @@ describe("Combobox", () => {
expect(onChange).toHaveBeenCalledWith(["id1"]); // single-select replaces
});
it("names a chip by a label that arrives with later options, in that same render", () => {
// E32 S3: the label cache was a ref an effect filled AFTER the render that needed it, and nothing
// re-rendered afterwards — the chip kept showing the raw id until something unrelated did.
const view = (options: { value: string; label?: string }[]) => (
<FloatingTree>
<Combobox
label="Collection"
clientFilter={false}
options={options}
selected={["id1"]}
onChange={vi.fn()}
/>
</FloatingTree>
);
const { rerender } = render(view([]));
expect(screen.getByText("id1")).toBeTruthy();
rerender(view([{ value: "id1", label: "Harry Potter" }]));
expect(screen.queryByText("id1")).toBeNull();
expect(screen.getByText("Harry Potter")).toBeTruthy();
// …and it outlives the option: a narrower list later still names the chip.
rerender(view([]));
expect(screen.getByText("Harry Potter")).toBeTruthy();
});
it("single-select: picking replaces the value and closes the list", () => {
const onChange = vi.fn();
wrap(
+37 -21
View File
@@ -1,5 +1,5 @@
import clsx from "clsx";
import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from "react";
import { useCallback, useMemo, useState, type ReactNode } from "react";
import { useTranslation } from "react-i18next";
import { ChevronDown } from "lucide-react";
import { Button } from "./Button";
@@ -78,6 +78,23 @@ export interface ComboboxProps {
labelsPending?: boolean;
}
/** Remember each value's display label as options flow through, so a selected chip can name itself
* even after the option leaves the query-narrowed list (async single-select). State, merged DURING
* render: a ref written in an effect was read by the render before it, so a chip could name itself
* by its raw value until something else re-rendered (E32). */
function useLabelCache(options: readonly ComboboxOption[]): Record<string, string> {
const [labelCache, setLabelCache] = useState<Record<string, string>>({});
const newLabels = options.filter(
(o): o is typeof o & { label: string } => o.label != null && labelCache[o.value] !== o.label,
);
if (newLabels.length)
setLabelCache((c) => ({
...c,
...Object.fromEntries(newLabels.map((o) => [o.value, o.label])),
}));
return labelCache;
}
export default function Combobox({
label,
options,
@@ -108,17 +125,15 @@ export default function Combobox({
if (queryProp === undefined) setQueryState(q);
onQueryChange?.(q);
};
const inputRef = useRef<HTMLInputElement | null>(null);
const fade = useScrollFade();
// The input node as STATE, not a ref: the handlers below are handed to getReferenceProps during
// render, and a ref inside them made the React Compiler skip the whole component (E32).
const [inputEl, setInputEl] = useState<HTMLInputElement | null>(null);
const { ref: fadeRef, style: fadeStyle } = useScrollFade();
// Remember each value's display label as options flow through, so a selected chip can name itself
// even after the option leaves the query-narrowed list (async single-select). Caller-supplied
// `selectedLabels` win (they survive a page reload where nothing has been fetched yet).
const labelCache = useRef<Record<string, string>>({});
useEffect(() => {
for (const o of options) if (o.label != null) labelCache.current[o.value] = o.label;
}, [options]);
const knownLabel = (v: string) => selectedLabels?.[v] ?? labelCache.current[v];
// Caller-supplied `selectedLabels` win over the cache (they survive a page reload where nothing
// has been fetched yet).
const labelCache = useLabelCache(options);
const knownLabel = (v: string) => selectedLabels?.[v] ?? labelCache[v];
const labelFor = (v: string) => knownLabel(v) ?? v;
const setOpenAnd = (next: boolean) => {
@@ -134,18 +149,19 @@ export default function Combobox({
if (!next) setQuery("");
};
const pop = useAnchoredPopover({ open, onOpenChange: setOpenAnd, mode: "combobox" });
const { setReference, setPositionReference } = pop.refs;
// The input is the combobox reference for floating-ui AND our own focus target (the field's
// click-to-focus handler, and commit() re-focusing after a pick). A merged callback ref feeds the
// node to BOTH — without it `inputRef` stays null, so the field's onMouseDown guard (`e.target !==
// inputRef.current`) is always true and preventDefault() cancels the input's own click-focus, making
// node to BOTH — without it the node stays null, so the field's onMouseDown guard (`e.target !==
// inputEl`) is always true and preventDefault() cancels the input's own click-focus, making
// the field impossible to type into with the mouse. Memoised so floating-ui's ref isn't re-attached
// every render.
const setInputRef = useCallback(
(node: HTMLInputElement | null) => {
pop.refs.setReference(node);
inputRef.current = node;
setReference(node);
setInputEl(node);
},
[pop.refs],
[setReference],
);
// `highlightIndex` = the row the user actually SEES highlighted (pointer hover, else keyboard); Enter
// commits THAT, not the bare keyboard `activeIndex` — otherwise arrow-then-hover-then-Enter would toggle
@@ -173,7 +189,7 @@ export default function Combobox({
selected.includes(value) ? selected.filter((x) => x !== value) : [...selected, value],
);
// Keep the field focused so the user can keep picking / typing; the list stays open.
inputRef.current?.focus();
inputEl?.focus();
} else {
// Single-select is final: close and DON'T re-focus — a refocus fires onFocus, which would
// immediately re-open the list (now that the merged ref makes focus() actually work).
@@ -187,7 +203,7 @@ export default function Combobox({
{/* The field: removable chips + a free-text input + a chevron, styled as one input box. The whole
box is the POSITION anchor (the panel matches its width); the input is the combobox reference. */}
<div
ref={pop.refs.setPositionReference}
ref={setPositionReference}
onMouseDown={(e) => {
// A press anywhere but the input (padding / a chip gap) focuses the field and opens the list —
// focus drives "open", so a caret click never toggles it shut. A press on a chip's × is its
@@ -197,9 +213,9 @@ export default function Combobox({
e.preventDefault();
return;
}
if (e.target !== inputRef.current) {
if (e.target !== inputEl) {
e.preventDefault();
inputRef.current?.focus();
inputEl?.focus();
setOpenAnd(true);
}
}}
@@ -277,7 +293,7 @@ export default function Combobox({
className={`${SURFACE.menu} w-full animate-[popIn_0.16s_ease]`}
>
{/* The scroll box owns the height cap AND the scroll-fade mask (ref+style on the same element). */}
<div ref={fade.ref} style={fade.style} className="max-h-64 overflow-y-auto no-scrollbar">
<div ref={fadeRef} style={fadeStyle} className="max-h-64 overflow-y-auto no-scrollbar">
{/* The listbox holds the options alone; the lines about the list sit beside it (F4). */}
<PopoverList>
{!showError &&
+2 -1
View File
@@ -64,6 +64,7 @@ export default function FilterSelect({
options.findIndex((o) => o.value === selected[0]),
),
});
const { setReference } = pop.refs;
const labelOf = (v: string) => options.find((o) => o.value === v)?.label ?? v;
// Summarise the trigger value. Multi lists the first chosen (in options order, so it's stable) + "+N".
@@ -96,7 +97,7 @@ export default function FilterSelect({
context="toolbar"
open={open}
label={hideLabel ? undefined : label}
ref={pop.refs.setReference}
ref={setReference}
{...pop.getReferenceProps()}
data-testid={testId}
// Carry the current value in the accessible name (not just the group label), matching
+19 -4
View File
@@ -1,4 +1,11 @@
import { forwardRef, useCallback, useRef, useState, type ButtonHTMLAttributes } from "react";
import {
forwardRef,
useCallback,
useRef,
useState,
useSyncExternalStore,
type ButtonHTMLAttributes,
} from "react";
import clsx from "clsx";
import { endsTextSelection, movedPastSlop } from "../../lib/textSelection";
@@ -23,6 +30,8 @@ import { endsTextSelection, movedPastSlop } from "../../lib/textSelection";
// <button> unless it is `user-select: text`, and then the mouseup that ends the drag still clicks the
// button (both measured, 2026-09-26), so the flag sets both halves: the text selects, and the click
// that ends a selection inside the button, or a press dragged past the slop, does nothing.
const noSubscription = () => () => {};
export const FocusSafeButton = forwardRef<
HTMLButtonElement,
ButtonHTMLAttributes<HTMLButtonElement> & {
@@ -58,9 +67,15 @@ export const FocusSafeButton = forwardRef<
);
// Focused by the event OR by the document's own answer at render time: when the window itself is
// not focused (another app in front) `.focus()` moves `activeElement` but fires no focus event
// (measured in Chrome), and the button disabled in that state lost the focus all the same.
const focused =
focusEvent || (!!node.current && node.current.ownerDocument.activeElement === node.current);
// (measured in Chrome), and the button disabled in that state lost the focus all the same. Read
// through useSyncExternalStore, which React asks on EVERY render, never from a memo: a bare
// render-time DOM read made the React Compiler skip the component (E32), and compiled it would be
// cached. Nothing to subscribe to — the focus events above already re-render it.
const activeNow = useSyncExternalStore(
noSubscription,
() => node.current !== null && node.current.ownerDocument.activeElement === node.current,
);
const focused = focusEvent || activeNow;
const soft = busy || (!!disabled && focused);
// A caller's own aria-disabled is the "unavailable, and here is why" state (DESIGN.md: the tooltip or
// label carries the reason, so the button stays focusable): it is kept, never overwritten by ours, and
+4 -4
View File
@@ -2,6 +2,7 @@ import {
createContext,
useContext,
useEffect,
useEffectEvent,
useId,
useRef,
useState,
@@ -124,15 +125,14 @@ export default function Modal({
// the backdrop wrongly dismisses — the click event bubbles to the common ancestor (the backdrop).
const pressedOnBackdrop = useRef(false);
// Keep a stable handler across renders so the stack id is assigned once per mount.
const onCloseRef = useRef(onClose);
onCloseRef.current = onClose;
// An effect event, so a new handler each render never re-registers: the stack id is assigned once per mount.
const close = useEffectEvent(onClose);
useEffect(() => {
const id = pushLayer();
const onKey = (e: KeyboardEvent) => {
if (takeEscape(e, id)) {
e.stopPropagation();
onCloseRef.current();
close();
return;
}
// Focus trap: keep Tab / Shift+Tab cycling inside the dialog instead of reaching the inert
+2 -1
View File
@@ -216,12 +216,13 @@ export function NavRailGroup({
placement: "right-start",
offset: 8,
});
const { setReference } = pop.refs;
const active = pages.some((p) => p.active);
return (
<>
<Tooltip hint={label} side="right" always openDelay={350}>
<Pressable
ref={pop.refs.setReference}
ref={setReference}
{...pop.getReferenceProps()}
data-testid={testId}
aria-label={showLabel ? shortLabel : label}
+28 -22
View File
@@ -1,4 +1,11 @@
import { useCallback, useLayoutEffect, useRef, useState, type ReactNode } from "react";
import {
useCallback,
useEffectEvent,
useLayoutEffect,
useRef,
useState,
type ReactNode,
} from "react";
import { useScrollFade } from "../../lib/useScrollFade";
import { FOCUS_REST_ATTRS, useFocusKeeper } from "../../lib/useFocusKeeper";
import { StickyTopProvider } from "./stickyTop";
@@ -130,12 +137,11 @@ export default function PageScroller({
setStickyTops((n) => n + 1);
return () => setStickyTops((n) => n - 1);
}, []);
const fade = useScrollFade(20, stickyTops === 0);
const { ref: fadeRef, style: fadeStyle } = useScrollFade(20, stickyTops === 0);
const fabUp = useBackToTopShown();
const fadeRef = fade.ref;
const elRef = useRef<HTMLElement | null>(null);
// Stable identity: an inline ref callback changes every render, which makes React detach and
// re-attach the ref each commit — and since fade.ref is a state setter, that re-attach loops it
// re-attach the ref each commit — and since fadeRef is a state setter, that re-attach loops it
// into "Maximum update depth exceeded". useScrollFade's ref setter is itself stable, so binding
// once here is safe.
const setRef = useCallback(
@@ -150,29 +156,31 @@ export default function PageScroller({
// all) leaves the focus in its slot rather than on <body> — see useFocusKeeper.
useFocusKeeper(elRef);
// Live scrollKey for the event/nav-bus callbacks (which are bound once, not per scrollKey).
const scrollKeyRef = useRef(scrollKey);
scrollKeyRef.current = scrollKey;
const lastComposite = useRef<string | null>(null);
const cancelRef = useRef<() => void>(() => {});
// Restore (or reset to top) whenever the (history entry + content) identity changes — a page/sub-view
// navigation, a Back/Forward, or a feed filter change. Idempotent: a no-op when the composite is
// unchanged, so it's safe to call from several triggers.
const resolve = useCallback(() => {
// unchanged, so it's safe to call from several triggers. An effect event: the listeners below are
// bound once, not per scrollKey, and still read the live one.
const resolve = useEffectEvent(() => {
const el = elRef.current;
if (!el || scrollKeyRef.current == null) return;
const key = `${ensureNavId()}:${scrollKeyRef.current}`;
if (!el || scrollKey == null) return;
const key = `${ensureNavId()}:${scrollKey}`;
if (key === lastComposite.current) return;
lastComposite.current = key;
cancelRef.current();
cancelRef.current = restoreScroll(el, savedScroll.get(key) ?? 0);
}, []);
});
const remember = useEffectEvent((el: HTMLElement) => {
if (scrollKey == null) return;
rememberScroll(`${peekNavId()}:${scrollKey}`, el.scrollTop);
});
// Trigger 1: mount + a content-key change (page switch, feed filter) — both re-render this component.
useLayoutEffect(() => {
resolve();
}, [scrollKey, resolve]);
}, [scrollKey]);
// Trigger 2: Back/Forward (popstate) and a Plex sub-view open (nav bus) — neither necessarily
// re-renders this component. Also records the position continuously (not at unmount): the scroller's
@@ -180,20 +188,18 @@ export default function PageScroller({
useLayoutEffect(() => {
const el = elRef.current;
if (!el) return;
const onScroll = () => {
if (scrollKeyRef.current == null) return;
rememberScroll(`${peekNavId()}:${scrollKeyRef.current}`, el.scrollTop);
};
const onScroll = () => remember(el);
const onNav = () => resolve();
el.addEventListener("scroll", onScroll, { passive: true });
window.addEventListener("popstate", resolve);
const unsubNav = subscribeNav(resolve);
window.addEventListener("popstate", onNav);
const unsubNav = subscribeNav(onNav);
return () => {
cancelRef.current();
el.removeEventListener("scroll", onScroll);
window.removeEventListener("popstate", resolve);
window.removeEventListener("popstate", onNav);
unsubNav();
};
}, [resolve]);
}, []);
return (
// `id` + `tabIndex={-1}`: the skip link's target (ui/SkipLink) takes the focus, so the next Tab
@@ -204,7 +210,7 @@ export default function PageScroller({
id={CONTENT_ID}
tabIndex={-1}
{...FOCUS_REST_ATTRS}
style={fade.style}
style={fadeStyle}
className="flex-1 min-w-0 overflow-y-auto scrollbar-gutter-stable focus:outline-none"
>
<StickyTopProvider value={registerStickyTop}>{children}</StickyTopProvider>
@@ -121,7 +121,7 @@ export function PriorityStepper({
onPriority(delta);
};
const editFocus = useReturnFocus(editing);
const { target: editTarget, arm: armEditReturn } = useReturnFocus(editing);
const commitEdit = () => {
const parsed = parseInt(draft, 10);
if (!Number.isNaN(parsed) && parsed !== priority) onSet(parsed);
@@ -162,7 +162,7 @@ export function PriorityStepper({
} ${cls.value}`}
>
<button
ref={editFocus.target}
ref={editTarget}
onClick={beginEdit}
aria-label={t("common.priority.edit")}
title={t("common.priority.edit")}
@@ -180,10 +180,10 @@ export function PriorityStepper({
onBlur={commitEdit}
onKeyDown={(e) => {
if (e.key === "Enter") {
editFocus.arm(e);
armEditReturn(e);
commitEdit();
} else if (e.key === "Escape") {
editFocus.arm(e);
armEditReturn(e);
setEditing(false);
}
}}
+3 -3
View File
@@ -66,7 +66,7 @@ export default function SidePanel({
const { t } = useTranslation();
const cardRef = useRef<HTMLElement>(null);
const tabRef = useRef<HTMLButtonElement>(null);
const fade = useScrollFade();
const { ref: fadeRef, style: fadeStyle } = useScrollFade();
const peek = useHoverFocusWithin();
const drawerMode = useMediaQuery(DRAWER_QUERY, false);
const pinRoom = useMediaQuery(PIN_ROOM_QUERY, true);
@@ -234,8 +234,8 @@ export default function SidePanel({
)}
<div
ref={fade.ref}
style={fade.style}
ref={fadeRef}
style={fadeStyle}
className="flex-1 min-h-0 overflow-y-auto no-scrollbar"
>
<div className="p-3 space-y-3">{body}</div>
+2 -1
View File
@@ -48,6 +48,7 @@ export default function SortControl({
mode: "menu",
placement: "bottom-end",
});
const { setReference } = pop.refs;
// A stored sort can name an option this surface no longer offers; show it as unsorted rather than
// letting an arrow claim a sort that `sortRows`/the server won't actually apply.
@@ -71,7 +72,7 @@ export default function SortControl({
context="toolbar"
open={open}
label={label}
ref={pop.refs.setReference}
ref={setReference}
{...pop.getReferenceProps()}
// Carry the active sort key AND direction in the accessible name (the visible arrow is
// aria-hidden), matching ViewSwitcher — a screen reader announces "Sort: Date, Descending",
@@ -94,6 +94,18 @@ describe("Toaster — handing the toasts to another host", () => {
expect(screen.getByText("fresh").closest(".toast-card")!.getAttribute("role")).toBe("alert");
});
it("takes the inherited set ONCE, at the takeover: a toast of its own stays an alert", () => {
// E32: the handover is a snapshot made when the host becomes top, not a read of what the top
// host last showed — re-read on every render, this host's own "first" turned silent the moment
// "second" arrived (its own screen had become the handover by then).
render(<Toaster />);
act(() => void notify({ level: "error", message: "first" }));
act(() => void notify({ level: "error", message: "second" }));
const card = (text: string) => screen.getByText(text).closest(".toast-card")!;
expect(card("first").getAttribute("role")).toBe("alert");
expect(card("second").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
+20 -21
View File
@@ -1,4 +1,4 @@
import { useEffect, useId, useRef, useState, useSyncExternalStore } from "react";
import { useEffect, useId, useLayoutEffect, useState, useSyncExternalStore } from "react";
import { useTranslation } from "react-i18next";
import { X } from "lucide-react";
import { dismiss, getActiveToasts, subscribe } from "../../lib/notifications";
@@ -70,30 +70,29 @@ export default function Toaster({
// the takeover render. Reading the store live also silenced a toast RAISED in the same commit as
// the takeover: a dialog that notifies and closes in one handler (UploadDialog) put its success
// toast up in the very batch that handed the app host the top spot, so it was classified as
// inherited and announced by neither path (deep-review round 5). Written during render on purpose
// — the handover must be readable by the taking-over host in the SAME commit, and an effect
// cleanup runs one phase too late; only the host that IS top writes, so the value is the losing
// host's last screen either way.
// inherited and announced by neither path (deep-review round 5). The top host records its screen
// after each commit (the layout effect below); a host taking over reads it in a LATER render, after
// that commit, so it always sees the outgoing host's last committed screen. (It was written during
// render, which made the React Compiler skip the Toaster — E32 — and let a discarded render
// overwrite it.)
//
// Snapshotted through a ref keyed on `isTop`, not a `useMemo`: React re-invokes a component body
// — and every `useMemo` factory in it — on a StrictMode double render and on a restarted one, and
// the LAST invocation is what commits. With a memo the second pass re-read `lastTopIds` after the
// first pass had already overwritten it on the line below, so the takeover host classified its
// OWN new toast as inherited and the dev build behaved exactly like the code this replaced
// (measured in jsdom: role "status" without StrictMode, null with it — round 6). The ref only
// recomputes when `isTop` actually flips, so a repeated invocation of one render is a no-op.
const handover = useRef<{ top: boolean; ids: ReadonlySet<number> }>({
// Snapshotted into state when `isTop` flips, adjusted DURING render — not a `useMemo`, which React
// may recompute (and the compiler is free to): a recompute while top would re-read this host's OWN
// screen and silence its next toast. Under StrictMode the double invocation reads the same,
// unchanged global, so both passes agree (the round-6 failure was a render-phase WRITE racing it).
const [handover, setHandover] = useState<{ top: boolean; ids: ReadonlySet<number> }>({
top: false,
ids: EMPTY_IDS,
});
if (handover.current.top !== isTop)
handover.current = { top: isTop, ids: new Set(isTop ? lastTopIds : []) };
const inherited = handover.current.ids;
// AFTER the snapshot above has read the outgoing host's screen — this host's own first render as
// top would otherwise overwrite the handover with the toasts it is about to classify. Writing the
// same value twice is harmless; that is the point of the ref.
// eslint-disable-next-line react-hooks/globals -- the handover between toast hosts is module-level BY DESIGN and must be written during render — see the 12 lines above, which record why a useMemo produced the wrong answer under StrictMode. The write is idempotent, which is what makes a repeated render invocation safe.
if (isTop) lastTopIds = toasts.map((n) => n.id);
let inherited = handover.ids;
if (handover.top !== isTop) {
const next = { top: isTop, ids: new Set(isTop ? lastTopIds : []) };
setHandover(next);
inherited = next.ids;
}
useLayoutEffect(() => {
if (isTop) lastTopIds = toasts.map((n) => n.id);
});
const [armed, setArmed] = useState(false);
useEffect(() => {
if (!isTop) {
+2 -1
View File
@@ -38,12 +38,13 @@ export default function Tooltip({
// rest-before-show for labels that shouldn't flash on a fast sweep across an icon row.
hoverDelay: { open: openDelay, close: 0 },
});
const { setReference } = pop.refs;
if (!enabled || !hint) return <>{children}</>;
return (
<>
<span ref={pop.refs.setReference} {...pop.getReferenceProps()} className="inline-flex">
<span ref={setReference} {...pop.getReferenceProps()} className="inline-flex">
{children}
</span>
{/* Passive caption: it must not swallow a click aimed at a neighbouring control that it
@@ -1,4 +1,4 @@
import { useEffect, useMemo, useRef, useState } from "react";
import { useEffect, useRef, useState } from "react";
import {
useFloating,
autoUpdate,
@@ -256,12 +256,9 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) {
setActiveIndex(to);
setHoverIndex(null);
};
const pageKeyRef = useRef(onPageKey);
pageKeyRef.current = onPageKey;
const paging = useMemo(() => {
const onKeyDown = (e: React.KeyboardEvent) => pageKeyRef.current(e);
return { reference: { onKeyDown }, floating: { onKeyDown } };
}, []);
// The compiler memoizes this on what onPageKey reads (the active row), so the prop getters below
// change only when that does.
const paging = { reference: { onKeyDown: onPageKey }, floating: { onKeyDown: onPageKey } };
const interactions = useInteractions([
click,
+11 -12
View File
@@ -1,4 +1,5 @@
import { useMemo, useRef } from "react";
import { useMemo } from "react";
import { useLatest } from "../../lib/useLatest";
import { useTranslation } from "react-i18next";
import { closestCenter } from "@dnd-kit/core";
import type {
@@ -38,8 +39,7 @@ export function useSortableA11y(describe: (id: UniqueIdentifier) => SortableSubj
announcements: Announcements;
} {
const { t } = useTranslation();
const latest = useRef(describe);
latest.current = describe;
const latest = useLatest(describe);
return useMemo(() => {
// Spread into a fresh object: i18next's options type is an index signature, which a named
// interface does not satisfy.
@@ -51,22 +51,21 @@ export function useSortableA11y(describe: (id: UniqueIdentifier) => SortableSubj
// dnd-kit reports the item over ITSELF right after the pick-up, in the same tick — announced,
// "moved to position 1" replaced "picked up … use the arrow keys" in the live region before a
// screen reader read it (measured in Chrome). Only a real move is announced as one.
let moved = false;
// Per-DRAG state the announcement closures share (a component-lifetime ref would be the wrong
// lifetime): an object owned by this memo, so the closures mutate a property, never a binding.
const drag = { moved: false };
return {
screenReaderInstructions: { draggable: t("common.dnd.instructions") },
announcements: {
onDragStart: ({ active }) => {
moved = false;
drag.moved = false;
return t("common.dnd.pickedUp", at(active.id));
},
onDragOver: ({ active, over }) => {
if (!over || (!moved && over.id === active.id)) return undefined;
// `moved` is per-DRAG state, and the announcement closures capturing it is the point:
if (!over || (!drag.moved && over.id === active.id)) return undefined;
// dnd-kit reports the item over ITSELF in the pick-up tick (see above), and this flag is
// the only thing separating that from a real move. A ref would be the wrong lifetime —
// one drag, not the component.
// eslint-disable-next-line react-hooks/immutability -- per-DRAG state the announcement closures share; a ref would live as long as the component
moved = true;
// the only thing separating that from a real move.
drag.moved = true;
return t("common.dnd.movedOver", landing(active.id, over.id));
},
onDragEnd: ({ active, over }) =>
@@ -76,7 +75,7 @@ export function useSortableA11y(describe: (id: UniqueIdentifier) => SortableSubj
onDragCancel: ({ active }) => t("common.dnd.cancelled", at(active.id)),
},
};
}, [t]);
}, [t, latest]);
}
// A context that nests sortable lists (a Plex playlist: its blocks, and each show's episodes) must
+5 -4
View File
@@ -29,11 +29,11 @@ import { SURFACE } from "./surfaces";
// The multi-select filter's option list, capped and scrollable. Its own component because it owns a
// hook — see the FilterPopover note below for why nesting either of these would remount them.
function FilterOptionList({ children }: { children: ReactNode }) {
const fade = useScrollFade();
const { ref: fadeRef, style: fadeStyle } = useScrollFade();
return (
<div
ref={fade.ref}
style={fade.style}
ref={fadeRef}
style={fadeStyle}
className="flex flex-col gap-0.5 max-h-60 overflow-y-auto no-scrollbar"
>
{children}
@@ -77,13 +77,14 @@ function FilterFunnel<T>({
mode: isText ? "dialog" : "menu",
role: isText ? "dialog" : "menu",
});
const { setReference } = pop.refs;
const filterOn = isActive(value);
return (
<>
<IconButton
variant={filterOn ? "ghost-accent" : "ghost"}
ref={pop.refs.setReference}
ref={setReference}
{...pop.getReferenceProps()}
label={t("datatable.filter")}
// The icon alone was a 14x14 hit target, the smallest in the app; the sm rung is 28.