fix(ui,i18n): E30 review round 3 — the stored language follows the screen, a fresh text starts collapsed, Retry waits again

This commit is contained in:
2026-10-01 10:43:59 +02:00
parent 08e62bbb16
commit 3563858fd1
14 changed files with 147 additions and 32 deletions
+23 -1
View File
@@ -8,7 +8,7 @@
//
// Real NavigationProvider, real storage and i18n; only the two boot probes are stubbed. The probe
// records EVERY render, so "never showed the wrong page, not even for one render" is what is asserted.
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from "vitest";
import { render, screen, cleanup, act, fireEvent, waitFor } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { useEffect } from "react";
@@ -47,6 +47,7 @@ function mount() {
return qc;
}
const account = (over: Partial<Me> = {}) => ({ id: 5, preferences: {}, ...over }) as Me;
let savePrefs: MockInstance<typeof api.savePrefs>;
const retryButton = () => screen.findByRole("button", { name: i18n.t("common.retry") });
beforeEach(async () => {
@@ -57,6 +58,7 @@ beforeEach(async () => {
mounts = 0;
await i18n.changeLanguage("en");
vi.spyOn(api, "setupStatus").mockResolvedValue({ configured: true });
savePrefs = vi.spyOn(api, "savePrefs").mockResolvedValue({ ok: true } as never);
});
afterEach(() => {
cleanup();
@@ -85,6 +87,26 @@ describe("BootGate", () => {
expect(renders.map((r) => r.lang)).toEqual(renders.map(() => "hu"));
});
it("an account with no language stored stores the one it is shown in, once", async () => {
await i18n.changeLanguage("hu"); // the browser's language, never saved
vi.spyOn(api, "me").mockResolvedValue(account());
const qc = mount();
await screen.findByTestId("app");
await waitFor(() => expect(savePrefs).toHaveBeenCalledWith({ language: "hu" }));
expect(savePrefs).toHaveBeenCalledTimes(1);
// The server now sorts in Hungarian, and so does what reads the stored language.
await waitFor(() => expect(qc.getQueryData<Me>(qk.me())?.preferences.language).toBe("hu"));
});
it("an account with its language stored stores nothing", async () => {
vi.spyOn(api, "me").mockResolvedValue(account({ preferences: { language: "hu" } }));
mount();
await screen.findByTestId("app");
expect(savePrefs).not.toHaveBeenCalled();
});
it("mounts nothing of the app while the identity is still unknown", async () => {
vi.spyOn(api, "me").mockReturnValue(new Promise<Me | null>(() => {}));
mount();
+21 -5
View File
@@ -1,7 +1,9 @@
import { useLayoutEffect, useRef, useState, type ReactNode } from "react";
import { useQuery } from "@tanstack/react-query";
import { useQuery, useQueryClient } from "@tanstack/react-query";
import { useTranslation } from "react-i18next";
import { api, getActiveAccount, setActiveAccount } from "../lib/api";
import { log } from "../lib/log";
import { saveLanguage } from "../lib/saveLanguage";
import { qk } from "../lib/queryKeys";
import i18n, { isSupported, setLanguage } from "../i18n";
import { LoadingState, StateMessage } from "./ui/QueryState";
@@ -38,6 +40,7 @@ export function BootScreen() {
export default function BootGate({ children }: { children: ReactNode }) {
const { t } = useTranslation();
const qc = useQueryClient();
// First-run install gate: a fresh instance reports configured=false and runs in setup mode, where
// every other route is locked — so `me` is held back until the instance is known to be set up
// (otherwise it would 503 against the setup-mode lock).
@@ -81,7 +84,10 @@ export default function BootGate({ children }: { children: ReactNode }) {
// not is corrected here — otherwise the tab would run as the default account on the server while
// reading and writing the gone account's local state.
// • Adopt the account's language, which follows the account across devices — the local copy only
// knows what this browser last showed.
// knows what this browser last showed. An account with none stored yet (the setup wizard's admin,
// an account older than the stamp sign-up gives) stores the language it is shown in: the server
// orders its lists and writes its mail in the stored one, else English, and a Hungarian screen
// over English-ordered lists breaks "A→Z in the UI language's alphabet".
const [readyFor, setReadyFor] = useState<number | null | undefined>(undefined);
// `me` is read for the identity it produced, so it rides a ref: a background refetch of the same
// account must not re-run the adoption over a language the user has just changed. That rule used
@@ -104,11 +110,21 @@ export default function BootGate({ children }: { children: ReactNode }) {
const account = meRef.current;
if (account && getActiveAccount() !== account.id) setActiveAccount(account.id);
const lang = account?.preferences?.language;
// eslint-disable-next-line local/locale-format -- a one-shot read in an effect, compared before adopting the account's language; nothing renders it, so there is no memoized value to go stale.
if (typeof lang === "string" && isSupported(lang) && lang !== i18n.language) setLanguage(lang);
if (typeof lang === "string" && isSupported(lang)) {
// eslint-disable-next-line local/locale-format -- a one-shot read in an effect, compared before adopting the account's language; nothing renders it, so there is no memoized value to go stale.
if (lang !== i18n.language) setLanguage(lang);
} else if (account) {
// eslint-disable-next-line local/locale-format -- a one-shot read of the language the app is about to be shown in, stored once; nothing renders it.
const shown = i18n.language;
saveLanguage(qc, shown).catch((e: unknown) =>
log.warn("boot.language_store_failed", "the shown language could not be stored", {
error: String(e),
}),
);
}
// eslint-disable-next-line react-hooks/set-state-in-effect -- the adoption latch. It cannot be derived: the whole point is that the tree mounts only AFTER the pin and the language have been applied, and both are writes to things outside React (sessionStorage, i18n). A layout effect so the frame that first shows the app already has them.
setReadyFor(identity);
}, [identity, readyFor]);
}, [identity, readyFor, qc]);
// No answer at all (a network or 5xx failure on the very first setup or account probe — past the
// one retry `req` already makes): say so, with a retry. An unanswered setup probe is NOT "signed
+1 -2
View File
@@ -27,7 +27,6 @@ import { useChannelTagActions } from "../lib/useChannelTagActions";
import { useTagSuggestions } from "./useTagSuggestions";
import { useChannelPriority } from "../lib/useChannelPriority";
import { placeByPriority } from "../lib/channelOrder";
import { serverOrderLang } from "../lib/saveLanguage";
import { focusNavItem } from "../lib/listNav";
import ViewSwitcher, { type ViewOption } from "./ViewSwitcher";
import ColumnSortControl from "./ui/ColumnSortControl";
@@ -174,7 +173,7 @@ export default function Channels() {
setPriority(c.id, next);
qc.setQueryData<ManagedChannel[]>(
qk.channels(),
(old) => old && placeByPriority(old, c.id, serverOrderLang(me.preferences)),
(old) => old && placeByPriority(old, c.id, me.preferences),
);
setFollow((f) => ({ key: c.id, seq: (f?.seq ?? 0) + 1 }));
};
+2
View File
@@ -483,7 +483,9 @@ export default function PlexBrowse() {
if (sub.view.kind === "show") {
const showId = sub.view.id;
return (
// Keyed by the show: another show is a fresh page (its own summary collapsed, no open dialog).
<PlexShowView
key={showId}
showId={showId}
onBack={sub.back}
onPlay={(epRk, queue, fromStart) =>
@@ -276,6 +276,29 @@ describe("PlexPersonView — a bio on its way (E30 S6)", () => {
}
});
it("a retry after the wait is over waits again: a bio still on its way still lands", async () => {
vi.useFakeTimers({ shouldAdvanceTime: true });
try {
plexPerson.mockReset();
plexPerson
.mockResolvedValueOnce(person({ biography: null, enriching: true }))
.mockRejectedValueOnce(new Error("server restarting"))
.mockResolvedValueOnce(person({ biography: null, enriching: true })) // the retry
.mockResolvedValue(person({ biography: "Late bio." }));
wrap(view);
await screen.findByText("Movie One");
await act(() => vi.advanceTimersByTimeAsync(BIO_POLL));
await act(() => vi.advanceTimersByTimeAsync(25_000)); // the first wait runs out
fireEvent.click(within(screen.getByRole("alert")).getByRole("button"));
await act(() => vi.advanceTimersByTimeAsync(10));
expect(screen.getByRole("status")).toBeTruthy(); // on its way again, not a blank
await act(() => vi.advanceTimersByTimeAsync(BIO_POLL));
await screen.findByText("Late bio.");
} finally {
vi.useRealTimers();
}
});
it("offers More only when the bio is cut, and Less once open", async () => {
const tall = vi.spyOn(HTMLElement.prototype, "scrollHeight", "get").mockReturnValue(400);
const box = vi.spyOn(HTMLElement.prototype, "clientHeight", "get").mockReturnValue(160);
+13 -3
View File
@@ -160,12 +160,14 @@ export function PlexPersonView({
// S6); the answer says so (`enriching`) and the page asks again until it lands — for a while, not
// forever: BIO_WAIT_MS after the page opened (by the clock, not by the last answer — a failing ask
// leaves that where it was) a TMDB that is down stops being waited on, and a failing ask ends it at
// once (the next open tries again). The page is keyed on the person, so the wait starts per person.
// once (the next open tries again). The page is keyed on the person, so the wait starts per person;
// a Retry starts it over (`waitRound`), or a retry answered "still on its way" would show nothing.
const [waitOver, setWaitOver] = useState(false);
const [waitRound, setWaitRound] = useState(0);
useEffect(() => {
const timer = setTimeout(() => setWaitOver(true), BIO_WAIT_MS);
return () => clearTimeout(timer);
}, []);
}, [waitRound]);
const q = useQuery({
queryKey: personKey,
queryFn: () => api.plexPerson(name, lang),
@@ -336,7 +338,15 @@ export function PlexPersonView({
// person with no bio; the retry asks (and waits) again.
d.enriching &&
q.isError && (
<StateMessage tone="error" compact onRetry={() => q.refetch()}>
<StateMessage
tone="error"
compact
onRetry={() => {
setWaitOver(false);
setWaitRound((n) => n + 1);
void q.refetch();
}}
>
{t("plex.person.bioFailed")}
</StateMessage>
)
@@ -45,6 +45,21 @@ describe("ClampedText", () => {
}
});
it("starts another text collapsed: a reused view opening the next item", () => {
const tall = vi.spyOn(HTMLElement.prototype, "scrollHeight", "get").mockReturnValue(400);
const box = vi.spyOn(HTMLElement.prototype, "clientHeight", "get").mockReturnValue(60);
try {
const { rerender } = render(view("The first show's long summary."));
fireEvent.click(screen.getByText("More"));
rerender(view("The next show's long summary."));
expect(screen.getByText(/next show/).className).toContain("line-clamp-3");
expect(screen.getByText("More").getAttribute("aria-expanded")).toBe("false");
} finally {
tall.mockRestore();
box.mockRestore();
}
});
it("measures again when its box changes: the window or the text size moved the cut", () => {
const seen: ResizeObserverCallback[] = []; // the callbacks of observers watching something
const real = globalThis.ResizeObserver;
+8 -1
View File
@@ -6,7 +6,9 @@ const CLAMP = { 3: "line-clamp-3", 5: "line-clamp-5", 8: "line-clamp-8" } as con
/** A long text (a biography, a summary) clamped to a few lines, with a More / Less toggle offered only
* when the text is actually cut — measured, so a short text carries no dead button. Re-measured on a
* resize: the window and the text-size slider both move where the cut falls. */
* resize: the window and the text-size slider both move where the cut falls. Another text starts
* collapsed: a view reused for the next item (a show page opening another show) must not open its
* text already expanded. */
export function ClampedText({
text,
lines,
@@ -22,6 +24,11 @@ export function ClampedText({
}) {
const [expanded, setExpanded] = useState(false);
const [clipped, setClipped] = useState(false);
const [shown, setShown] = useState(text);
if (shown !== text) {
setShown(text);
setExpanded(false);
}
const ref = useRef<HTMLParagraphElement>(null);
useLayoutEffect(() => {
const el = ref.current;