fix(a11y,test): deep-review round 2 — e2e helper attr, trigger names, observer test
Second gate round over the full branch surfaced 4 verified findings (1 was a
regression the round-1 role change introduced); a 5th (--sbw load jump) was
dropped as refuted (the header only mounts after auth+data, so --sbw is set
before it ever paints).
- e2e (medium): the round-1 switch of FilterSelect rows from role=option to
menuitemcheckbox moved their state from aria-selected to aria-checked, but
the Playwright helper expectContentType still asserted aria-selected, so the
channel specs would fail whenever the e2e lane runs (not part of the fast
gate, so it stayed green). Assert aria-checked.
- a11y (low): the FilterSelect and SortControl pill triggers set
aria-label={label}, which overrides the visible "Label: value" text, so a
screen reader announced only the group name, never the current selection.
Carry the value (and, for sort, the direction) in the accessible name,
matching ViewSwitcher. Updated the unit-test trigger name queries to match.
- tests (low): cover the PageShell font-scale MutationObserver recompute path
(a post-mount root font-size change → --rail-pinned-w re-computes once, no
loop) — previously only the pre-render and resize paths were exercised.
Verified live: the triggers now announce e.g. "Sort: Date, Descending" /
"Watch state: All". Gate green: tsc (app+node+e2e), 373 vitest, prettier,
eslint (0 err).
This commit is contained in:
@@ -36,12 +36,13 @@ export async function clickVisibleChannelLink(page: Page): Promise<boolean> {
|
||||
/**
|
||||
* Assert a content-type (e.g. "includeLive") is on/off via the Content-types FilterSelect dropdown.
|
||||
* The options only exist while the dropdown is open, so this opens it, checks the option's
|
||||
* `aria-selected`, then closes it (Escape) so the floating panel can't block a later click.
|
||||
* `aria-checked` (the multi FilterSelect renders its rows as menuitemcheckbox), then closes it
|
||||
* (Escape) so the floating panel can't block a later click.
|
||||
*/
|
||||
export async function expectContentType(page: Page, value: string, on: boolean): Promise<void> {
|
||||
await page.getByTestId("feed-content").click();
|
||||
await expect(page.getByTestId(`feed-content-${value}`)).toHaveAttribute(
|
||||
"aria-selected",
|
||||
"aria-checked",
|
||||
on ? "true" : "false",
|
||||
);
|
||||
await page.keyboard.press("Escape");
|
||||
|
||||
@@ -38,7 +38,7 @@ const tree = (ui: React.ReactNode) => render(<FloatingTree>{ui}</FloatingTree>);
|
||||
describe("FilterSelect", () => {
|
||||
it("renders a labeled trigger `Label: value` and keeps options unmounted until opened", () => {
|
||||
tree(<FilterSelect label="Status" options={STATUS} selected={["all"]} onChange={vi.fn()} />);
|
||||
const trigger = screen.getByRole("button", { name: "Status" });
|
||||
const trigger = screen.getByRole("button", { name: /^Status/ });
|
||||
expect(trigger.textContent).toContain("Status:");
|
||||
expect(trigger.textContent).toContain("All");
|
||||
expect(screen.queryByRole("menuitemradio", { hidden: true })).toBeNull();
|
||||
@@ -47,7 +47,7 @@ describe("FilterSelect", () => {
|
||||
it("single: picking a row replaces the selection and closes", () => {
|
||||
const onChange = vi.fn();
|
||||
tree(<FilterSelect label="Status" options={STATUS} selected={["all"]} onChange={onChange} />);
|
||||
fireEvent.click(screen.getByRole("button", { name: "Status" }));
|
||||
fireEvent.click(screen.getByRole("button", { name: /^Status/ }));
|
||||
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
|
||||
@@ -61,7 +61,7 @@ describe("FilterSelect", () => {
|
||||
tree(
|
||||
<FilterSelect label="Type" multiple options={TYPES} selected={["n"]} onChange={onChange} />,
|
||||
);
|
||||
fireEvent.click(screen.getByRole("button", { name: "Type" }));
|
||||
fireEvent.click(screen.getByRole("button", { name: /^Type/ }));
|
||||
fireEvent.click(screen.getAllByRole("menuitemcheckbox", { hidden: true })[1]!); // Shorts
|
||||
expect(onChange).toHaveBeenCalledWith(["n", "s"]);
|
||||
expect(screen.queryAllByRole("menuitemcheckbox", { hidden: true }).length).toBeGreaterThan(0); // still open
|
||||
@@ -72,7 +72,7 @@ describe("FilterSelect", () => {
|
||||
tree(
|
||||
<FilterSelect label="Type" multiple options={TYPES} selected={["n"]} onChange={onChange} />,
|
||||
);
|
||||
fireEvent.click(screen.getByRole("button", { name: "Type" }));
|
||||
fireEvent.click(screen.getByRole("button", { name: /^Type/ }));
|
||||
fireEvent.click(screen.getAllByRole("menuitemcheckbox", { hidden: true })[0]!); // Normal — the only one selected
|
||||
expect(onChange).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -87,6 +87,6 @@ describe("FilterSelect", () => {
|
||||
onChange={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByRole("button", { name: "Type" }).textContent).toContain("Normal +1");
|
||||
expect(screen.getByRole("button", { name: /^Type/ }).textContent).toContain("Normal +1");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -81,7 +81,9 @@ export default function FilterSelect({
|
||||
ref={pop.refs.setReference}
|
||||
{...pop.getReferenceProps()}
|
||||
data-testid={testId}
|
||||
aria-label={label}
|
||||
// Carry the current value in the accessible name (not just the group label), matching
|
||||
// ViewSwitcher — a screen reader announces "Watch state: Unwatched", not a bare "Watch state".
|
||||
aria-label={`${label}: ${summary}`}
|
||||
className={`glass-card glass-hover inline-flex items-center gap-1.5 rounded-full border px-3 py-1 text-sm transition ${
|
||||
open ? "border-accent" : "border-border"
|
||||
}`}
|
||||
|
||||
@@ -124,4 +124,16 @@ describe("PageShell --rail-pinned-w / --content-mw", () => {
|
||||
render(<PageShell maxWidth="64rem">content</PageShell>);
|
||||
expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("16.75rem");
|
||||
});
|
||||
|
||||
it("recomputes at runtime when --font-scale changes the root font, without looping", async () => {
|
||||
setWidth(1650); // roomy at 16px/rem (1650 >= 1600) → overlay
|
||||
render(<PageShell maxWidth="64rem">content</PageShell>);
|
||||
expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("2.25rem");
|
||||
// The text-size slider changes the root font-size with NO resize event — the MutationObserver
|
||||
// must recompute. At 17px/rem the threshold is 100rem*17 = 1700 > 1650, so the rail goes in-flow.
|
||||
// (If the observer's self-write guard were broken this would loop forever and time out.)
|
||||
root.style.fontSize = "17px";
|
||||
await new Promise((r) => setTimeout(r, 0)); // flush the MutationObserver microtask
|
||||
expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("16.75rem");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -48,7 +48,7 @@ function renderControl(sort: SortState, extra: Record<string, unknown> = {}) {
|
||||
return { onChange };
|
||||
}
|
||||
|
||||
const openPanel = () => fireEvent.click(screen.getByRole("button", { name: "Sort" }));
|
||||
const openPanel = () => fireEvent.click(screen.getByRole("button", { name: /^Sort/ }));
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
@@ -71,7 +71,7 @@ describe("SortControl", () => {
|
||||
|
||||
it("summarises the trigger with the first option (not blank) for an unavailable key without defaultLabel", () => {
|
||||
renderControl({ key: "ghost", dir: "asc" });
|
||||
expect(screen.getByRole("button", { name: "Sort" }).textContent).toContain("Date");
|
||||
expect(screen.getByRole("button", { name: /^Sort/ }).textContent).toContain("Date");
|
||||
});
|
||||
|
||||
it("picking a new option fires onChange at that option's defaultDir", () => {
|
||||
|
||||
@@ -67,7 +67,12 @@ export default function SortControl({
|
||||
type="button"
|
||||
ref={pop.refs.setReference}
|
||||
{...pop.getReferenceProps()}
|
||||
aria-label={label}
|
||||
// Carry the active sort key AND direction in the accessible name (the visible arrow is
|
||||
// aria-hidden), matching ViewSwitcher — a screen reader announces "Sort: Date, Descending",
|
||||
// not a bare "Sort".
|
||||
aria-label={`${label}: ${summary}${
|
||||
showArrow && active ? `, ${active.dir === "asc" ? ascLabel : descLabel}` : ""
|
||||
}`}
|
||||
className={`glass-card glass-hover inline-flex items-center gap-1.5 rounded-full border px-3 py-1 text-sm transition ${
|
||||
open ? "border-accent" : "border-border"
|
||||
}`}
|
||||
|
||||
Reference in New Issue
Block a user