fix(menus): one tick for every choice; a listbox holds its options only

- The saved views, the view switcher, the year filter and the table's single-choice filter mark the chosen row with MenuCheck, as DESIGN asks, not a trailing tick, a dot or a filled row; a contract test holds every checkable menu row to it. keepSelectedPaint had no caller left and is gone. - A combobox's panel carries its listbox role on an inner PopoverList (listInside), so the failed load, Loading, No matches and the 'n more' line sit beside the options, not inside the listbox. The Plex search box's Loading line likewise. - The Plex sidebar's playlist picker offers Create only once the lists have loaded: over a failed load it pushed the error and its Retry out at the first keystroke.
2026-09-27 19:43:04 +02:00
parent cda626be10
commit e31a588f19
12 changed files with 266 additions and 116 deletions
@@ -219,4 +219,14 @@ describe("PlexSearchBox — typeahead", () => {
    await waitFor(() => expect(input.getAttribute("aria-expanded")).toBe("false"));
    expect(input.getAttribute("aria-controls")).toBeNull();
  });
+
+  // Round 2 (F4 sibling): a listbox may own options only — "Loading…" sat inside it.
+  it("shows its loading line beside the listbox, not in it", async () => {
+    plexState.q = "matrix";
+    plexLibrary.mockReturnValue(new Promise(() => {}));
+    renderBox();
+    fireEvent.focus(screen.getByRole("combobox"));
+    const loading = await screen.findByText("plex.loading");
+    expect(screen.getByRole("listbox").contains(loading)).toBe(false);
+  });
 });
@@ -137,47 +137,46 @@ export default function PlexSearchBox() {
      below={
        open ? (
          <div
-            ref={listRef}
-            id={listId}
-            role="listbox"
-            aria-label={t("plex.suggest.aria")}
            className={`${SURFACE.menu} absolute left-0 right-0 top-full z-menu mt-2 max-h-[70dvh] overflow-y-auto animate-[popIn_0.16s_ease]`}
          >
+            {/* Beside the listbox, not in it: a listbox may own options only (round 2, F4). */}
            {suggestions.length === 0 && suggestQ.isFetching && (
              <div className="px-3 py-2 text-sm text-muted">{t("plex.loading")}</div>
            )}
-            {suggestions.map((c, i) => (
-              <Pressable
-                key={c.id}
-                id={`${listId}-opt-${i}`}
-                data-idx={i}
-                role="option"
-                aria-selected={hi === i}
-                onMouseDown={(e) => e.preventDefault()} // keep input focus through the click
-                onMouseEnter={() => setHi(i)}
-                onClick={() => pick(i)}
-                // The pointer moves `hi` too, so the highlight IS the hover: the menus' roving
-                // recipe (AnchoredPopover's ROVING_HIGHLIGHT), one look for the mouse and the keys.
-                data-active={hi === i ? "true" : undefined}
-                className={`flex w-full items-center gap-3 rounded-lg p-1.5 transition ${ROVING_HIGHLIGHT}`}
-              >
-                <img
-                  src={c.thumb}
-                  alt=""
-                  loading="lazy"
-                  className="h-12 w-8 shrink-0 rounded-md object-cover bg-border/40"
-                />
-                <span className="min-w-0 flex-1 truncate text-sm">{c.title}</span>
-                {c.year != null && (
-                  <span className="shrink-0 text-xs text-muted tabular-nums">{c.year}</span>
-                )}
-                {c.type === "show" ? (
-                  <Tv className="w-3.5 h-3.5 shrink-0 text-muted" aria-hidden="true" />
-                ) : (
-                  <Film className="w-3.5 h-3.5 shrink-0 text-muted" aria-hidden="true" />
-                )}
-              </Pressable>
-            ))}
+            <div ref={listRef} id={listId} role="listbox" aria-label={t("plex.suggest.aria")}>
+              {suggestions.map((c, i) => (
+                <Pressable
+                  key={c.id}
+                  id={`${listId}-opt-${i}`}
+                  data-idx={i}
+                  role="option"
+                  aria-selected={hi === i}
+                  onMouseDown={(e) => e.preventDefault()} // keep input focus through the click
+                  onMouseEnter={() => setHi(i)}
+                  onClick={() => pick(i)}
+                  // The pointer moves `hi` too, so the highlight IS the hover: the menus' roving
+                  // recipe (AnchoredPopover's ROVING_HIGHLIGHT), one look for the mouse and the keys.
+                  data-active={hi === i ? "true" : undefined}
+                  className={`flex w-full items-center gap-3 rounded-lg p-1.5 transition ${ROVING_HIGHLIGHT}`}
+                >
+                  <img
+                    src={c.thumb}
+                    alt=""
+                    loading="lazy"
+                    className="h-12 w-8 shrink-0 rounded-md object-cover bg-border/40"
+                  />
+                  <span className="min-w-0 flex-1 truncate text-sm">{c.title}</span>
+                  {c.year != null && (
+                    <span className="shrink-0 text-xs text-muted tabular-nums">{c.year}</span>
+                  )}
+                  {c.type === "show" ? (
+                    <Tv className="w-3.5 h-3.5 shrink-0 text-muted" aria-hidden="true" />
+                  ) : (
+                    <Film className="w-3.5 h-3.5 shrink-0 text-muted" aria-hidden="true" />
+                  )}
+                </Pressable>
+              ))}
+            </div>
          </div>
        ) : null
      }
@@ -0,0 +1,76 @@
+// @vitest-environment jsdom
+//
+// The sidebar's playlist picker (E28 review round 2, P7): typing a name offers "Create …" — but only
+// once the lists have loaded. Over a failed load the create row pushed the error and its Retry out
+// of the list at the first keystroke, and whether the name already existed was unknown anyway.
+import { describe, it, expect, vi, afterEach, beforeAll } from "vitest";
+import { render, cleanup, screen, fireEvent, waitFor } from "@testing-library/react";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import { FloatingTree } from "@floating-ui/react";
+
+vi.mock("react-i18next", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("react-i18next")>();
+  return {
+    ...actual,
+    useTranslation: () => ({
+      t: (key: string, o?: { name?: string }) => (o?.name != null ? `${key}:${o.name}` : key),
+      i18n: { language: "en" },
+    }),
+  };
+});
+vi.mock("./plexContext", () => ({
+  usePlex: () => ({}),
+  usePlexActions: () => ({ setPlaylistOpen: vi.fn() }),
+}));
+
+import { PlaylistPicker } from "./PlexSidebar";
+import { api } from "../lib/api";
+
+beforeAll(() => {
+  class Stub {
+    observe() {}
+    unobserve() {}
+    disconnect() {}
+    takeRecords() {
+      return [];
+    }
+  }
+  globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver;
+});
+
+afterEach(() => {
+  cleanup();
+  vi.restoreAllMocks();
+});
+
+function mount() {
+  const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
+  render(
+    <QueryClientProvider client={qc}>
+      <FloatingTree>
+        <PlaylistPicker />
+      </FloatingTree>
+    </QueryClientProvider>,
+  );
+  return screen.getByRole("combobox");
+}
+
+describe("the sidebar playlist picker", () => {
+  it("keeps the load error and its Retry while a name is typed, with no Create row", async () => {
+    vi.spyOn(api, "plexPlaylists").mockRejectedValue(new TypeError("Failed to fetch"));
+    const input = mount();
+    fireEvent.focus(input);
+    fireEvent.change(input, { target: { value: "Road trip" } });
+    await waitFor(() => expect(screen.getByRole("alert", { hidden: true })).toBeTruthy());
+    expect(screen.getByText("common.retry")).toBeTruthy();
+    expect(screen.queryByText("plex.playlist.createNamed:Road trip")).toBeNull();
+  });
+
+  it("offers Create for a new name once the lists are in", async () => {
+    vi.spyOn(api, "plexPlaylists").mockResolvedValue({ playlists: [] } as never);
+    const input = mount();
+    fireEvent.focus(input);
+    fireEvent.change(input, { target: { value: "Road trip" } });
+    expect(await screen.findByText("plex.playlist.createNamed:Road trip")).toBeTruthy();
+  });
+});
@@ -332,7 +332,7 @@ export default function PlexSidebar({ layout, setLayout, collapsed, onToggleColl
 // you can still add one — the same reuse-the-Combobox idea as the feed's "Your tags" widget. `selected`
 // stays empty: this picks an action (open / create), it doesn't hold a persistent selection.
 const CREATE_PLAYLIST = "__create__";
-function PlaylistPicker() {
+export function PlaylistPicker() {
  const { t } = useTranslation();
  const { setPlaylistOpen: onOpenPlaylist } = usePlexActions();
  const qc = useQueryClient();
@@ -346,8 +346,9 @@ function PlaylistPicker() {
  const exact = playlists.some((p) => p.title.toLowerCase() === ql);
  const options: ComboboxOption[] = [
    ...matches.map((p) => ({ value: `pl:${p.id}`, label: p.title, count: p.item_count })),
-    // Offer to create only when the typed name isn't already a playlist.
-    ...(q && !exact ? [{ value: CREATE_PLAYLIST, label: q }] : []),
+    // Offer to create only when the typed name isn't already a playlist — which is unknown until the
+    // lists load: over a failed load the row pushed out the error and its Retry (round 2, P7).
+    ...(q && !exact && playlistsQ.data ? [{ value: CREATE_PLAYLIST, label: q }] : []),
  ];

  async function createPlaylist(title: string) {
@@ -1,10 +1,11 @@
 import { useState, type ReactNode } from "react";
 import { useTranslation } from "react-i18next";
-import { Bookmark, Check, Settings2 } from "lucide-react";
+import { Settings2 } from "lucide-react";
 import { PopoverItem, PopoverSurface } from "./ui/AnchoredPopover";
 import { closeThenAct, useAnchoredPopover } from "./ui/anchoredPopoverHooks";
 import { useSavedViews } from "./savedViewsContext";
 import { IconButton } from "./ui/IconButton";
+import { MenuCheck } from "./ui/MenuCheck";
 import { SURFACE } from "./ui/surfaces";
 import { SelectTrigger } from "./ui/SelectTrigger";

@@ -80,9 +81,8 @@ export default function SavedViewsPopover({
                  v.id === activeViewId ? "text-fg" : "text-muted"
                }`}
              >
-                <Bookmark className="h-3.5 w-3.5 shrink-0 text-muted" aria-hidden="true" />
+                <MenuCheck kind="one" on={v.id === activeViewId} />
                <span className="flex-1 truncate">{v.name}</span>
-                {v.id === activeViewId && <Check className="h-3.5 w-3.5 shrink-0 text-accent" />}
              </PopoverItem>
            ))}
          </div>
@@ -1,8 +1,9 @@
 import { useState } from "react";
-import { Check, type LucideIcon } from "lucide-react";
+import { type LucideIcon } from "lucide-react";
 import clsx from "clsx";
 import { PopoverItem, PopoverSurface } from "./ui/AnchoredPopover";
 import { useAnchoredPopover } from "./ui/anchoredPopoverHooks";
+import { MenuCheck } from "./ui/MenuCheck";
 import { SelectTrigger } from "./ui/SelectTrigger";
 import { SURFACE } from "./ui/surfaces";

@@ -99,9 +100,9 @@ export default function ViewSwitcher<T extends string>({
                  on ? "text-accent" : "text-fg"
                }`}
              >
+                <MenuCheck kind="one" on={on} />
                <Icon className="w-4 h-4 shrink-0" />
                <span className="flex-1 truncate">{o.label}</span>
-                {on && <Check className="w-3.5 h-3.5 shrink-0" />}
              </PopoverItem>
            );
          })}
@@ -2,6 +2,7 @@ import { useMemo, useState } from "react";
 import { useTranslation } from "react-i18next";
 import { keepPreviousData, useQuery } from "@tanstack/react-query";
 import { ChevronDown } from "lucide-react";
+import { MenuCheck } from "./ui/MenuCheck";
 import { qk } from "../lib/queryKeys";
 import { api, type FeedFilters, type YearCount } from "../lib/api";
 import { PopoverItem, PopoverSurface } from "./ui/AnchoredPopover";
@@ -178,9 +179,7 @@ export default function YearFilter({
                      on ? "text-fg" : "text-muted"
                    }`}
                  >
-                    <span
-                      className={`h-1.5 w-1.5 shrink-0 rounded-full ${on ? "bg-accent" : "bg-transparent"}`}
-                    />
+                    <MenuCheck kind="one" on={on} />
                    <span className="flex-1 tabular-nums">{y.year}</span>
                    <span className="text-2xs text-muted shrink-0 tabular-nums">{y.count}</span>
                  </PopoverItem>
@@ -93,17 +93,12 @@ type ItemRole = "menuitem" | "menuitemradio" | "menuitemcheckbox" | "option";
 // selected state stays legible ON the fill because these menus mark "selected" with a CHILD element (a
 // checkmark / checkbox) that paints over the row tint; the selected row just gets a stronger ring
 // (ring-2, accent/80) than an unselected one (ring-1, accent/60) to keep the two apart.
-// EXCEPTION: a menu whose SELECTED item paints its OWN row background (a solid bg-accent — the
-// table-filter options) passes `keepSelectedPaint`, so the highlight yields to ring-only there; a fill
-// would downgrade that solid paint to a 35% tint (the R9 concern, now scoped to just that idiom).
 // Exported as ROVING_HIGHLIGHT for a listbox that is not a PopoverItem (the command palette, the Plex
 // typeahead): the row carries `data-active` and takes this, so the keyboard row looks the same everywhere.
 export const ROVING_HIGHLIGHT =
  "data-active:bg-accent/35 data-active:text-fg data-active:ring-1 data-active:ring-inset data-active:ring-accent/60";
 const HIGHLIGHT_SELECTED =
  "data-active:bg-accent/35 data-active:text-fg data-active:ring-2 data-active:ring-inset data-active:ring-accent/80";
-const HIGHLIGHT_SELECTED_RING_ONLY =
-  "data-active:ring-2 data-active:ring-inset data-active:ring-accent/70";
 // On the always-dark media chrome (a player's menus, `bg-media-chrome` under white text) the theme's
 // `text-fg` is dark in the light theme: the highlighted row would go dark-on-dark. White instead.
 const MEDIA_HIGHLIGHT =
@@ -115,7 +110,6 @@ export function PopoverItem({
  role = "menuitem",
  checked,
  selected,
-  keepSelectedPaint,
  disabled,
  className,
  elementRef,
@@ -129,9 +123,6 @@ export function PopoverItem({
  role?: ItemRole;
  checked?: boolean;
  selected?: boolean;
-  /** Set when the SELECTED item paints its OWN row background (a solid bg-accent, not a child checkmark):
-   *  the active highlight then yields to ring-only so the fill doesn't downgrade that solid paint. */
-  keepSelectedPaint?: boolean;
  disabled?: boolean;
  className?: string;
  /** Hand the item's DOM node to the caller, ON TOP of the list registration this component owns.
@@ -222,9 +213,7 @@ export function PopoverItem({
        tone === "media"
          ? MEDIA_HIGHLIGHT
          : checked || selected
-            ? keepSelectedPaint
-              ? HIGHLIGHT_SELECTED_RING_ONLY
-              : HIGHLIGHT_SELECTED
+            ? HIGHLIGHT_SELECTED
            : ROVING_HIGHLIGHT
      }${className ? ` ${className}` : ""}`}
    >
@@ -247,6 +236,19 @@ interface PopoverSurfaceProps {
   *  aimed at whatever it happens to cover during its close delay. Interactive hover cards (scrollable
   *  content, links) must NOT set this. */
  passthrough?: boolean;
+  /** The panel's role (a combobox's `listbox`) goes on an inner `<PopoverList>` instead of the panel
+   *  itself, so the panel can also hold lines ABOUT the list — loading, nothing found, a failed load
+   *  with its Retry — outside it: a listbox may own options only (round 2, F4). */
+  listInside?: boolean;
+}
+
+// The role, id and name the panel hands to its `<PopoverList>` under `listInside`.
+const ListRoleContext = createContext<Record<string, unknown> | null>(null);
+
+/** The element carrying a `listInside` panel's list role: wrap the options, and only them, in it. */
+export function PopoverList({ children }: { children: ReactNode }) {
+  const props = useContext(ListRoleContext);
+  return <div {...props}>{children}</div>;
 }

 /** Renders the popover panel: the FloatingTree node + portal + the positioned surface. Merges the
@@ -270,6 +272,7 @@ function PopoverSurfaceInner({
  initialFocus,
  passthrough,
  ariaLabel,
+  listInside,
 }: PopoverSurfaceProps) {
  const {
    context,
@@ -309,6 +312,15 @@ function PopoverSurfaceInner({
  // useRole stamps `aria-labelledby` (the trigger) on a menu-role panel, and it WINS over aria-label
  // in the accessible-name computation — so an explicit ariaLabel is dead unless we drop the ref.
  if (ariaLabel) delete floatingProps["aria-labelledby"];
+  // Under `listInside` the list's identity (the role, the id the field's aria-controls names, its
+  // name and aria-* state) moves in; the panel keeps its handlers and position.
+  const listProps: Record<string, unknown> | null = listInside ? { "aria-label": ariaLabel } : null;
+  if (listProps)
+    for (const key of Object.keys(floatingProps))
+      if (key === "role" || key === "id" || key.startsWith("aria-")) {
+        listProps[key] = floatingProps[key];
+        delete floatingProps[key];
+      }

  // A menu/listbox/dialog traps focus into the surface (and returns it on close). A hover card is
  // passive, and a COMBOBOX must leave focus in the input (its list is navigated virtually) — neither
@@ -359,7 +371,7 @@ function PopoverSurfaceInner({
    <div
      ref={refs.setFloating}
      {...floatingProps}
-      aria-label={ariaLabel}
+      aria-label={listProps ? undefined : ariaLabel}
      className={zClass}
      style={{
        ...floatingPropStyle,
@@ -382,7 +394,11 @@ function PopoverSurfaceInner({
            the field swallows arrow keys, so there is no list-nav — has nothing to reclaim it otherwise,
            so it used to snap back to the first item). Keyboard nav / typeahead take the highlight back
            by nulling hoverIndex in onNavigate/onMatch above. */}
-        {children}
+        {listProps ? (
+          <ListRoleContext.Provider value={listProps}>{children}</ListRoleContext.Provider>
+        ) : (
+          children
+        )}
      </div>
    </div>
  );
@@ -339,6 +339,35 @@ describe("Combobox · honest states", () => {
    expect(document.activeElement).toBe(field);
  });

+  // Round 2 (F4): a listbox may own options only — the alert, "Loading…", "No matches" and the
+  // "n more" line sat inside it (axe: aria-required-children). They sit beside it now.
+  it("keeps the lines about the list outside the listbox, which the field still controls", () => {
+    const { rerender } = wrap(
+      <Combobox label="User" options={[]} selected={[]} onChange={vi.fn()} error="Load failed" />,
+    );
+    const field = screen.getByRole("combobox");
+    field.focus();
+    fireEvent.focus(field);
+    const listbox = () => screen.getByRole("listbox", { hidden: true });
+    expect(field.getAttribute("aria-controls")).toBe(listbox().id);
+    expect(listbox().getAttribute("aria-label")).toBe("User");
+    expect(listbox().contains(screen.getByRole("alert", { hidden: true }))).toBe(false);
+    rerender(
+      <FloatingTree>
+        <Combobox label="User" options={[]} selected={[]} onChange={vi.fn()} />
+      </FloatingTree>,
+    );
+    expect(listbox().textContent).toBe("");
+    rerender(
+      <FloatingTree>
+        <Combobox label="User" options={OPTIONS} selected={[]} onChange={vi.fn()} />
+      </FloatingTree>,
+    );
+    const kids = [...listbox().children].map((c) => c.getAttribute("role"));
+    expect(kids.length).toBeGreaterThan(0);
+    expect(kids.every((r) => r === "option")).toBe(true);
+  });
+
  it("disabled: keeps the focus, opens nothing and picks nothing", () => {
    const onChange = vi.fn();
    wrap(<Combobox label="Genre" options={OPTIONS} selected={[]} onChange={onChange} disabled />);
@@ -4,7 +4,7 @@ import { useTranslation } from "react-i18next";
 import { ChevronDown } from "lucide-react";
 import { Button } from "./Button";
 import { MenuCheck } from "./MenuCheck";
-import { PopoverSurface, PopoverItem } from "./AnchoredPopover";
+import { PopoverItem, PopoverList, PopoverSurface } from "./AnchoredPopover";
 import { useAnchoredPopover } from "./anchoredPopoverHooks";
 import { useScrollFade } from "../../lib/useScrollFade";
 import { InnerInput } from "./Input";
@@ -263,42 +263,15 @@ export default function Combobox({
      <PopoverSurface
        popover={pop}
        ariaLabel={label}
+        listInside
        className={`${SURFACE.menu} w-full animate-[popIn_0.16s_ease]`}
      >
        {/* The scroll box owns the height cap AND the scroll-fade mask (ref+style on the same element). */}
        <div ref={fade.ref} style={fade.style} className="max-h-64 overflow-y-auto no-scrollbar">
-          {showError ? (
-            <div className="flex items-center gap-2 px-2 py-1.5">
-              <p role="alert" className="flex-1 text-xs text-danger">
-                {error}
-                {onRetry && <span className="sr-only"> {t("common.retryWithEnter")}</span>}
-              </p>
-              {onRetry && (
-                // For the pointer only: this panel is a listbox, which may hold options and nothing
-                // else, so the keyboard's Retry is Enter in the field (said in the alert above).
-                <Button
-                  variant="outline"
-                  size="xs"
-                  type="button"
-                  tabIndex={-1}
-                  aria-hidden
-                  // Keep the focus in the field: a press that moved it out would close the list.
-                  onMouseDown={(e) => e.preventDefault()}
-                  onClick={onRetry}
-                >
-                  {t("common.retry")}
-                </Button>
-              )}
-            </div>
-          ) : loading && filtered.length === 0 ? (
-            <p className="px-2 py-1.5 text-xs text-muted">{t("common.loading")}</p>
-          ) : filtered.length === 0 ? (
-            <p className="px-2 py-1.5 text-xs text-muted">
-              {emptyText ?? t("common.noResults", { defaultValue: "No matches" })}
-            </p>
-          ) : (
-            <>
-              {visible.map((o) => {
+          {/* The listbox holds the options alone; the lines about the list sit beside it (F4). */}
+          <PopoverList>
+            {!showError &&
+              visible.map((o) => {
                const on = selected.includes(o.value);
                return (
                  <PopoverItem
@@ -328,15 +301,45 @@ export default function Combobox({
                  </PopoverItem>
                );
              })}
-              {hiddenCount > 0 && (
-                <p className="px-2 py-1.5 text-2xs text-muted">
-                  {t("common.moreRefine", {
-                    count: hiddenCount,
-                    defaultValue: "{{count}} more — refine with search",
-                  })}
-                </p>
+          </PopoverList>
+          {showError ? (
+            <div className="flex items-center gap-2 px-2 py-1.5">
+              <p role="alert" className="flex-1 text-xs text-danger">
+                {error}
+                {onRetry && <span className="sr-only"> {t("common.retryWithEnter")}</span>}
+              </p>
+              {onRetry && (
+                // For the pointer: the keyboard's Retry is Enter in the field (said in the alert
+                // above), since the focus stays there the whole time the list is open.
+                <Button
+                  variant="outline"
+                  size="xs"
+                  type="button"
+                  tabIndex={-1}
+                  aria-hidden
+                  // Keep the focus in the field: a press that moved it out would close the list.
+                  onMouseDown={(e) => e.preventDefault()}
+                  onClick={onRetry}
+                >
+                  {t("common.retry")}
+                </Button>
              )}
-            </>
+            </div>
+          ) : loading && filtered.length === 0 ? (
+            <p className="px-2 py-1.5 text-xs text-muted">{t("common.loading")}</p>
+          ) : filtered.length === 0 ? (
+            <p className="px-2 py-1.5 text-xs text-muted">
+              {emptyText ?? t("common.noResults", { defaultValue: "No matches" })}
+            </p>
+          ) : (
+            hiddenCount > 0 && (
+              <p className="px-2 py-1.5 text-2xs text-muted">
+                {t("common.moreRefine", {
+                  count: hiddenCount,
+                  defaultValue: "{{count}} more — refine with search",
+                })}
+              </p>
+            )
          )}
        </div>
      </PopoverSurface>
@@ -1,19 +1,35 @@
 import { readdirSync, readFileSync } from "node:fs";
 import { describe, expect, it } from "vitest";

+const root = new URL("../", import.meta.url);
+const sources = () =>
+  readdirSync(root, { recursive: true, encoding: "utf8" })
+    .filter((f) => f.endsWith(".tsx") && !f.includes(".test."))
+    .map((f) => ({ f, src: readFileSync(new URL(f.replace(/\\/g, "/"), root), "utf8") }));
+
 // E28 review L2: `ui/MenuCheck` is the ONE drawn check box of a checkable row (S3e3b replaced seven
 // hand-drawn copies), yet FilterSelect and Combobox still painted their own — a 14px box beside the
 // menus' 16px one. The filled-box paint is the tell: it may live in MenuCheck only.
 describe("the drawn check box", () => {
  it("is painted by MenuCheck alone", () => {
-    const root = new URL("../", import.meta.url);
-    const offenders = readdirSync(root, { recursive: true, encoding: "utf8" })
-      .filter((f) => f.endsWith(".tsx") && !f.includes(".test.") && !f.endsWith("MenuCheck.tsx"))
-      .filter((f) =>
-        /border-accent bg-accent text-accent-fg/.test(
-          readFileSync(new URL(f.replace(/\\/g, "/"), root), "utf8"),
-        ),
-      );
+    const offenders = sources()
+      .filter(({ f }) => !f.endsWith("MenuCheck.tsx"))
+      .filter(({ src }) => /border-accent bg-accent text-accent-fg/.test(src))
+      .map(({ f }) => f);
+    expect(offenders).toEqual([]);
+  });
+
+  // Round 2 (P6): the player's track rows marked the choice only by an accent label — which the
+  // highlight's white text wiped — and four more menus ticked it their own way (a trailing tick, a
+  // dot, a filled row). A file with a checkable menu row draws MenuCheck, and no tick of its own.
+  it("marks every checkable menu row, with no tick of its own beside it", () => {
+    const offenders = sources()
+      .filter(({ src }) => {
+        const checkable = (src.match(/role="menuitem(?:radio|checkbox)"/g) ?? []).length;
+        const drawn = (src.match(/<MenuCheck\b/g) ?? []).length;
+        return checkable > drawn || (checkable > 0 && /<Check\b/.test(src));
+      })
+      .map(({ f }) => f);
    expect(offenders).toEqual([]);
  });
 });
@@ -112,13 +112,13 @@ function FilterFunnel<T>({
            <PopoverItem
              role="menuitemradio"
              checked={!filterOn}
-              keepSelectedPaint
              onSelect={() => onApply(col.key, "")}
              label={t("datatable.all")}
-              className={`text-left text-xs px-2 py-1.5 rounded-lg outline-hidden transition ${
-                !filterOn ? "bg-accent text-accent-fg" : "text-muted state-layer"
+              className={`flex w-full items-center gap-2 text-left text-xs px-2 py-1.5 rounded-lg outline-hidden transition ${
+                !filterOn ? "text-fg" : "text-muted"
              }`}
            >
+              <MenuCheck kind="one" on={!filterOn} />
              {t("datatable.all")}
            </PopoverItem>
            {f.options.map((o) => (
@@ -126,13 +126,13 @@ function FilterFunnel<T>({
                key={o.value}
                role="menuitemradio"
                checked={value === o.value}
-                keepSelectedPaint
                onSelect={() => onApply(col.key, o.value)}
                label={o.label}
-                className={`text-left text-xs px-2 py-1.5 rounded-lg outline-hidden transition ${
-                  value === o.value ? "bg-accent text-accent-fg" : "text-muted state-layer"
+                className={`flex w-full items-center gap-2 text-left text-xs px-2 py-1.5 rounded-lg outline-hidden transition ${
+                  value === o.value ? "text-fg" : "text-muted"
                }`}
              >
+                <MenuCheck kind="one" on={value === o.value} />
                {o.label}
              </PopoverItem>
            ))}