diff --git a/frontend/src/components/Playlists.tsx b/frontend/src/components/Playlists.tsx
index 8f25f8ab..fa03016d 100644
--- a/frontend/src/components/Playlists.tsx
+++ b/frontend/src/components/Playlists.tsx
@@ -634,7 +634,10 @@ export default function Playlists() {
})),
}
: // Kept visible and disabled rather than dropped: an action that vanishes teaches
- // nobody why. With one playlist there is nowhere to move to, and the title says so.
+ // nobody why. KNOWN GAP: the `title` that carries the why currently reaches nobody
+ // either — see ActionMenu's `title` prop, where the two failed attempts at fixing
+ // that are recorded. The row is honest about being off; it is not yet honest about
+ // the reason.
{
id: "move",
label: t("playlists.moveTo"),
@@ -656,8 +659,9 @@ export default function Playlists() {
label,
icon,
separatorBefore: i === 0,
- // Same gate as the drag handle, and it explains itself the same way instead of
- // silently doing nothing.
+ // Same gate as the drag handle. NOTE the drag handle explains itself and this row
+ // does not: the handle is `aria-disabled` (focusable, tooltip-capable) while a menu
+ // row cannot be — see ActionMenu's `title` prop for why.
disabled: !storedOrderEditable,
title: storedOrderEditable ? undefined : dragHint,
onSelect: () => moveInOrder(ctxRow, to),
diff --git a/frontend/src/components/PlaylistsRail.tsx b/frontend/src/components/PlaylistsRail.tsx
index 4446efe0..3442f3c8 100644
--- a/frontend/src/components/PlaylistsRail.tsx
+++ b/frontend/src/components/PlaylistsRail.tsx
@@ -163,38 +163,42 @@ export default function PlaylistsRail({
},
});
- // Which row to hand focus back to once the inline editor closes. Enter and Escape both unmount the
- // focused and re-render the row as a that nothing focused, so a keyboard user was
- // dropped on and lost their place in the rail (E10 review round 1).
- const refocusIdRef = useRef(null);
- const rowRefs = useRef(new Map());
+ // KNOWN GAP: Enter and Escape unmount the focused and re-render the row as a that
+ // nothing focuses, so a keyboard user is dropped on and loses their place in the rail (E10
+ // review round 1). A refocus-the-row fix was written in round 1 and REMOVED in round 3 because a
+ // real-browser check showed it never worked: Escape's handler does run (typing then Escape leaves
+ // the name unchanged, i.e. it cancels rather than blur-commits) and the row button is focusable
+ // when focused from outside — yet activeElement is still BODY afterwards. Three explanations were
+ // tried and none survived contact, so the machinery went rather than a fourth guess. Whoever picks
+ // this up: instrument the effect in a real browser first, do not reason about the commit order.
const renameInputRef = useRef(null);
- // NOT `autoFocus`. The context menu's own close hands focus back through floating-ui's
- // FloatingFocusManager, which does it in a `queueMicrotask` — that runs AFTER React's autoFocus,
- // so an autofocused field opened FROM the menu was blurred straight back out and its onBlur
- // committed and closed it (E10 review round 1: "the rename field flashes and vanishes").
- // A passive effect is scheduled after that microtask, so focusing here wins the race instead of
- // fighting it. Round 2 reverted the first attempt at this, which turned floating-ui's returnFocus
- // off platform-wide and broke two other surfaces; the fix belongs at the field.
+ // UNRESOLVED — do not "simplify" this back to `autoFocus` without reading the whole story.
+ //
+ // Opening this field from the context menu is a focus race: `MenuItemList` runs `onSelect()` and
+ // `onClose()` in one batch, so the field mounts in the same commit that unmounts the popover, and
+ // floating-ui's FloatingFocusManager hands focus back from a `queueMicrotask` queued in its
+ // layout-effect cleanup. If that microtask wins, the field is blurred straight back out and its
+ // own onBlur commits and closes it — "the rename field flashes and vanishes" (E10 review round 1).
+ //
+ // Three answers have been tried and none is VERIFIED. Round 1 turned floating-ui's returnFocus off
+ // platform-wide and broke two other surfaces (a non-focusable trigger; a scroll-dismiss that
+ // scrolled the list back). Round 2 replaced it with this effect, on the reasoning that a passive
+ // effect is scheduled after the microtask. Round 3 disputed that (React flushes passive effects
+ // synchronously at the end of a SyncLane commit, which would put the microtask LAST) — and a jsdom
+ // harness built to settle it reproduced no failure for ANY of autoFocus / this effect / a
+ // microtask, i.e. jsdom does not exercise the race, because focus never enters the menu there.
+ //
+ // So: this is the shape that is deliberate rather than accidental, and the question is open. It
+ // needs ONE real-browser check (open the rail's context menu, pick Rename, see whether the field
+ // survives) — not a fourth guess. Logged in the E10 plan.
useEffect(() => {
if (renamingId !== null) renameInputRef.current?.focus();
}, [renamingId]);
- useEffect(() => {
- const id = refocusIdRef.current;
- if (renamingId !== null || id === null) return;
- refocusIdRef.current = null;
- rowRefs.current.get(id)?.focus();
- }, [renamingId]);
- function endRename(pl: Playlist, refocus: boolean) {
- if (refocus) refocusIdRef.current = pl.id;
- setRenamingId(null);
- }
-
- async function commitRename(pl: Playlist, refocus = false) {
+ async function commitRename(pl: Playlist) {
const name = renameValue.trim();
- endRename(pl, refocus);
+ setRenamingId(null);
if (name && name !== pl.name) await plActions.rename(pl.id, name);
}
@@ -327,13 +331,11 @@ export default function PlaylistsRail({
ref={renameInputRef}
value={renameValue}
onChange={(e) => setRenameValue(e.target.value)}
- // A blur has already moved focus somewhere the user chose — only the keys need
- // the row handed back to them.
onBlur={() => void commitRename(pl)}
onKeyDown={(e) => {
- if (e.key === "Enter") void commitRename(pl, true);
+ if (e.key === "Enter") void commitRename(pl);
// Unmounting the input fires no blur in React, so Escape really does cancel.
- if (e.key === "Escape") endRename(pl, true);
+ if (e.key === "Escape") setRenamingId(null);
}}
aria-label={t("playlists.rename")}
className="flex-1 min-w-0 bg-surface border border-border rounded-md px-2 py-1 text-xs focus:outline-hidden focus:ring-2 focus:ring-accent/40"
@@ -342,10 +344,7 @@ export default function PlaylistsRail({
) : (
{
- rowRefs.current.set(pl.id, el);
- if (pl.id === selectedId) selectedRef.current = el;
- }}
+ ref={pl.id === selectedId ? selectedRef : null}
onClick={() => setSelectedId(pl.id)}
{...railMenu.triggerProps(pl)}
className={`${ROW_BASE} text-left ${
diff --git a/frontend/src/components/PlexPlayer.tsx b/frontend/src/components/PlexPlayer.tsx
index a4535445..ff076f6e 100644
--- a/frontend/src/components/PlexPlayer.tsx
+++ b/frontend/src/components/PlexPlayer.tsx
@@ -710,14 +710,22 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) {
stats: () => setStatsOpen((v) => !v),
back: (ev) => {
// HTPC-style "stop & back to the feed" — mirrors the mouse Back button (shared cascade).
- // Opening the cheat sheet (or the palette) over the player and pressing ⌫ used to stop
- // playback and drop to the Plex grid while the overlay stayed up: Modal stopPropagation()s
- // Escape ONLY, and its focused close button is not a typing target, so the key sailed
- // through (E10 review round 1). The first fix deferred to `anyLayerAbove`, which was far
- // too broad — the player's OWN gear/tracks menus register layers via useDismiss, so ⌫
- // stopped closing them and did nothing at all (round 2). Only these two app-wide overlays
- // own Backspace, so name them rather than every layer.
- if (helpOpenRef.current || paletteOpenRef.current) return;
+ // Who owns ⌫ when something is open over the player. Modal stopPropagation()s Escape ONLY
+ // and its focused close button is not a typing target, so the key reaches us regardless.
+ // Two wrong answers so far: deferring to nothing at all (round 1's bug — the cheat sheet
+ // took ⌫ and the player stopped underneath it), then deferring to `anyLayerAbove` outright
+ // (round 1's fix, round 2's bug — the player's OWN gear/tracks popovers push layers too, so
+ // ⌫ stopped closing them), then naming just the sheet and palette (round 2's fix, round 3's
+ // bug — the app-wide ErrorDialog is neither, so ⌫ tore the player down under it).
+ // The rule that holds: the player's own panels answer first, because `handleBack`'s cascade
+ // is what closes them; anything else layered above us takes the key.
+ const ownPanelOpen =
+ menuOpenRef.current ||
+ infoOpenRef.current ||
+ statsOpenRef.current ||
+ upNextOpenRef.current;
+ if (!ownPanelOpen && layerIdRef.current != null && anyLayerAbove(layerIdRef.current))
+ return;
ev.preventDefault();
if (skipProgressRef.current != null)
cancelAutoSkipRef.current(); // cancel auto-skip, don't go back