From c51df4e0b957d1b56e47335e0533e3ba2e407919 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 16 Aug 2026 08:21:29 +0200 Subject: [PATCH] fix(plex): address S2 /code-review findings (typeahead focus + a11y + test) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Decouple the typeahead's dismiss from real focus: a separate `dismissed` flag (set on Escape/pick, cleared on typing/refocus) replaces manually clearing `focused`, so closing the panel no longer wedges it shut while the input keeps DOM focus — typing again re-opens it (verified in-browser). - Scroll the highlighted option into view during arrow-key nav (listRef + data-idx + scrollIntoView), instead of leaving listRef unused. - Wire the input as a combobox for screen readers: role=combobox, aria-autocomplete/expanded/controls/activedescendant + per-option ids. - Drop the redundant client-side .slice (the request already caps at limit=10). - SearchBar gains an `inputProps` passthrough for the combobox ARIA. - Add PlexSearchBox.dom.test.tsx (4 tests): per-scope pill options, suggestion render + keyboard pick→jumpTo, and the Escape→retype reopen regression. Reviewer's scrollbar-blur concern was a false positive — the listbox container is not focusable, so a mousedown on it never blurs the input. --- .../src/components/PlexSearchBox.dom.test.tsx | 169 ++++++++++++++++++ frontend/src/components/PlexSearchBox.tsx | 50 ++++-- frontend/src/components/SearchBar.tsx | 12 +- 3 files changed, 220 insertions(+), 11 deletions(-) create mode 100644 frontend/src/components/PlexSearchBox.dom.test.tsx diff --git a/frontend/src/components/PlexSearchBox.dom.test.tsx b/frontend/src/components/PlexSearchBox.dom.test.tsx new file mode 100644 index 00000000..82c0df6f --- /dev/null +++ b/frontend/src/components/PlexSearchBox.dom.test.tsx @@ -0,0 +1,169 @@ +// @vitest-environment jsdom +// +// Contract of the Plex header search box (S2): the field scope-pill and the instant typeahead. +// Covers the pieces the /code-review flagged: the pill's per-scope options (Episode only in +// show/both), the suggestion list rendering, keyboard pick → jumpTo hand-off, and the regression +// that closing the panel (Escape) must NOT wedge it shut while the input keeps focus — typing again +// re-opens it. i18n is key-passthrough; the provider, api, and debounce are mocked so the test drives +// state directly. ActionMenu is floating-ui based, so we stub the observers and wrap in a FloatingTree. +import { describe, it, expect, afterEach, beforeAll, beforeEach, vi } from "vitest"; +import { render, screen, fireEvent, cleanup, waitFor } from "@testing-library/react"; +import { FloatingTree } from "@floating-ui/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useTranslation: () => ({ t: (key: string) => key }) }; +}); +// Identity debounce so a typed value is reflected immediately. +vi.mock("../lib/useDebounced", () => ({ useDebounced: (v: unknown) => v })); + +// Controllable provider — PlexSearchBox reads q/scope/field and calls setQ/setField/setJumpTo. +const plexState = { scope: "both", field: "all", q: "" } as { scope: string; field: string; q: string }; +const setQ = vi.fn(); +const setField = vi.fn(); +const setJumpTo = vi.fn(); +vi.mock("./PlexProvider", () => ({ + usePlex: () => plexState, + usePlexActions: () => ({ setQ, setField, setJumpTo }), +})); + +// api.plexLibrary stub — returns whatever the current test queued. +const plexLibrary = vi.fn(); +vi.mock("../lib/api", () => ({ api: { plexLibrary: (...a: unknown[]) => plexLibrary(...a) } })); + +import PlexSearchBox from "./PlexSearchBox"; + +const card = (id: string, title: string, type: "movie" | "show" = "movie", year = 2000) => ({ + id, + type, + title, + year, + thumb: `/api/plex/image/${id}`, +}); +const result = (items: ReturnType[]) => ({ + scope: "both", + total: items.length, + offset: 0, + limit: 10, + items, + episodes: [], +}); + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; + // jsdom has no layout, so scrollIntoView (used to keep the active option in view) is absent. + Element.prototype.scrollIntoView ??= () => {}; +}); + +beforeEach(() => { + plexState.scope = "both"; + plexState.field = "all"; + plexState.q = ""; + setQ.mockReset(); + setField.mockReset(); + setJumpTo.mockReset(); + plexLibrary.mockReset(); + plexLibrary.mockResolvedValue(result([])); +}); +afterEach(cleanup); + +const renderBox = () => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + + + , + ); +}; + +const openPill = () => + fireEvent.click(screen.getByRole("button", { name: "plex.searchField.aria" })); + +describe("PlexSearchBox — scope-pill", () => { + it("offers all five field scopes when shows are in scope", () => { + plexState.scope = "both"; + renderBox(); + openPill(); + const labels = screen.getAllByRole("menuitem", { hidden: true }).map((b) => b.textContent); + expect(labels).toEqual([ + "plex.searchField.all", + "plex.searchField.title", + "plex.searchField.people", + "plex.searchField.summary", + "plex.searchField.episode", + ]); + }); + + it("hides the Episode scope in a movie-only scope", () => { + plexState.scope = "movie"; + renderBox(); + openPill(); + const labels = screen.getAllByRole("menuitem", { hidden: true }).map((b) => b.textContent); + expect(labels).toEqual([ + "plex.searchField.all", + "plex.searchField.title", + "plex.searchField.people", + "plex.searchField.summary", + ]); + expect(labels).not.toContain("plex.searchField.episode"); + }); +}); + +describe("PlexSearchBox — typeahead", () => { + it("renders the top matches on focus and picks one via keyboard → jumpTo", async () => { + plexState.q = "matrix"; + plexLibrary.mockResolvedValue( + result([card("m1", "The Matrix"), card("m2", "The Matrix Reloaded")]), + ); + renderBox(); + fireEvent.focus(screen.getByRole("combobox")); + + // Both matches show up as options. + await waitFor(() => + expect(screen.getAllByRole("option").map((o) => o.textContent)).toEqual([ + expect.stringContaining("The Matrix"), + expect.stringContaining("The Matrix Reloaded"), + ]), + ); + // The request carried the active field scope + the limit. + expect(plexLibrary).toHaveBeenCalledWith( + expect.objectContaining({ scope: "both", q: "matrix", field: "all", limit: 10 }), + ); + + // ArrowDown highlights the first, Enter opens it via the jumpTo hand-off (not a Go search). + const input = screen.getByRole("combobox"); + fireEvent.keyDown(input, { key: "ArrowDown" }); + fireEvent.keyDown(input, { key: "Enter" }); + expect(setJumpTo).toHaveBeenCalledWith({ id: "m1", type: "movie" }); + }); + + it("reopens after Escape when the user types again (focus/dismiss are decoupled)", async () => { + plexState.q = "matrix"; + plexLibrary.mockResolvedValue(result([card("m1", "The Matrix")])); + renderBox(); + const input = screen.getByRole("combobox"); + fireEvent.focus(input); + await screen.findByRole("listbox"); + + // Escape dismisses the panel (input stays focused — Escape does not blur). + fireEvent.keyDown(input, { key: "Escape" }); + await waitFor(() => expect(screen.queryByRole("listbox")).toBeNull()); + + // Typing again must re-open it without a blur+refocus round-trip. + fireEvent.change(input, { target: { value: "matrixx" } }); + await screen.findByRole("listbox"); + expect(screen.getByRole("listbox")).toBeTruthy(); + }); +}); diff --git a/frontend/src/components/PlexSearchBox.tsx b/frontend/src/components/PlexSearchBox.tsx index 3134bc42..9529993f 100644 --- a/frontend/src/components/PlexSearchBox.tsx +++ b/frontend/src/components/PlexSearchBox.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from "react"; +import { useEffect, useId, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { useQuery } from "@tanstack/react-query"; import { ChevronDown, Film, Tv } from "lucide-react"; @@ -29,13 +29,20 @@ export default function PlexSearchBox() { const { setQ, setField, setJumpTo } = usePlexActions(); const activeField = healPlexField(field, scope); const dq = useDebounced(q.trim(), 200); + // `focused` mirrors the input's real DOM focus (only onFocus/onBlur touch it). `dismissed` is the + // separate "the user closed the panel" flag (Escape / after a pick) — kept apart so closing the + // panel never desyncs from actual focus: typing again clears `dismissed` and the panel re-opens + // without needing a blur+refocus cycle. const [focused, setFocused] = useState(false); + const [dismissed, setDismissed] = useState(false); const [hi, setHi] = useState(-1); // highlighted suggestion index (-1 = none) const listRef = useRef(null); + const listId = useId(); // The Episode scope surfaces episodes in the grid's grouped section, not as title cards, so the // titles-only typeahead has nothing to show there — suppress it rather than show "no results". - const suggestEnabled = focused && dq.length >= MIN_CHARS && activeField !== "episode"; + const suggestEnabled = + focused && !dismissed && dq.length >= MIN_CHARS && activeField !== "episode"; const suggestQ = useQuery({ queryKey: qk.plexSuggest(scope, dq, activeField), enabled: suggestEnabled, @@ -43,31 +50,44 @@ export default function PlexSearchBox() { api.plexLibrary({ scope, q: dq, field: activeField, limit: SUGGEST_LIMIT }), staleTime: 30_000, }); - const suggestions = suggestEnabled ? (suggestQ.data?.items ?? []).slice(0, SUGGEST_LIMIT) : []; + const suggestions = suggestEnabled ? (suggestQ.data?.items ?? []) : []; const open = suggestEnabled && (suggestQ.isFetching || suggestions.length > 0); - // A fresh query resets the highlight; keep it in range if the list shrank. + // A fresh query resets the highlight. useEffect(() => setHi(-1), [dq, activeField, scope]); + // Keep the highlighted option scrolled into view during arrow-key navigation. + useEffect(() => { + if (hi < 0) return; + listRef.current + ?.querySelector(`[data-idx="${hi}"]`) + ?.scrollIntoView({ block: "nearest" }); + }, [hi]); const pick = (i: number) => { const c = suggestions[i]; if (!c) return; setJumpTo({ id: c.id, type: c.type === "show" ? "show" : "movie" }); - setFocused(false); + setDismissed(true); // close the panel; typing again re-opens it (see `dismissed`) setHi(-1); }; const fields = availablePlexFields(scope); const pillLabel = t(plexFieldLabelKey(activeField)); + const activeOptionId = hi >= 0 ? `${listId}-opt-${hi}` : undefined; return ( setFocused(true)} - // Suggestion rows preventDefault on mousedown so the input keeps focus through the click; any - // other blur (Tab, the pill, clicking away) closes the panel. + // Typing after a dismiss (Escape / pick) re-opens the panel. + onChange={(v) => { + setDismissed(false); + setQ(v); + }} + onFocus={() => { + setFocused(true); + setDismissed(false); + }} onBlur={() => setFocused(false)} onKeyDown={(e) => { if (!open) return; @@ -81,9 +101,16 @@ export default function PlexSearchBox() { e.preventDefault(); // suppress SearchBar's Go; open the highlighted title instead pick(hi); } else if (e.key === "Escape") { - setFocused(false); + setDismissed(true); } }} + inputProps={{ + role: "combobox", + "aria-autocomplete": "list", + "aria-expanded": open, + "aria-controls": open ? listId : undefined, + "aria-activedescendant": activeOptionId, + }} leading={ (