Files
siftlode/frontend/src/components/comfortPin.tsx
T
peter e4ffd59ca7 fix(e24): review round 2 — a buried history entry, a stale hero, and a spinner that was not ours
Nine of the round's ten surviving findings. The tenth (a failed shelf reorder leaving an
order the server rejected on screen) is on ROADMAP.md: its obvious remedy reintroduces the
overlapping-move defect the code already documents, so it wants a design, not a review edit.

- `navigateOverOverlays` buried the live overlay entries before knowing whether the
  navigation pushed one. `setPage` no-ops when the page is already current and Home is the
  default landing page, so Shift+W over a player ON Home marked its entry buried under a
  push that never happened: the unmount consumed the mark instead of reclaiming the entry,
  and the next browser Back was a press that did nothing. `setPage` now reports whether it
  navigated, and one `buryUnder` rule serves both entry points — `closeThenNavigate` takes
  the same no-op navigation from a palette command for the page you are already on.
- The hero could answer a new time budget with the previous one's pick. The chip row resets
  seed/seen/press but cannot reset an answer that already resolved, and `keepPreviousData`
  chained through the disabled seed-0 query — so a reroll under "10 perc" showed the film
  picked for "Bármennyi" while it loaded, and kept it if the request failed. `lastPick` now
  carries the server's own budget echo, and the placeholder is scoped to the budget.
- The reroll button spun and swallowed clicks after mark-watched, bookmark, pin and shelf
  moves: the pick key sits under the invalidated `discover` prefix on purpose, so raw
  `isFetching` was true for background refetches. It now means "a press of mine is
  outstanding" — which still covers the first press, where there is no placeholder.
- Failed writes on Home toasted the page's LOAD-failure string over a fully loaded page;
  they get their own key (HU+EN). The empty-state reroll had no `loading`, so the press was
  unacknowledged and a second click skipped a tier. `danger` moved into the shared unpin row
  so the same command is not red on the shelf and neutral in the rail below it.
- Two comments claimed things the code does not do: the `exclude` cap guards no "unbounded
  scan" (it never reaches SQL), and the route test that named the cap sends three keys.

Tests: the burial rule gets its own file (it is the one assertion that lets a reconcile
issue a real programmatic back()); App's keymap wiring gets a source contract, because
dropping it left all 818 tests green; `add_pin`'s IntegrityError arm was reached by nothing.
Every new test mutation-checked red, both edges where the predicate has two.

Gate: ruff, ruff format, 3x tsc, eslint (0 errors), prettier, knip, vitest 822, pytest 1005.
2026-09-09 00:12:53 +02:00

210 lines
9.2 KiB
TypeScript

import { useCallback, useEffect, useState } from "react";
import { useTranslation } from "react-i18next";
import { useQuery, useQueryClient } from "@tanstack/react-query";
import { Loader2, Pin, PinOff } from "lucide-react";
import { api, type DiscoverCard, type PinKind } from "../lib/api";
import { notifyActionError } from "../lib/actionErrors";
import { notify } from "../lib/notifications";
import { qk } from "../lib/queryKeys";
import { type ActionMenuItem } from "./ui/ActionMenu";
// E24 S3 — "put this on my comfort shelf", from wherever you happen to be looking at the thing.
//
// The shelf is the Home surface's object, but the GESTURE belongs on every card and row in the app:
// you decide something is comfort media while you are watching it, not while you are on a page that
// already lists it. So this file is the one place the pin lives, and every surface composes the same
// `ActionMenuItem` into the menu it already has (`Playlists`, `PlaylistsRail`) or mounts the
// `ComfortPinMenu` wrapper when it has none.
//
// Feature layer, not `components/ui/`: it speaks a domain API slice, which the platform boundary
// (eslint `no-restricted-imports`) forbids a `ui/` primitive from doing. The MENU SURFACE that
// renders this item lives in `ComfortPinMenu.tsx` — the same hook/component split, for the same
// react-refresh reason, as `useContextMenu.ts` beside `ContextMenu.tsx`.
/** What the API needs to pin something: the kind, and the id the rest of the API already speaks for
* that kind (a video id, a channel id, a Plex `rating_key`, a playlist id). Mirrors
* `app/discover/pins.py::PIN_COLUMNS`. */
export interface PinTarget {
kind: PinKind;
ref: string;
}
// Constructors rather than five call sites writing the kind strings by hand. The mapping from a
// domain object to its pin kind is an invariant, so it lives in a function — a `"plex_show"` typo'd
// as `"plex-show"` at one of eight call sites would be a 404 nobody notices until they try it.
export const pinVideo = (videoId: string): PinTarget => ({ kind: "video", ref: videoId });
export const pinChannel = (channelId: string): PinTarget => ({ kind: "channel", ref: channelId });
export const pinPlaylist = (playlistId: number): PinTarget => ({
kind: "playlist",
ref: String(playlistId),
});
/** A Plex tile. A SHOW is pinned as a show ("put that series on" resolves to whichever episode is
* next); a movie or a single episode is pinned as the leaf item it is. */
export const pinPlex = (card: { id: string; type: string }): PinTarget => ({
kind: card.type === "show" ? "plex_show" : "plex_item",
ref: card.id,
});
/** A unified discover card back to the thing it was pinned as — the inverse of the mapping in
* `app/discover/cards.py`. Null for a kind that cannot be pinned. */
export function pinTargetForCard(card: DiscoverCard): PinTarget | null {
if (card.source === "plex") return pinPlex({ id: card.id, type: card.kind });
if (card.kind === "video") return pinVideo(card.id);
if (card.kind === "channel") return pinChannel(card.id);
if (card.kind === "playlist") return { kind: "playlist", ref: card.id };
return null;
}
/** Where a shelf move lands. The shelf is a HORIZONTAL rail, so the words are left/right rather than
* the playlist table's up/down — the same four moves, named after what the user actually sees. */
export type ShelfMove = "first" | "left" | "right" | "last";
/** `from` -> the index the move targets, or -1 when there is nowhere to go. Pure, so the disabled
* state of a menu row and the move it would perform are decided by ONE rule instead of two that can
* disagree (a row that looks available and then no-ops is the worst of the three states). */
export function shelfMoveTarget(from: number, to: ShelfMove, length: number): number {
if (from < 0 || from >= length) return -1;
const target =
to === "first" ? 0 : to === "last" ? length - 1 : to === "left" ? from - 1 : from + 1;
return target === from || target < 0 || target >= length ? -1 : target;
}
/** The identity a target is looked up by. One function, so "is this pinned?" and "what did the
* server give back?" can never key the shelf two different ways. */
const targetKey = (target: PinTarget): string => `${target.kind}:${target.ref}`;
export interface ComfortPin {
/** `pinIdOf` and `toggle` stay module-local until a second consumer needs them: publishing them
* invites the next surface to re-derive the three-state loading dance (pinned / not pinned /
* not known yet) that `item` already encapsulates, which is what this file exists to prevent. */
remove: (pinId: number, title: string) => void;
/** The pin/unpin row, ready to spread into any `ActionMenuItem[]`. Empty for a null target. */
item: (target: PinTarget | null, title: string) => ActionMenuItem[];
}
/**
* The shelf's client half.
*
* `open` is the host menu's own open flag. The shelf is only fetched once a menu has actually been
* opened (and stays cached afterwards), because the pinned-ness of a row is needed to LABEL one menu
* item and nothing else — firing `/api/discover/pins`, which resolves a `next` per non-leaf pin, on
* every feed load would be a real request for a string almost nobody reads that session.
*/
export function useComfortPin(open: boolean): ComfortPin {
const { t } = useTranslation();
const qc = useQueryClient();
// Latch: react-query keeps the data cached when `enabled` goes false again, so this only decides
// WHEN the first fetch happens, not whether the answer survives the menu closing.
const [everOpen, setEverOpen] = useState(false);
useEffect(() => {
if (open) setEverOpen(true);
}, [open]);
const shelf = useQuery({
queryKey: qk.comfortPins(),
queryFn: () => api.comfortPins(),
enabled: everOpen,
// The shelf is hand-curated and short; re-asking on every menu open would be noise.
staleTime: 60_000,
});
const pins = shelf.data?.items;
const pinIdOf = useCallback(
(target: PinTarget): number | null | undefined => {
if (!pins) return undefined;
const key = targetKey(target);
for (const card of pins) {
const other = pinTargetForCard(card);
if (other && targetKey(other) === key) return card.pin_id ?? null;
}
return null;
},
[pins],
);
const invalidate = useCallback(() => {
void qc.invalidateQueries({ queryKey: qk.comfortPins() });
// The prefix, not an exact key: the Home payload is per-budget, and pinning something can change
// the hero as well as the shelf.
void qc.invalidateQueries({ queryKey: qk.discover() });
}, [qc]);
const add = useCallback(
async (target: PinTarget, title: string) => {
try {
await api.addComfortPin(target.kind, target.ref);
// The shelf lives on another page, so without this the click has no visible consequence at
// all from the feed — the one place a toast genuinely carries the whole feedback.
notify({ level: "success", message: t("home.pin.added", { title }) });
} catch (e) {
notifyActionError(e, t("home.pin.addFailed"));
}
invalidate();
},
[invalidate, t],
);
const remove = useCallback(
(pinId: number, title: string) => {
void (async () => {
try {
await api.removeComfortPin(pinId);
notify({ level: "success", message: t("home.pin.removed", { title }) });
} catch (e) {
notifyActionError(e, t("home.pin.removeFailed"));
}
invalidate();
})();
},
[invalidate, t],
);
const toggle = useCallback(
(target: PinTarget, title: string) => {
const pinId = pinIdOf(target);
if (pinId == null) void add(target, title);
else remove(pinId, title);
},
[add, pinIdOf, remove],
);
const item = useCallback(
(target: PinTarget | null, title: string): ActionMenuItem[] => {
if (!target) return [];
const pinId = pinIdOf(target);
// Still loading: an honest "we don't know yet" row rather than a confident "Pin" over
// something that is already on the shelf. Only ever seen on the first menu open of a session.
if (pinId === undefined)
return [
{
id: "comfort-pin",
label: t("home.pin.add"),
icon: <Loader2 className="w-4 h-4 animate-spin" />,
separatorBefore: true,
disabled: true,
onSelect: () => {},
},
];
return [
{
id: "comfort-pin",
label: pinId === null ? t("home.pin.add") : t("home.pin.remove"),
icon: pinId === null ? <Pin className="w-4 h-4" /> : <PinOff className="w-4 h-4" />,
separatorBefore: true,
// One row, two states, and only ONE of them is destructive — so `danger` flips with the
// label and the icon rather than being decided at a call site. The shelf's own hand-rolled
// unpin row on Home got the flag first; leaving this one neutral painted the identical
// command (same label, same icon, same `removeComfortPin`) two different colours on one
// screen — the hero's menu and the Continue-watching rail come through here (round 2).
danger: pinId !== null,
onSelect: () => toggle(target, title),
},
];
},
[pinIdOf, t, toggle],
);
return { remove, item };
}