diff --git a/.changeset/tidy-rails-stack.md b/.changeset/tidy-rails-stack.md new file mode 100644 index 00000000..ded55102 --- /dev/null +++ b/.changeset/tidy-rails-stack.md @@ -0,0 +1,26 @@ +--- +"@qorpe/ui": patch +--- + +fix(app-shell): the head joins the icon column instead of sitting above it + +Collapsed, the head is now ONE rail item: the mark at rest, the expand chevron under the pointer +or under keyboard focus, in the same 36x40 box as every item below. That slot's action never +changes — it expands, whichever glyph it is showing — so the swap is an affordance rather than a +mode, and a device with no hover still gets a mark it can tap. + +This replaces what was there: the mark and the toggle side by side in the 50px the rail leaves +between its own padding, both `shrink-0` and together wanting about 70px, so `justify-center` +split the overflow and put the mark some 15px off the column every nav row centres on. Stacking +them fixed the centring but cost a row, which made the head the only part of the rail whose +height moved. One slot costs neither. + +Going home is not on that slot. A rail already showing its nav has its own home item a row or two +down, and a second hidden action on a control whose glyph changes under the cursor is a guess. + +Collapsed with no mark, the toggle keeps its own slot and tooltip — it just gets the column's +36x40 now instead of a 30px box of its own. + +Expanded, the brand head faded on hover — the only hover in the rail that was not a background +change. It now takes the same rounded box, wash and `px-2.5` as the items under it, and the head +gives up its own `px-1` so the two washes start on the same line rather than four pixels apart. diff --git a/.gitignore b/.gitignore index 368f9c1d..a97f9439 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,8 @@ node_modules dist coverage .vite + +# Playwright writes actual/diff PNGs here on every run, update-visual.sh included — artefacts +# of a run, never inputs to one. +test-results/ +playwright-report/ diff --git a/src/components/AppShell.test.tsx b/src/components/AppShell.test.tsx index f5820479..2e4a14c8 100644 --- a/src/components/AppShell.test.tsx +++ b/src/components/AppShell.test.tsx @@ -245,6 +245,62 @@ describe("the v1.1 rail (u7-b1)", () => { expect(onHome).toHaveBeenCalledTimes(1); }); + it("collapsed, the mark and the expand control share ONE slot — the head never gains a row", async () => { + const onToggleCollapsed = vi.fn(); + const onHome = vi.fn(); + render( + } title="Mockifyr" nav={[item("a")]} activeId="a" collapsed + onHome={onHome} onToggleCollapsed={onToggleCollapsed}>x, + ); + // Exactly one control, holding the mark. A second would push the icon column down a row and + // make the head the only part of the rail whose height moves between states. + const slot = screen.getByRole("button", { name: /expand navigation/i }); + expect(screen.getAllByRole("button", { name: /expand navigation/i })).toHaveLength(1); + expect(slot).toContainElement(screen.getByTestId("mark")); + + // Its action does not depend on the pointer being over it — which is what makes the glyph + // swap an affordance rather than a mode, and what keeps it working on a device with no hover. + await userEvent.click(slot); + expect(onToggleCollapsed).toHaveBeenCalledTimes(1); + expect(onHome).not.toHaveBeenCalled(); + }); + + it("collapsed with no way to expand, the mark is just the mark", () => { + render( + } title="Mockifyr" nav={[item("a")]} activeId="a" collapsed>x, + ); + expect(screen.getByTestId("mark")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /expand navigation/i })).not.toBeInTheDocument(); + }); + + it("collapsed, the toggle takes a rail item's slot instead of its own smaller one", () => { + const slot = (el: HTMLElement) => + el.className.split(" ").filter((c) => ["h-9", "w-10", "rounded-lg"].includes(c)).sort().join(" "); + const { rerender } = render( + {}}>x, + ); + // The measure is the nav item beside it, not a literal — the two must agree, whatever they are. + expect(slot(screen.getByRole("button", { name: /expand navigation/i }))) + .toBe(slot(screen.getByRole("button", { name: "a" }))); + + // Expanded it is not in that column at all, so it keeps its own smaller affordance. + rerender( {}}>x); + expect(screen.getByRole("button", { name: /collapse navigation/i }).className).toContain("p-1.5"); + }); + + it("expanded, the brand head hovers like the items under it, not like a fading link", () => { + render( + } title="Mockifyr" nav={[item("a"), item("b")]} activeId="a" onHome={() => {}}>x, + ); + const head = screen.getByRole("button", { name: /Mockifyr/ }); + // The RESTING item, not the active one — the active row wears the accent instead of a hover. + const navItem = screen.getByRole("button", { name: "b" }); + // The measure is the item beside it: whatever the rail's hover is, the head must use it. + for (const c of ["hover:bg-muted", "rounded-lg", "px-2.5"]) expect(navItem.className).toContain(c); + for (const c of ["hover:bg-muted", "rounded-lg", "px-2.5"]) expect(head.className).toContain(c); + expect(head.className).not.toContain("hover:opacity-70"); + }); + it("a count badge is NEUTRAL unless the console says it is alarming", () => { render(