diff --git a/frontend/src/components/ConfigPanel.choice.dom.test.tsx b/frontend/src/components/ConfigPanel.choice.dom.test.tsx index 24a81332..d22dab5f 100644 --- a/frontend/src/components/ConfigPanel.choice.dom.test.tsx +++ b/frontend/src/components/ConfigPanel.choice.dom.test.tsx @@ -4,8 +4,10 @@ // a dropdown, and the pick is what Save sends. A cleared draft (reset to the default) shows the default // as the active option rather than no option at all. // -// i18n is a key-passthrough, except that an option's label is its value (the bar-or-dropdown choice -// weighs the real label length, which a full key would distort); the admin config API is stubbed. +// i18n is a key-passthrough, except for an option's label: a full key would distort the +// bar-or-dropdown choice, which weighs the real label length, so a choice id resolves to its value +// under a one-character marker. The marker is what makes a lost lookup visible — rendering the bare +// value instead of the id would be indistinguishable from `label: c`. The admin config API is stubbed. import { afterEach, describe, expect, it, vi } from "vitest"; import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; @@ -16,7 +18,10 @@ vi.mock("react-i18next", async (importOriginal) => { return { ...actual, useTranslation: () => ({ - t: (key: string) => (key.startsWith("config.choices.") ? key.split(".").pop() : key), + // Only an id the fixture really defines resolves: a dropped lookup renders the bare value and + // a malformed id renders the key (long enough to turn every bar into a dropdown), so the + // assertions below see both. + t: (key: string) => CHOICE_LABELS[key] ?? key, i18n: { language: "en" }, }), }; @@ -37,6 +42,18 @@ const choice = (key: string, choices: string[], value: string, is_set = false): }); const PRESETS = ["ultrafast", "superfast", "veryfast", "faster", "fast", "medium", "slow"]; +const LEVELS = ["TRACE", "DEBUG", "INFO", "WARNING", "ERROR"]; +const CHOICE_LABELS: Record = Object.fromEntries( + ( + [ + ["plex_transcode_hw", ["auto", "on", "off"]], + ["plex_transcode_preset", PRESETS], + ["log_level", LEVELS], + ] as const + ).flatMap(([key, choices]) => + choices.map((c) => [`config.choices.${key}.${c}`, `~${c}`] as const), + ), +); const setConfig = vi.fn((_key: string, _value: unknown) => Promise.resolve({})); const resetConfig = vi.fn((_key: string) => Promise.resolve({})); @@ -52,7 +69,7 @@ vi.mock("../lib/api", async (importOriginal) => { g: [ choice("plex_transcode_hw", ["auto", "on", "off"], "on", true), choice("plex_transcode_preset", PRESETS, "veryfast"), - choice("log_level", ["TRACE", "DEBUG", "INFO", "WARNING", "ERROR"], "INFO"), + choice("log_level", LEVELS, "INFO"), ], }, secrets_manageable: true, @@ -84,10 +101,10 @@ describe("ConfigPanel choice keys", () => { const bar = await screen.findByRole("group", { name: "config.fields.plex_transcode_hw.label" }); const pressed = () => [...bar.querySelectorAll('[aria-pressed="true"]')].map((b) => b.textContent); - expect(pressed()).toEqual(["on"]); + expect(pressed()).toEqual(["~on"]); expect(screen.queryByRole("textbox")).toBeNull(); // no free-text field for a choice key - fireEvent.click(screen.getByRole("button", { name: "off" })); + fireEvent.click(screen.getByRole("button", { name: "~off" })); fireEvent.click(await screen.findByRole("button", { name: "config.save" })); await waitFor(() => expect(setConfig).toHaveBeenCalledWith("plex_transcode_hw", "off")); }); @@ -96,7 +113,7 @@ describe("ConfigPanel choice keys", () => { mount(); await screen.findByRole("group", { name: "config.fields.plex_transcode_hw.label" }); fireEvent.click(screen.getByRole("button", { name: /config\.reset$/ })); - expect(screen.getByRole("button", { name: "auto" }).getAttribute("aria-pressed")).toBe("true"); + expect(screen.getByRole("button", { name: "~auto" }).getAttribute("aria-pressed")).toBe("true"); fireEvent.click(await screen.findByRole("button", { name: "config.save" })); await waitFor(() => expect(resetConfig).toHaveBeenCalledWith("plex_transcode_hw")); }); @@ -104,20 +121,20 @@ describe("ConfigPanel choice keys", () => { it("renders a long list as a dropdown showing only the value", async () => { mount(); const trigger = await screen.findByRole("button", { - name: "config.fields.plex_transcode_preset.label: veryfast", + name: "config.fields.plex_transcode_preset.label: ~veryfast", }); - expect(trigger.textContent).toBe("veryfast"); + expect(trigger.textContent).toBe("~veryfast"); fireEvent.click(trigger); // The popover rows are queried as FilterSelect.dom.test does (`hidden: true`, picked by text). const rows = screen.getAllByRole("menuitemradio", { hidden: true }); - fireEvent.click(rows.find((r) => r.textContent === "slow")!); + fireEvent.click(rows.find((r) => r.textContent === "~slow")!); fireEvent.click(await screen.findByRole("button", { name: "config.save" })); await waitFor(() => expect(setConfig).toHaveBeenCalledWith("plex_transcode_preset", "slow")); }); it("a few options with long labels are a dropdown too: a bar that wide does not fit a phone", async () => { mount(); - await screen.findByRole("button", { name: "config.fields.log_level.label: INFO" }); + await screen.findByRole("button", { name: "config.fields.log_level.label: ~INFO" }); expect(screen.queryByRole("group", { name: "config.fields.log_level.label" })).toBeNull(); }); }); diff --git a/frontend/src/components/ConfigPanel.field.dom.test.tsx b/frontend/src/components/ConfigPanel.field.dom.test.tsx new file mode 100644 index 00000000..38d4ab24 --- /dev/null +++ b/frontend/src/components/ConfigPanel.field.dom.test.tsx @@ -0,0 +1,76 @@ +// @vitest-environment jsdom +// +// E22 round-2 review: locks the stacking that makes an admin config row usable on a phone. The +// control column cannot shrink, so beside a wide control it left the label and its hint a sliver of +// the ~250 px a phone leaves next to the navigation rail. The sibling fix — a long-labelled choice +// becoming a dropdown — is covered in ConfigPanel.choice.dom.test.tsx, but that one does nothing for +// a text, number or secret key, whose `w-52` input is the wide control: this is their only fix. +// +// jsdom does no layout, so what is asserted is the class contract those rules rest on, the same +// trade SearchBar.dom.test.tsx already makes for the narrow-header rules. +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, render, screen } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ConfigItem } from "../lib/api"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }), + }; +}); + +const text = (key: string): ConfigItem => ({ + key, + type: "str", + group: "g", + secret: false, + min: null, + max: null, + is_set: false, + allow_empty: true, + choices: [], + value: "", + default: "", +}); + +vi.mock("../lib/api", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + api: { + ...actual.api, + adminConfig: () => + Promise.resolve({ groups: { g: [text("site_name")] }, secrets_manageable: true }), + }, + }; +}); +vi.mock("../lib/useMe", () => ({ useMe: () => ({ email_configured: true }) })); + +import ConfigPanel from "./ConfigPanel"; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe("ConfigPanel field row (E22 round 2)", () => { + it("stacks the label above the control below `sm`, and lets the control fit the screen", async () => { + render( + + + , + ); + const label = await screen.findByText("config.fields.site_name.label"); + const row = label.parentElement!.parentElement!; // label → the label/hint column → the row + // One column on a phone, the two-column row only from `sm` up: + expect(row.className).toContain("flex-col"); + expect(row.className).toContain("sm:flex-row"); + // The control column may not out-width the screen, and only stops shrinking from `sm` up: + const control = row.lastElementChild as HTMLElement; + expect(control.className).toContain("max-w-full"); + expect(control.className).toContain("sm:shrink-0"); + expect(control.className).not.toMatch(/(^|\s)shrink-0(\s|$)/); + }); +}); diff --git a/frontend/src/components/DiagnosticsSettings.dom.test.tsx b/frontend/src/components/DiagnosticsSettings.dom.test.tsx index f4531b10..f4fb8c7b 100644 --- a/frontend/src/components/DiagnosticsSettings.dom.test.tsx +++ b/frontend/src/components/DiagnosticsSettings.dom.test.tsx @@ -136,6 +136,39 @@ describe("DiagnosticsSettings", () => { expect(document.activeElement).toBe(levelButton("settings.diagnostics.levels.INFO")); }); + it("the watch outlives a render in which the element it is watching is still there", async () => { + // The distinguishing property of the restore, and the one the browser found: it keeps looking + // until the element is GONE, instead of spending itself on the first render after the change. + // Chrome commits a saved level as two renders and jsdom as one, so this drives the general + // case directly — the user moved the focus somewhere else while the request was out, and that + // somewhere else disappears later, several renders on. + vi.useFakeTimers(); + try { + // Raised, so the status line's countdown is mounted and its 30 s tick keeps re-rendering. + const until = new Date(Date.now() + 3_600_000).toISOString(); + mount({ diagnostics: { level: "DEBUG", until, retention_days: 7 } }); + fireEvent.click(levelButton("settings.diagnostics.levels.TRACE")); + const elsewhere = document.createElement("button"); + document.body.append(elsewhere); + elsewhere.focus(); + + // Cancel closes the box; the watch is armed on `elsewhere`, which is still in the page. + fireEvent.click(screen.getByRole("button", { name: "settings.diagnostics.consent.cancel" })); + expect(document.activeElement).toBe(elsewhere); // their place is kept, nothing is stolen + + // Renders keep coming (the countdown ticks) while it is still there, and must not spend it. + await vi.advanceTimersByTimeAsync(60_000); + expect(document.activeElement).toBe(elsewhere); + + elsewhere.remove(); // now the focus really has fallen to + expect(document.activeElement).toBe(document.body); + await vi.advanceTimersByTimeAsync(30_000); + expect(document.activeElement).toBe(levelButton("settings.diagnostics.levels.DEBUG")); + } finally { + vi.useRealTimers(); + } + }); + it("the ticking status line is not a live region", async () => { const until = new Date(Date.now() + 3_600_000).toISOString(); mount({ diagnostics: { level: "DEBUG", until, retention_days: 7 } }); diff --git a/frontend/src/lib/format.test.ts b/frontend/src/lib/format.test.ts index cb723338..e5193c59 100644 --- a/frontend/src/lib/format.test.ts +++ b/frontend/src/lib/format.test.ts @@ -198,11 +198,13 @@ describe("formatRuntime", () => { // runtime's own locale and time zone are the dev box's, so both languages — and a zone west of UTC — // are pinned explicitly. describe("formatDateTime / formatDate", () => { - const savedTz = process.env.TZ; + // Node re-reads TZ when it is ASSIGNED and not when it is deleted, so the zone has to be put back + // by name — and `process.env.TZ` is unset here, so the name has to come from the resolved zone. + // Vitest's per-file process would contain a leak today, but only until someone adds a test below. + const savedTz = process.env.TZ ?? Intl.DateTimeFormat().resolvedOptions().timeZone; afterEach(() => { i18nStub.language = "en"; - if (savedTz === undefined) delete process.env.TZ; - else process.env.TZ = savedTz; + process.env.TZ = savedTz; }); it("formats a timestamp in the UI language, not in the runtime's", () => {