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).
140 lines
6.7 KiB
TypeScript
140 lines
6.7 KiB
TypeScript
// @vitest-environment jsdom
|
||
//
|
||
// E5 S7: PageShell measures the floating header and publishes its real height as `--hdr-h` onto the
|
||
// content wrapper, so a header that flex-wraps to 2–3 rows on a narrow window never overlaps the
|
||
// content below it. jsdom does no layout (offsetHeight is 0) and has no ResizeObserver, so we stub
|
||
// both: a ResizeObserver that hands us its callback, and offset metrics on the measured header.
|
||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||
import { render, cleanup } from "@testing-library/react";
|
||
import PageShell from "./PageShell";
|
||
|
||
vi.mock("./PageScroller", () => ({
|
||
default: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||
}));
|
||
|
||
let roCb: (() => void) | null = null;
|
||
beforeEach(() => {
|
||
roCb = null;
|
||
vi.stubGlobal(
|
||
"ResizeObserver",
|
||
class {
|
||
constructor(cb: () => void) {
|
||
roCb = cb;
|
||
}
|
||
observe() {}
|
||
disconnect() {}
|
||
},
|
||
);
|
||
});
|
||
afterEach(() => {
|
||
cleanup();
|
||
vi.unstubAllGlobals();
|
||
});
|
||
|
||
describe("PageShell --hdr-h (E5 S7)", () => {
|
||
it("publishes the measured header height onto the content wrapper", () => {
|
||
const { container } = render(
|
||
<PageShell header={<div data-testid="hdr">H</div>}>content</PageShell>,
|
||
);
|
||
const content = container.querySelector('[class*="pt-(--hdr-h)"]') as HTMLElement;
|
||
expect(content).toBeTruthy();
|
||
// Give the header a real (multi-row) box, then fire the ResizeObserver callback the effect made.
|
||
const hdr = container.querySelector('[data-testid="hdr"]') as HTMLElement;
|
||
Object.defineProperty(hdr, "offsetTop", { value: 12, configurable: true });
|
||
Object.defineProperty(hdr, "offsetHeight", { value: 96, configurable: true });
|
||
roCb?.();
|
||
expect(content.style.getPropertyValue("--hdr-h")).toBe("112px"); // 12 + 96 + 4
|
||
});
|
||
|
||
it("adds no `--hdr-h` pad when there is no header", () => {
|
||
const { container } = render(<PageShell>content</PageShell>);
|
||
expect(container.querySelector('[class*="pt-(--hdr-h)"]')).toBeNull();
|
||
});
|
||
|
||
it("re-measures when the header is removed then restored (feed→channel→back)", () => {
|
||
// PageShell is REUSED across feed↔channel navigation (same component position in App.tsx) — it
|
||
// never remounts, so the header DOM node is swapped under a stable PageShell. The measurement
|
||
// must re-run on that swap; a one-shot effect kept observing the FIRST node and, when it detached,
|
||
// rewrote --hdr-h to a stale 4px that stuck on Back. Regression for the channel-Back header overlap.
|
||
const hdr = () => <div data-testid="hdr">H</div>;
|
||
const { container, rerender } = render(<PageShell header={hdr()}>content</PageShell>);
|
||
const measure = (top: number, h: number) => {
|
||
const el = container.querySelector('[data-testid="hdr"]') as HTMLElement;
|
||
Object.defineProperty(el, "offsetTop", { value: top, configurable: true });
|
||
Object.defineProperty(el, "offsetHeight", { value: h, configurable: true });
|
||
roCb?.();
|
||
};
|
||
measure(12, 52);
|
||
const content = () => container.querySelector('[class*="pt-(--hdr-h)"]') as HTMLElement;
|
||
expect(content().style.getPropertyValue("--hdr-h")).toBe("68px"); // 12 + 52 + 4
|
||
|
||
rerender(<PageShell>content</PageShell>); // → channel page: header gone
|
||
rerender(<PageShell header={hdr()}>content</PageShell>); // → Back: header returns as a NEW node
|
||
measure(12, 52); // fires the NEW header's observer
|
||
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(<PageShell maxWidth="64rem">content</PageShell>);
|
||
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(<PageShell maxWidth="64rem">content</PageShell>);
|
||
expect(root.style.getPropertyValue("--rail-pinned-w")).toBe("16.75rem");
|
||
});
|
||
|
||
it("never overlays on a full-width page (no maxWidth)", () => {
|
||
setWidth(3000);
|
||
render(<PageShell>content</PageShell>);
|
||
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(<PageShell maxWidth="64rem">content</PageShell>);
|
||
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(<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");
|
||
});
|
||
});
|