diff --git a/frontend/src/components/FilterSelect.dom.test.tsx b/frontend/src/components/FilterSelect.dom.test.tsx index 4b6435c5..6ffe8210 100644 --- a/frontend/src/components/FilterSelect.dom.test.tsx +++ b/frontend/src/components/FilterSelect.dom.test.tsx @@ -2,8 +2,9 @@ // // Contract of the labeled-pill FilterSelect. Labels are plain props (no i18n). Built on the floating-ui // AnchoredPopover platform, so we stub the two observers autoUpdate wires and wrap in a FloatingTree. -// floating-ui never positions in jsdom → the surface stays visibility:hidden, so option/listbox role -// queries pass `{ hidden: true }`. +// floating-ui never positions in jsdom → the surface stays visibility:hidden, so the menuitem role +// queries pass `{ hidden: true }`. Single-select rows are `menuitemradio`, multi-select `menuitemcheckbox` +// (a menu surface with checkable items, not a listbox of orphaned options). import { describe, it, expect, vi, afterEach, beforeAll } from "vitest"; import { render, screen, fireEvent, cleanup } from "@testing-library/react"; import { FloatingTree } from "@floating-ui/react"; @@ -40,20 +41,19 @@ describe("FilterSelect", () => { const trigger = screen.getByRole("button", { name: "Status" }); expect(trigger.textContent).toContain("Status:"); expect(trigger.textContent).toContain("All"); - expect(screen.queryByRole("option", { hidden: true })).toBeNull(); + expect(screen.queryByRole("menuitemradio", { hidden: true })).toBeNull(); }); it("single: picking a row replaces the selection and closes", () => { const onChange = vi.fn(); tree(); fireEvent.click(screen.getByRole("button", { name: "Status" })); - expect(screen.getAllByRole("option", { hidden: true }).map((o) => o.textContent)).toEqual([ - "All", - "New", - ]); - fireEvent.click(screen.getAllByRole("option", { hidden: true })[1]!); // New + const rows = screen.getAllByRole("menuitemradio", { hidden: true }); + expect(rows.map((o) => o.textContent)).toEqual(["All", "New"]); + expect(rows[0]!.getAttribute("aria-checked")).toBe("true"); // "All" is the current selection + fireEvent.click(rows[1]!); // New expect(onChange).toHaveBeenCalledWith(["new"]); - expect(screen.queryByRole("option", { hidden: true })).toBeNull(); + expect(screen.queryByRole("menuitemradio", { hidden: true })).toBeNull(); }); it("multiple: toggling a row adds it and keeps the panel open", () => { @@ -62,9 +62,9 @@ describe("FilterSelect", () => { , ); fireEvent.click(screen.getByRole("button", { name: "Type" })); - fireEvent.click(screen.getAllByRole("option", { hidden: true })[1]!); // Shorts + fireEvent.click(screen.getAllByRole("menuitemcheckbox", { hidden: true })[1]!); // Shorts expect(onChange).toHaveBeenCalledWith(["n", "s"]); - expect(screen.queryAllByRole("option", { hidden: true }).length).toBeGreaterThan(0); // still open + expect(screen.queryAllByRole("menuitemcheckbox", { hidden: true }).length).toBeGreaterThan(0); // still open }); it("multiple + requireOne: the last remaining selection can't be turned off", () => { @@ -73,7 +73,7 @@ describe("FilterSelect", () => { , ); fireEvent.click(screen.getByRole("button", { name: "Type" })); - fireEvent.click(screen.getAllByRole("option", { hidden: true })[0]!); // Normal — the only one selected + fireEvent.click(screen.getAllByRole("menuitemcheckbox", { hidden: true })[0]!); // Normal — the only one selected expect(onChange).not.toHaveBeenCalled(); }); diff --git a/frontend/src/components/FilterSelect.tsx b/frontend/src/components/FilterSelect.tsx index 05b5b619..8759ad06 100644 --- a/frontend/src/components/FilterSelect.tsx +++ b/frontend/src/components/FilterSelect.tsx @@ -3,7 +3,7 @@ import { ChevronDown, Check } from "lucide-react"; import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; // A compact "labeled pill → dropdown" filter control: the trigger reads `Label: value ▾`, and clicking -// it opens a listbox of options (facet counts optional). It replaces a wall of individual toolbar chips +// it opens a menu of options (facet counts optional). It replaces a wall of individual toolbar chips // with one pill per filter group, in two flavours: // - single (default): picking a row replaces the choice and closes — always exactly one active. // - multiple: rows toggle independently and the panel stays open; `requireOne` blocks clearing the last. @@ -45,7 +45,7 @@ export default function FilterSelect({ const pop = useAnchoredPopover({ open, onOpenChange: setOpen, - mode: "listbox", + mode: "menu", placement: "bottom-start", }); @@ -96,39 +96,44 @@ export default function FilterSelect({ ariaLabel={label} className="glass-menu min-w-52 rounded-xl p-1.5 shadow-lg animate-[popIn_0.16s_ease]" > - {options.map((o) => { - const on = selected.includes(o.value); - return ( - commit(o.value)} - label={o.label} - data-testid={testId ? `${testId}-${o.value}` : undefined} - className={`flex w-full items-center gap-2 rounded-md px-2 py-1.5 text-left text-sm transition ${ - on ? "text-fg" : "text-muted" - }`} - > - {multiple ? ( - - {on && } - - ) : ( - - )} - {o.label} - {o.count != null && ( - {o.count} - )} - - ); - })} + {/* Single = one exclusive choice (radios), multi = independent toggles (checkboxes). Both are + menuitem-family roles scoped by a group, so AT announces the checked state — the surface is a + menu (see the mode above), never a bare listbox with orphaned options. */} +
+ {options.map((o) => { + const on = selected.includes(o.value); + return ( + commit(o.value)} + label={o.label} + data-testid={testId ? `${testId}-${o.value}` : undefined} + className={`flex w-full items-center gap-2 rounded-md px-2 py-1.5 text-left text-sm transition ${ + on ? "text-fg" : "text-muted" + }`} + > + {multiple ? ( + + {on && } + + ) : ( + + )} + {o.label} + {o.count != null && ( + {o.count} + )} + + ); + })} +
); diff --git a/frontend/src/components/ItemCount.dom.test.tsx b/frontend/src/components/ItemCount.dom.test.tsx new file mode 100644 index 00000000..560adc93 --- /dev/null +++ b/frontend/src/components/ItemCount.dom.test.tsx @@ -0,0 +1,45 @@ +// @vitest-environment jsdom +// +// Contract of the fixed-width count pill. Uses the REAL i18n instance (not a key-passthrough) so the +// plural selection (itemCount_one/_other) and the "1M+" overflow interpolation are actually exercised — +// those are the two branches worth guarding. English is forced so the singular/plural strings are stable. +import { describe, it, expect, beforeAll, afterEach } from "vitest"; +import { render, screen, cleanup } from "@testing-library/react"; +import "../i18n"; // side-effect: initialise the shared i18next instance +import i18n from "i18next"; +import ItemCount from "./ItemCount"; + +beforeAll(async () => { + await i18n.changeLanguage("en"); +}); +afterEach(cleanup); + +const text = () => screen.getByTestId("item-count").textContent; + +describe("ItemCount", () => { + it("uses the singular string at 1 and the plural above it", () => { + render(); + expect(text()).toBe("1 item"); + }); + + it("pluralises and thousands-separates a normal count", () => { + render(); + // Separators come from toLocaleString (runtime-locale dependent) — compare against it, not a fixed + // en-US string, so the test asserts ItemCount's formatting path rather than the CI locale. + expect(text()).toBe(`${(192045).toLocaleString()} items`); + }); + + it("collapses a count of a million or more to 1M+, with the exact figure in the tooltip", () => { + render(); + expect(text()).toBe("1M+ items"); // the "1M+" collapse is locale-independent + expect(screen.getByTestId("item-count").getAttribute("title")).toBe( + (1_000_000).toLocaleString(), + ); + }); + + it("shows the icon-only fixed box (empty label) while the count is undefined", () => { + render(); + expect(text()).toBe(""); // the Layers icon carries no text + expect(screen.getByTestId("item-count").getAttribute("title")).toBeNull(); + }); +}); diff --git a/frontend/src/components/PageShell.dom.test.tsx b/frontend/src/components/PageShell.dom.test.tsx index 39d5ffdd..3ac8da84 100644 --- a/frontend/src/components/PageShell.dom.test.tsx +++ b/frontend/src/components/PageShell.dom.test.tsx @@ -74,3 +74,54 @@ describe("PageShell --hdr-h (E5 S7)", () => { expect(content().style.getPropertyValue("--hdr-h")).toBe("68px"); // re-measured, not the stale 4px }); }); + +// The pinned-rail overlay decision: a rail pins as OVERLAY (--rail-pinned-w 2.25rem, the toolbar keeps a +// slim gutter and content stays put) once the window is wide enough that the centred content leaves room +// for the rail in its left margin (maxWidth + ~36rem); below that it pins in-flow (16.75rem). The rem→px +// conversion must use the LIVE root font-size, not a hardcoded 16, so it stays correct at every text size. +describe("PageShell --rail-pinned-w / --content-mw", () => { + const root = document.documentElement; + const setWidth = (w: number) => + Object.defineProperty(window, "innerWidth", { value: w, configurable: true }); + afterEach(() => { + root.style.removeProperty("font-size"); + root.style.removeProperty("--content-mw"); + root.style.removeProperty("--rail-pinned-w"); + }); + + it("overlays the rail once the window clears maxWidth + 36rem (at 16px/rem)", () => { + setWidth(1600); // 64rem*16 + 36rem*16 = 1024 + 576 = 1600 → exactly roomy + render(content); + expect(root.style.getPropertyValue("--content-mw")).toBe("64rem"); + expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("2.25rem"); + }); + + it("keeps the rail in-flow just below the threshold", () => { + setWidth(1599); + render(content); + expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("16.75rem"); + }); + + it("never overlays on a full-width page (no maxWidth)", () => { + setWidth(3000); + render(content); + expect(root.style.getPropertyValue("--content-mw")).toBe("none"); + expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("16.75rem"); + }); + + it("recomputes on resize", () => { + setWidth(1599); + render(content); + expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("16.75rem"); + setWidth(1600); + window.dispatchEvent(new Event("resize")); + expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("2.25rem"); + }); + + it("scales the rem threshold by the live root font-size, not a hardcoded 16", () => { + root.style.fontSize = "17px"; // as the --font-scale 1.06 default renders it + setWidth(1650); // roomy at 16px/rem (>1600) but NOT at 17px/rem (needs 100rem*17 = 1700) + render(content); + expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("16.75rem"); + }); +}); diff --git a/frontend/src/components/PageShell.tsx b/frontend/src/components/PageShell.tsx index f21f9dea..419bca41 100644 --- a/frontend/src/components/PageShell.tsx +++ b/frontend/src/components/PageShell.tsx @@ -75,20 +75,34 @@ export default function PageShell({ const root = document.documentElement; root.style.setProperty("--content-mw", maxWidth ?? "none"); // Room the pinned rail needs: its own width (~16rem) on each side of the centred content, plus the - // nav rail (~4rem). Below that, or on a full-width page (no maxWidth), pin in-flow (w-67). + // nav rail (~4rem) = ~36rem. Below that, or on a full-width page (no maxWidth), pin in-flow (w-67). + let lastPxPerRem = 0; const apply = () => { + // Convert the rem maxWidth (and the 36rem slack) against the LIVE root font-size, not a hardcoded + // 16: index.css scales the root by --font-scale (default 1.06 ≈ 17px, slider 0.9–1.3), and the page's + // own content wrapper is a rem `max-w-*`, so a fixed 16 mis-sizes the threshold at every text size. + const pxPerRem = parseFloat(getComputedStyle(root).fontSize) || 16; + lastPxPerRem = pxPerRem; const mwPx = maxWidth ? maxWidth.endsWith("rem") - ? parseFloat(maxWidth) * 16 + ? parseFloat(maxWidth) * pxPerRem : parseFloat(maxWidth) : null; - const roomy = mwPx != null && window.innerWidth >= mwPx + 576; + const roomy = mwPx != null && window.innerWidth >= mwPx + 36 * pxPerRem; root.style.setProperty("--rail-pinned-w", roomy ? "2.25rem" : "16.75rem"); }; apply(); window.addEventListener("resize", apply); + // The text-size slider changes --font-scale (hence the root font-size) WITHOUT firing resize, so the + // rem→px threshold would go stale. Re-run when the root font actually changes; the pxPerRem guard keeps + // our own --rail-pinned-w write (a rem string, no font change) from re-triggering the observer. + const fontObserver = new MutationObserver(() => { + if ((parseFloat(getComputedStyle(root).fontSize) || 16) !== lastPxPerRem) apply(); + }); + fontObserver.observe(root, { attributes: true, attributeFilter: ["style"] }); return () => { window.removeEventListener("resize", apply); + fontObserver.disconnect(); root.style.removeProperty("--content-mw"); root.style.removeProperty("--rail-pinned-w"); }; diff --git a/frontend/src/components/SortControl.dom.test.tsx b/frontend/src/components/SortControl.dom.test.tsx index 3412d73b..84c39dc1 100644 --- a/frontend/src/components/SortControl.dom.test.tsx +++ b/frontend/src/components/SortControl.dom.test.tsx @@ -2,8 +2,9 @@ // // The rendering integration of SortControl (its pickSort/heal decision logic is covered in // columnSort.test.ts). Labels are plain props here — no i18n needed. It's a floating-ui popover now -// (a labeled pill + a listbox with a footer direction segment), so we stub the two autoUpdate -// observers and wrap in a FloatingTree; options only exist while open → role queries pass { hidden: true }. +// (a labeled pill + a menu of `menuitemradio` sort keys with a footer direction segment), so we stub +// the two autoUpdate observers and wrap in a FloatingTree; rows only exist while open → role queries +// pass { hidden: true }. import { describe, it, expect, vi, afterEach, beforeAll } from "vitest"; import { render, screen, fireEvent, cleanup } from "@testing-library/react"; import { FloatingTree } from "@floating-ui/react"; @@ -55,22 +56,17 @@ describe("SortControl", () => { it("renders one option per SortOption, no no-sort row without defaultLabel", () => { renderControl({ key: "title", dir: "asc" }); openPanel(); - expect(screen.getAllByRole("option", { hidden: true }).map((o) => o.textContent)).toEqual([ - "Date", - "Title", - "Shuffle", - ]); + expect( + screen.getAllByRole("menuitemradio", { hidden: true }).map((o) => o.textContent), + ).toEqual(["Date", "Title", "Shuffle"]); }); it("adds a leading no-sort option when defaultLabel is set (tables)", () => { renderControl(null, { defaultLabel: "None" }); openPanel(); - expect(screen.getAllByRole("option", { hidden: true }).map((o) => o.textContent)).toEqual([ - "None", - "Date", - "Title", - "Shuffle", - ]); + expect( + screen.getAllByRole("menuitemradio", { hidden: true }).map((o) => o.textContent), + ).toEqual(["None", "Date", "Title", "Shuffle"]); }); it("summarises the trigger with the first option (not blank) for an unavailable key without defaultLabel", () => { @@ -81,7 +77,7 @@ describe("SortControl", () => { it("picking a new option fires onChange at that option's defaultDir", () => { const { onChange } = renderControl({ key: "title", dir: "asc" }); openPanel(); - fireEvent.click(screen.getAllByRole("option", { hidden: true })[0]!); // Date + fireEvent.click(screen.getAllByRole("menuitemradio", { hidden: true })[0]!); // Date expect(onChange).toHaveBeenCalledWith({ key: "date", dir: "desc" }); }); diff --git a/frontend/src/components/SortControl.tsx b/frontend/src/components/SortControl.tsx index 671f6d70..7bde36cb 100644 --- a/frontend/src/components/SortControl.tsx +++ b/frontend/src/components/SortControl.tsx @@ -41,7 +41,7 @@ export default function SortControl({ const pop = useAnchoredPopover({ open, onOpenChange: setOpen, - mode: "listbox", + mode: "menu", placement: "bottom-end", }); @@ -89,43 +89,47 @@ export default function SortControl({ ariaLabel={label} className="glass-menu min-w-52 rounded-xl p-1.5 shadow-lg animate-[popIn_0.16s_ease]" > - {defaultLabel !== undefined && ( - { - onChange(null); - setOpen(false); // "no sort" is a final choice — nothing left to direct - }} - label={defaultLabel} - className={`flex w-full items-center gap-2 rounded-md px-2 py-1.5 text-left text-sm transition ${ - active === null ? "text-fg" : "text-muted" - }`} - > - - {defaultLabel} - - )} - {options.map((o) => { - const on = active?.key === o.key; - return ( + {/* One exclusive choice of sort key (radios), scoped by a group so AT announces which is + checked — the surface is a menu (see the mode above), never a listbox with orphaned options. */} +
+ {defaultLabel !== undefined && ( onChange(pickSort(options, active, o.key))} // keep open to set direction - label={o.label} + role="menuitemradio" + checked={active === null} + onSelect={() => { + onChange(null); + setOpen(false); // "no sort" is a final choice — nothing left to direct + }} + label={defaultLabel} className={`flex w-full items-center gap-2 rounded-md px-2 py-1.5 text-left text-sm transition ${ - on ? "text-fg" : "text-muted" + active === null ? "text-fg" : "text-muted" }`} > - - {o.label} + + {defaultLabel} - ); - })} + )} + {options.map((o) => { + const on = active?.key === o.key; + return ( + onChange(pickSort(options, active, o.key))} // keep open to set direction + label={o.label} + className={`flex w-full items-center gap-2 rounded-md px-2 py-1.5 text-left text-sm transition ${ + on ? "text-fg" : "text-muted" + }`} + > + + {o.label} + + ); + })} +
{((showArrow && active) || trailing) && (