fix(plex): address S2 /code-review findings (typeahead focus + a11y + test)
- 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.
This commit is contained in:
@@ -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<typeof import("react-i18next")>();
|
||||||
|
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<typeof card>[]) => ({
|
||||||
|
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(
|
||||||
|
<QueryClientProvider client={qc}>
|
||||||
|
<FloatingTree>
|
||||||
|
<PlexSearchBox />
|
||||||
|
</FloatingTree>
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
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();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,4 +1,4 @@
|
|||||||
import { useEffect, useRef, useState } from "react";
|
import { useEffect, useId, useRef, useState } from "react";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import { useQuery } from "@tanstack/react-query";
|
import { useQuery } from "@tanstack/react-query";
|
||||||
import { ChevronDown, Film, Tv } from "lucide-react";
|
import { ChevronDown, Film, Tv } from "lucide-react";
|
||||||
@@ -29,13 +29,20 @@ export default function PlexSearchBox() {
|
|||||||
const { setQ, setField, setJumpTo } = usePlexActions();
|
const { setQ, setField, setJumpTo } = usePlexActions();
|
||||||
const activeField = healPlexField(field, scope);
|
const activeField = healPlexField(field, scope);
|
||||||
const dq = useDebounced(q.trim(), 200);
|
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 [focused, setFocused] = useState(false);
|
||||||
|
const [dismissed, setDismissed] = useState(false);
|
||||||
const [hi, setHi] = useState(-1); // highlighted suggestion index (-1 = none)
|
const [hi, setHi] = useState(-1); // highlighted suggestion index (-1 = none)
|
||||||
const listRef = useRef<HTMLDivElement>(null);
|
const listRef = useRef<HTMLDivElement>(null);
|
||||||
|
const listId = useId();
|
||||||
|
|
||||||
// The Episode scope surfaces episodes in the grid's grouped section, not as title cards, so the
|
// 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".
|
// 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({
|
const suggestQ = useQuery({
|
||||||
queryKey: qk.plexSuggest(scope, dq, activeField),
|
queryKey: qk.plexSuggest(scope, dq, activeField),
|
||||||
enabled: suggestEnabled,
|
enabled: suggestEnabled,
|
||||||
@@ -43,31 +50,44 @@ export default function PlexSearchBox() {
|
|||||||
api.plexLibrary({ scope, q: dq, field: activeField, limit: SUGGEST_LIMIT }),
|
api.plexLibrary({ scope, q: dq, field: activeField, limit: SUGGEST_LIMIT }),
|
||||||
staleTime: 30_000,
|
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);
|
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]);
|
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 pick = (i: number) => {
|
||||||
const c = suggestions[i];
|
const c = suggestions[i];
|
||||||
if (!c) return;
|
if (!c) return;
|
||||||
setJumpTo({ id: c.id, type: c.type === "show" ? "show" : "movie" });
|
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);
|
setHi(-1);
|
||||||
};
|
};
|
||||||
|
|
||||||
const fields = availablePlexFields(scope);
|
const fields = availablePlexFields(scope);
|
||||||
const pillLabel = t(plexFieldLabelKey(activeField));
|
const pillLabel = t(plexFieldLabelKey(activeField));
|
||||||
|
const activeOptionId = hi >= 0 ? `${listId}-opt-${hi}` : undefined;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<SearchBar
|
<SearchBar
|
||||||
value={q}
|
value={q}
|
||||||
placeholder={t("plex.searchPlaceholder")}
|
placeholder={t("plex.searchPlaceholder")}
|
||||||
onChange={setQ}
|
// Typing after a dismiss (Escape / pick) re-opens the panel.
|
||||||
onFocus={() => setFocused(true)}
|
onChange={(v) => {
|
||||||
// Suggestion rows preventDefault on mousedown so the input keeps focus through the click; any
|
setDismissed(false);
|
||||||
// other blur (Tab, the pill, clicking away) closes the panel.
|
setQ(v);
|
||||||
|
}}
|
||||||
|
onFocus={() => {
|
||||||
|
setFocused(true);
|
||||||
|
setDismissed(false);
|
||||||
|
}}
|
||||||
onBlur={() => setFocused(false)}
|
onBlur={() => setFocused(false)}
|
||||||
onKeyDown={(e) => {
|
onKeyDown={(e) => {
|
||||||
if (!open) return;
|
if (!open) return;
|
||||||
@@ -81,9 +101,16 @@ export default function PlexSearchBox() {
|
|||||||
e.preventDefault(); // suppress SearchBar's Go; open the highlighted title instead
|
e.preventDefault(); // suppress SearchBar's Go; open the highlighted title instead
|
||||||
pick(hi);
|
pick(hi);
|
||||||
} else if (e.key === "Escape") {
|
} 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={
|
leading={
|
||||||
<ActionMenu
|
<ActionMenu
|
||||||
ariaLabel={t("plex.searchField.aria")}
|
ariaLabel={t("plex.searchField.aria")}
|
||||||
@@ -105,6 +132,7 @@ export default function PlexSearchBox() {
|
|||||||
open ? (
|
open ? (
|
||||||
<div
|
<div
|
||||||
ref={listRef}
|
ref={listRef}
|
||||||
|
id={listId}
|
||||||
role="listbox"
|
role="listbox"
|
||||||
aria-label={t("plex.suggest.aria")}
|
aria-label={t("plex.suggest.aria")}
|
||||||
className="glass-menu absolute left-0 right-0 top-full z-20 mt-2 max-h-[70vh] overflow-y-auto rounded-2xl p-1.5 animate-[popIn_0.16s_ease]"
|
className="glass-menu absolute left-0 right-0 top-full z-20 mt-2 max-h-[70vh] overflow-y-auto rounded-2xl p-1.5 animate-[popIn_0.16s_ease]"
|
||||||
@@ -115,6 +143,8 @@ export default function PlexSearchBox() {
|
|||||||
{suggestions.map((c, i) => (
|
{suggestions.map((c, i) => (
|
||||||
<button
|
<button
|
||||||
key={c.id}
|
key={c.id}
|
||||||
|
id={`${listId}-opt-${i}`}
|
||||||
|
data-idx={i}
|
||||||
type="button"
|
type="button"
|
||||||
role="option"
|
role="option"
|
||||||
aria-selected={hi === i}
|
aria-selected={hi === i}
|
||||||
|
|||||||
@@ -1,4 +1,9 @@
|
|||||||
import { type FocusEventHandler, type KeyboardEventHandler, type ReactNode } from "react";
|
import {
|
||||||
|
type FocusEventHandler,
|
||||||
|
type InputHTMLAttributes,
|
||||||
|
type KeyboardEventHandler,
|
||||||
|
type ReactNode,
|
||||||
|
} from "react";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import { Search, X } from "lucide-react";
|
import { Search, X } from "lucide-react";
|
||||||
|
|
||||||
@@ -26,6 +31,7 @@ export default function SearchBar({
|
|||||||
onFocus,
|
onFocus,
|
||||||
onBlur,
|
onBlur,
|
||||||
onKeyDown,
|
onKeyDown,
|
||||||
|
inputProps,
|
||||||
}: {
|
}: {
|
||||||
value: string;
|
value: string;
|
||||||
onChange: (q: string) => void;
|
onChange: (q: string) => void;
|
||||||
@@ -41,6 +47,9 @@ export default function SearchBar({
|
|||||||
onFocus?: FocusEventHandler<HTMLInputElement>;
|
onFocus?: FocusEventHandler<HTMLInputElement>;
|
||||||
onBlur?: FocusEventHandler<HTMLInputElement>;
|
onBlur?: FocusEventHandler<HTMLInputElement>;
|
||||||
onKeyDown?: KeyboardEventHandler<HTMLInputElement>;
|
onKeyDown?: KeyboardEventHandler<HTMLInputElement>;
|
||||||
|
// Extra attributes spread onto the <input> (e.g. Plex's combobox ARIA). Spread FIRST so the
|
||||||
|
// component's own value/handlers always win.
|
||||||
|
inputProps?: InputHTMLAttributes<HTMLInputElement>;
|
||||||
}) {
|
}) {
|
||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
const fire = () => {
|
const fire = () => {
|
||||||
@@ -51,6 +60,7 @@ export default function SearchBar({
|
|||||||
<div className="h-11 pl-3 pr-2 rounded-2xl glass-menu flex items-center gap-2 focus-within:outline-2 focus-within:outline-accent/60 focus-within:outline-offset-2">
|
<div className="h-11 pl-3 pr-2 rounded-2xl glass-menu flex items-center gap-2 focus-within:outline-2 focus-within:outline-accent/60 focus-within:outline-offset-2">
|
||||||
{leading ?? <Search className="w-4 h-4 shrink-0 text-muted" />}
|
{leading ?? <Search className="w-4 h-4 shrink-0 text-muted" />}
|
||||||
<input
|
<input
|
||||||
|
{...inputProps}
|
||||||
value={value}
|
value={value}
|
||||||
onChange={(e) => onChange(e.target.value)}
|
onChange={(e) => onChange(e.target.value)}
|
||||||
onFocus={onFocus}
|
onFocus={onFocus}
|
||||||
|
|||||||
Reference in New Issue
Block a user