peter
4ee452b854
Epic-end deep-review of the branch surfaced six verified findings; fix them. a11y (medium): FilterSelect and SortControl opened their AnchoredPopover in mode:"listbox" but never passed a role, so the surface fell through to role="menu" while the rows rendered role="option"+aria-selected — options orphaned outside a listbox, so AT could not announce the selected value. Switch both to the house menu pattern (as ViewSwitcher already uses): a menu surface with menuitemradio (single) / menuitemcheckbox (multi) rows carrying aria-checked, scoped by a role=group. Trigger stays a button+haspopup=menu. a11y (low): the SortControl asc/desc direction segment signalled the active direction by colour alone. Add aria-pressed so the current direction is programmatically exposed, matching SegmentedToggle. correctness (low): PageShell converted a rem maxWidth to px with a hardcoded 16 to decide the pinned-rail overlay threshold, but index.css scales the root font by --font-scale (default 1.06 ~ 17px, slider 0.9-1.3), so the threshold was wrong at every text size on the rem pages (playlists is the only one with a pinned rail). Use the live root font-size, scale the 36rem slack the same way, and recompute when --font-scale changes (a guarded MutationObserver — the slider fires no resize). tests: cover the two gaps the review flagged — ItemCount (plural, 1M+ overflow, undefined icon-only, tooltip) and the PageShell rail-pinned-w/content-mw threshold math incl. the font-scale scaling. Updated the FilterSelect/ SortControl role queries to the new menuitemradio/checkbox roles. Verified in the running app: menu surfaces carry role=group + menuitemradio/ checkbox with 0 orphaned options, and the sort direction segment exposes aria-pressed. Dropped two review findings (max-width duplication, pill-class reuse) as refuted on adversarial verification. Gate green: tsc, 372 vitest, prettier, eslint (0 err).
46 lines
1.9 KiB
TypeScript
46 lines
1.9 KiB
TypeScript
// @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(<ItemCount count={1} />);
|
|
expect(text()).toBe("1 item");
|
|
});
|
|
|
|
it("pluralises and thousands-separates a normal count", () => {
|
|
render(<ItemCount count={192045} />);
|
|
// 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(<ItemCount count={1_000_000} />);
|
|
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(<ItemCount />);
|
|
expect(text()).toBe(""); // the Layers icon carries no text
|
|
expect(screen.getByTestId("item-count").getAttribute("title")).toBeNull();
|
|
});
|
|
});
|