refactor(ui): reuse the store, media-query and deleted-tag rules the review found rebuilt, and name a loading pick
E32 review round 1 (F3–F6). - Channels' open-row store and the chat dock use lib/store's createStore instead of a hand-rolled listener set (the chat dock gets a test of its own). - The Plex backdrop reads prefers-reduced-motion through useMediaQuery; the query string is one constant (REDUCED_MOTION) for the hook and the carousel's one-off read. - A link-restored collection filter showed its raw rating_key in the picker while the titles loaded: usePlexCollectionTitles now says it is pending, the picker draws a placeholder (labelsPending, as the channel picker does) and so does the chip row. - The "deleted tag" rule was written twice with two i18n keys and had drifted: the channel manager showed NO chip for a pick while the tag list loaded, or ever after a failed load, while the pick still narrowed the table. One rule now (lib/tagPicks), one key (common.deletedTag); the manager's row shows every pick, as a placeholder while loading and as its id after a failure.
This commit is contained in:
@@ -0,0 +1,52 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// The floating chat dock's store (E32 review: moved onto lib/store's createStore). It restores the
|
||||
// signed-in user's windows, keeps at most three, flashes a window a message arrives for, and
|
||||
// persists everything but the transient flash counter.
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import {
|
||||
closeChat,
|
||||
initDock,
|
||||
notifyIncoming,
|
||||
openChat,
|
||||
toggleMinimize,
|
||||
useDockChats,
|
||||
} from "./chatDock";
|
||||
import { LS } from "./storage";
|
||||
|
||||
const partner = (id: number) => ({ id, name: `P${id}`, avatar_url: null });
|
||||
|
||||
describe("the chat dock", () => {
|
||||
it("restores, caps, flashes and persists the user's windows", () => {
|
||||
localStorage.setItem(
|
||||
LS.chatDock(7),
|
||||
JSON.stringify([{ partnerId: 1, name: "P1", avatar: null, minimized: true }]),
|
||||
);
|
||||
const { result } = renderHook(() => useDockChats());
|
||||
act(() => initDock(7));
|
||||
expect(result.current).toEqual([
|
||||
{ partnerId: 1, name: "P1", avatar: null, minimized: true, flash: 0 },
|
||||
]);
|
||||
|
||||
act(() => notifyIncoming(partner(1))); // already open: flash it, keep it minimised
|
||||
expect(result.current[0]).toMatchObject({ partnerId: 1, minimized: true, flash: 1 });
|
||||
|
||||
act(() => {
|
||||
for (const id of [2, 3, 4]) openChat(partner(id));
|
||||
});
|
||||
expect(result.current.map((c) => c.partnerId)).toEqual([4, 3, 2]); // newest first, three at most
|
||||
|
||||
act(() => toggleMinimize(3));
|
||||
act(() => closeChat(2));
|
||||
expect(result.current.map((c) => [c.partnerId, c.minimized])).toEqual([
|
||||
[4, false],
|
||||
[3, true],
|
||||
]);
|
||||
const stored = JSON.parse(localStorage.getItem(LS.chatDock(7))!) as object[];
|
||||
expect(stored).toEqual([
|
||||
{ partnerId: 4, name: "P4", avatar: null, minimized: false },
|
||||
{ partnerId: 3, name: "P3", avatar: null, minimized: true },
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -2,8 +2,8 @@
|
||||
// A window can be opened from anywhere (a conversation's "pop out" button, or automatically
|
||||
// when a message arrives) and stays open across page navigation. The dock state (which chats
|
||||
// are open + their minimised state) is persisted per user so it survives a reload.
|
||||
import { useSyncExternalStore } from "react";
|
||||
import { LS } from "./storage";
|
||||
import { createStore } from "./store";
|
||||
|
||||
export interface DockChat {
|
||||
partnerId: number;
|
||||
@@ -16,9 +16,8 @@ export interface DockChat {
|
||||
type PartnerLike = { id: number; name: string; avatar_url: string | null };
|
||||
|
||||
const MAX_OPEN = 3; // keep the dock from overflowing the corner
|
||||
let chats: DockChat[] = [];
|
||||
const dock = createStore<DockChat[]>([]);
|
||||
let userId: number | null = null;
|
||||
const subs = new Set<() => void>();
|
||||
|
||||
function storageKey(): string | null {
|
||||
return userId != null ? LS.chatDock(userId) : null;
|
||||
@@ -29,16 +28,15 @@ function persist(): void {
|
||||
if (!k) return;
|
||||
try {
|
||||
// Don't persist the transient flash counter.
|
||||
localStorage.setItem(k, JSON.stringify(chats.map(({ flash, ...c }) => c)));
|
||||
localStorage.setItem(k, JSON.stringify(dock.get().map(({ flash, ...c }) => c)));
|
||||
} catch {
|
||||
/* ignore quota/serialization errors */
|
||||
}
|
||||
}
|
||||
|
||||
function emit(next: DockChat[]): void {
|
||||
chats = next;
|
||||
dock.set(next);
|
||||
persist();
|
||||
for (const s of subs) s();
|
||||
}
|
||||
|
||||
// Bind the dock to the signed-in user and restore their persisted windows (once per user).
|
||||
@@ -53,15 +51,17 @@ export function initDock(uid: number): void {
|
||||
} catch {
|
||||
restored = [];
|
||||
}
|
||||
chats = restored.slice(0, MAX_OPEN);
|
||||
for (const s of subs) s();
|
||||
dock.set(restored.slice(0, MAX_OPEN));
|
||||
}
|
||||
|
||||
export function openChat(partner: PartnerLike): void {
|
||||
const existing = chats.find((c) => c.partnerId === partner.id);
|
||||
const existing = dock.get().find((c) => c.partnerId === partner.id);
|
||||
if (existing) {
|
||||
// Re-opening focuses it: ensure expanded and move to the front.
|
||||
emit([{ ...existing, minimized: false }, ...chats.filter((c) => c.partnerId !== partner.id)]);
|
||||
emit([
|
||||
{ ...existing, minimized: false },
|
||||
...dock.get().filter((c) => c.partnerId !== partner.id),
|
||||
]);
|
||||
return;
|
||||
}
|
||||
emit(
|
||||
@@ -73,7 +73,7 @@ export function openChat(partner: PartnerLike): void {
|
||||
minimized: false,
|
||||
flash: 0,
|
||||
},
|
||||
...chats,
|
||||
...dock.get(),
|
||||
].slice(0, MAX_OPEN),
|
||||
);
|
||||
}
|
||||
@@ -81,7 +81,7 @@ export function openChat(partner: PartnerLike): void {
|
||||
// An incoming message arrived from `partner`: open the window if it's closed, otherwise just
|
||||
// flash it (whether expanded or minimised — don't disturb the minimised state).
|
||||
export function notifyIncoming(partner: PartnerLike): void {
|
||||
const existing = chats.find((c) => c.partnerId === partner.id);
|
||||
const existing = dock.get().find((c) => c.partnerId === partner.id);
|
||||
if (!existing) {
|
||||
emit(
|
||||
[
|
||||
@@ -92,29 +92,22 @@ export function notifyIncoming(partner: PartnerLike): void {
|
||||
minimized: false,
|
||||
flash: 1,
|
||||
},
|
||||
...chats,
|
||||
...dock.get(),
|
||||
].slice(0, MAX_OPEN),
|
||||
);
|
||||
return;
|
||||
}
|
||||
emit(chats.map((c) => (c.partnerId === partner.id ? { ...c, flash: c.flash + 1 } : c)));
|
||||
emit(dock.get().map((c) => (c.partnerId === partner.id ? { ...c, flash: c.flash + 1 } : c)));
|
||||
}
|
||||
|
||||
export function closeChat(partnerId: number): void {
|
||||
emit(chats.filter((c) => c.partnerId !== partnerId));
|
||||
emit(dock.get().filter((c) => c.partnerId !== partnerId));
|
||||
}
|
||||
|
||||
export function toggleMinimize(partnerId: number): void {
|
||||
emit(chats.map((c) => (c.partnerId === partnerId ? { ...c, minimized: !c.minimized } : c)));
|
||||
emit(dock.get().map((c) => (c.partnerId === partnerId ? { ...c, minimized: !c.minimized } : c)));
|
||||
}
|
||||
|
||||
export function useDockChats(): DockChat[] {
|
||||
return useSyncExternalStore(
|
||||
(cb) => {
|
||||
subs.add(cb);
|
||||
return () => subs.delete(cb);
|
||||
},
|
||||
() => chats,
|
||||
() => chats,
|
||||
);
|
||||
return dock.useValue();
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { useLayoutEffect, useSyncExternalStore } from "react";
|
||||
import { useLayoutEffect } from "react";
|
||||
import { REDUCED_MOTION, useMediaQuery } from "./useMediaQuery";
|
||||
import { usePref, useSavePref } from "./usePref";
|
||||
import type { TFunction } from "i18next";
|
||||
import type { PlexCard, PlexTmdb } from "./api";
|
||||
@@ -7,18 +8,6 @@ import type { PlexCard, PlexTmdb } from "./api";
|
||||
// prefs, the TMDB backdrop URL, the crossfading art backdrop hook and the episode label. Split out so
|
||||
// plexDetailUi exports only components (Fast Refresh).
|
||||
|
||||
const REDUCED_MOTION = "(prefers-reduced-motion: reduce)";
|
||||
const reducedMotion = () =>
|
||||
typeof window !== "undefined" &&
|
||||
typeof window.matchMedia === "function" &&
|
||||
window.matchMedia(REDUCED_MOTION).matches;
|
||||
const onReducedMotionChange = (cb: () => void) => {
|
||||
if (typeof window === "undefined" || typeof window.matchMedia !== "function") return () => {};
|
||||
const m = window.matchMedia(REDUCED_MOTION);
|
||||
m.addEventListener("change", cb);
|
||||
return () => m.removeEventListener("change", cb);
|
||||
};
|
||||
|
||||
export function useDetailPrefs() {
|
||||
// Three per-user detail-page toggles, each a server pref adopted on login (usePref). Default ON:
|
||||
// undefined → true, only an explicit false hides. The detail pages mount after `me` is cached, so
|
||||
@@ -31,7 +20,7 @@ export function useDetailPrefs() {
|
||||
// Windows also reports whenever its animation effects are off). Once the reader turns it on or off
|
||||
// here, that pick wins on every device: the owner never saw the crossfade, because the system signal
|
||||
// used to switch it off whatever the toggle said (2026-10-03). The toggle shows the state in force.
|
||||
const reduce = useSyncExternalStore(onReducedMotionChange, reducedMotion, () => false);
|
||||
const reduce = useMediaQuery(REDUCED_MOTION, false);
|
||||
const [crossfadePick, setCrossfade] = usePref<boolean | null>("plexInfoArtCrossfade", null, {
|
||||
parse: (raw) => (typeof raw === "boolean" ? raw : null),
|
||||
});
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
import type { TFunction } from "i18next";
|
||||
import type { Tag } from "./api";
|
||||
|
||||
// A saved view or a stored filter can outlive its tag (deleted here or on another device). The filter
|
||||
// then matches nothing (E32 S2), so its chip must say why instead of reading as a bare id — but only
|
||||
// once the tag list has loaded: while it loads the chip is a placeholder (`isPending`), and if the load
|
||||
// failed it shows the id, never a guessed "deleted". One rule for the feed's applied-filter row and the
|
||||
// channel manager's tag row, which had each written their own and drifted (E32 review).
|
||||
|
||||
/** The tag list as react-query hands it over — only what the rule reads. */
|
||||
export interface TagList {
|
||||
data?: Tag[];
|
||||
isSuccess: boolean;
|
||||
isPending: boolean;
|
||||
}
|
||||
|
||||
/** A picked tag's chip label: its name, "Deleted tag" once the list says it is gone, else the id. */
|
||||
export function tagPickLabel(id: number, tags: TagList, t: TFunction): string {
|
||||
const tag = tags.data?.find((x) => x.id === id);
|
||||
if (tag) return tag.name;
|
||||
return tags.isSuccess ? t("common.deletedTag") : String(id);
|
||||
}
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { TFunction } from "i18next";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { DEFAULT_TAG_MODES, type FeedFilters, type Tag } from "./api";
|
||||
import { DEFAULT_TAG_MODES, type FeedFilters } from "./api";
|
||||
import { tagPickLabel } from "./tagPicks";
|
||||
import { useChannelNames } from "./useChannelNames";
|
||||
import { parseSort } from "./feedSort";
|
||||
import { useTags } from "./useTags";
|
||||
@@ -36,15 +36,6 @@ export function clearedFilters(current: FeedFilters): FeedFilters {
|
||||
return { ...DEFAULT_SIDEBAR_FILTERS, q: current.q, scope: "my" };
|
||||
}
|
||||
|
||||
// A tag chip's name. A saved view or the stored filter can outlive its tag: the feed then matches
|
||||
// nothing (E32 S2), and the chip says why instead of reading as a bare id — but only once the tag
|
||||
// list has loaded; while it loads (a placeholder) or if it failed, the id, never a guessed "deleted".
|
||||
function tagLabel(id: number, tags: { data?: Tag[]; isSuccess: boolean }, t: TFunction): string {
|
||||
const tag = tags.data?.find((x) => x.id === id);
|
||||
if (tag) return tag.name;
|
||||
return tags.isSuccess ? t("feed.chips.deletedTag") : String(id);
|
||||
}
|
||||
|
||||
/**
|
||||
* The single source of truth for "what is currently narrowing the feed" — it backs BOTH the side
|
||||
* panel's active-count badge and the removable chip row above the feed, so the two can never
|
||||
@@ -68,7 +59,7 @@ export function useActiveFilters(
|
||||
for (const id of filters.tags) {
|
||||
out.push({
|
||||
key: `tag:${id}`,
|
||||
label: tagLabel(id, tagsQuery, t),
|
||||
label: tagPickLabel(id, tagsQuery, t),
|
||||
pending: tagsQuery.isPending,
|
||||
remove: () => setFilters({ ...filters, tags: filters.tags.filter((x) => x !== id) }),
|
||||
});
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
import { useSyncExternalStore } from "react";
|
||||
|
||||
/** The reader asked the system for less motion (Windows also reports it whenever its animation
|
||||
* effects are off). One spelling for the hook and for a one-off `matchMedia` read alike. */
|
||||
export const REDUCED_MOTION = "(prefers-reduced-motion: reduce)";
|
||||
|
||||
/** Whether a CSS media query matches, live. For a component that must RENDER differently at a
|
||||
* breakpoint rather than merely hide one of two renders with CSS (`ui/DataTable` mounted its table
|
||||
* AND its card list at every width: double the DOM, duplicate ids and keyboard targets). Where
|
||||
|
||||
@@ -44,7 +44,7 @@ describe("usePlexCollectionTitles", () => {
|
||||
wrapper: wrap(),
|
||||
});
|
||||
await waitFor(() =>
|
||||
expect(result.current["17045"]).toBe("Pirates of the Caribbean Collection"),
|
||||
expect(result.current.titles["17045"]).toBe("Pirates of the Caribbean Collection"),
|
||||
);
|
||||
// Only the missing ids are asked for — nothing else.
|
||||
expect(plexCollections).toHaveBeenCalledWith(undefined, undefined, ["17045"]);
|
||||
@@ -55,7 +55,7 @@ describe("usePlexCollectionTitles", () => {
|
||||
const { result } = renderHook(() => usePlexCollectionTitles(["17045"], known), {
|
||||
wrapper: wrap(),
|
||||
});
|
||||
expect(result.current["17045"]).toBe("Pirates of the Caribbean Collection");
|
||||
expect(result.current.titles["17045"]).toBe("Pirates of the Caribbean Collection");
|
||||
// Give a would-be fetch a chance to happen before asserting it did not.
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
expect(plexCollections).not.toHaveBeenCalled();
|
||||
@@ -66,10 +66,10 @@ describe("usePlexCollectionTitles", () => {
|
||||
() => usePlexCollectionTitles(["17045", "68231"], { "17045": "Already Known" }),
|
||||
{ wrapper: wrap() },
|
||||
);
|
||||
await waitFor(() => expect(result.current["68231"]).toBe("IMDb Top 250"));
|
||||
await waitFor(() => expect(result.current.titles["68231"]).toBe("IMDb Top 250"));
|
||||
// The known id is not in the request — and its stored title survives the merge.
|
||||
expect(plexCollections).toHaveBeenCalledWith(undefined, undefined, ["68231"]);
|
||||
expect(result.current["17045"]).toBe("Already Known");
|
||||
expect(result.current.titles["17045"]).toBe("Already Known");
|
||||
});
|
||||
|
||||
it("treats the same ids in a different order as one query", async () => {
|
||||
@@ -77,12 +77,12 @@ describe("usePlexCollectionTitles", () => {
|
||||
const a = renderHook(() => usePlexCollectionTitles(["68231", "17045"], undefined), {
|
||||
wrapper: w,
|
||||
});
|
||||
await waitFor(() => expect(a.result.current["17045"]).toBeDefined());
|
||||
await waitFor(() => expect(a.result.current.titles["17045"]).toBeDefined());
|
||||
const calls = plexCollections.mock.calls.length;
|
||||
const b = renderHook(() => usePlexCollectionTitles(["17045", "68231"], undefined), {
|
||||
wrapper: w,
|
||||
});
|
||||
await waitFor(() => expect(b.result.current["17045"]).toBeDefined());
|
||||
await waitFor(() => expect(b.result.current.titles["17045"]).toBeDefined());
|
||||
expect(plexCollections.mock.calls.length).toBe(calls);
|
||||
});
|
||||
|
||||
@@ -91,7 +91,21 @@ describe("usePlexCollectionTitles", () => {
|
||||
const { result } = renderHook(() => usePlexCollectionTitles(["17045"], undefined), {
|
||||
wrapper: wrap(),
|
||||
});
|
||||
// The caller falls back to its own label; it must not be handed a half-built map.
|
||||
expect(result.current["17045"]).toBeUndefined();
|
||||
// The caller falls back to its own label; it must not be handed a half-built map — and it is
|
||||
// told the names are on their way, so it draws a placeholder rather than the raw id.
|
||||
expect(result.current.titles["17045"]).toBeUndefined();
|
||||
expect(result.current.pending).toBe(true);
|
||||
});
|
||||
|
||||
it("is not pending when nothing is missing, nor once the lookup answered", async () => {
|
||||
const known = renderHook(() => usePlexCollectionTitles(["17045"], { "17045": "Known" }), {
|
||||
wrapper: wrap(),
|
||||
});
|
||||
expect(known.result.current.pending).toBe(false);
|
||||
const cold = renderHook(() => usePlexCollectionTitles(["17045"], undefined), {
|
||||
wrapper: wrap(),
|
||||
});
|
||||
await waitFor(() => expect(cold.result.current.pending).toBe(false));
|
||||
expect(cold.result.current.titles["17045"]).toBe("Pirates of the Caribbean Collection");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,11 +10,13 @@ import { qk } from "./queryKeys";
|
||||
// This resolves the missing ones from the server and merges them over what the filter state already
|
||||
// knows. It stays OUT of `collectionTitles`: that map is hashed into the Plex query keys, so writing a
|
||||
// resolved title back in would split the cache from the logically identical state and refetch the grid
|
||||
// for a label. Titles are display-only, so they belong in a display-only read.
|
||||
// for a label. Titles are display-only, so they belong in a display-only read. `pending` says the
|
||||
// missing ones are still on their way, so a token or chip can show a placeholder rather than the raw id
|
||||
// (Combobox `labelsPending`, ActiveFilter `pending` — the feed's channel picker does the same).
|
||||
export function usePlexCollectionTitles(
|
||||
ids: string[],
|
||||
known: Record<string, string> | undefined,
|
||||
): Record<string, string> {
|
||||
): { titles: Record<string, string>; pending: boolean } {
|
||||
// Sorted so the same set of ids in a different order is the same query.
|
||||
const missing = ids.filter((id) => !known?.[id]).sort();
|
||||
const q = useQuery({
|
||||
@@ -25,7 +27,7 @@ export function usePlexCollectionTitles(
|
||||
// of the sidebar and the chip row.
|
||||
staleTime: 60 * 60_000,
|
||||
});
|
||||
const out: Record<string, string> = { ...known };
|
||||
for (const c of q.data?.collections ?? []) out[c.id] = c.title;
|
||||
return out;
|
||||
const titles: Record<string, string> = { ...known };
|
||||
for (const c of q.data?.collections ?? []) titles[c.id] = c.title;
|
||||
return { titles, pending: missing.length > 0 && q.isPending };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user