fix(ui): align the shell to the content grid - #39
Conversation
The header was full-bleed while page content sat in a centred .page column, so the seal, the nav and the page H1 landed on three different verticals — a violation of the system's own ruled-form premise (DESIGN.md "Layout"). - Wrap the header contents in .shell__bar, constrained to --measure-page with .page's horizontal padding. The bar itself stays full-bleed; only the contents are measured. The measure is fixed at the wide value on every route, so .page--narrow pages do not move the frame as you navigate. - Introduce --measure-page, shared by .page and .shell__bar, so the two cannot drift apart. - Replace the active-nav gold outlined box with a gold hairline under the label. The box was the only bordered element in the header, so it read as a button and competed with the page's one primary action. aria-current and the global focus ring are unchanged. - Make the wordmark hover consistent. The gold seen on /admin/sync was the global a:hover leaking past .shell__mark's colour, and it reached only the first line because the second sets --ink-faint on itself. Both lines now respond together. - Let the /account footnote hairline run the full content column so it terminates on the crew-manifest table's right edge; the prose keeps its 68ch cap on an inner span.
The shell bar was pinned to --measure-page on every route, so on /account (a .page--narrow route at 60rem) the seal sat 144px inside the H1. Give SiteHeader a `measure` prop that selects the matching bar variant, and introduce --measure-page-narrow so .page--narrow and .shell__bar--narrow share one literal. The measure is passed rather than derived from the DOM for the same reason `current` is: SiteHeader stays a server component, and this avoids betting on Next's layout DOM shape with :has(+ ...). Trade-off: the header shifts when an admin crosses between /account and /admin/*. Members only ever see /account, so the frame is fixed for almost everyone, and admins get alignment on all four screens instead of three. Verified: seal.left === h1.left on /account (264), /admin/accounts, /admin/audit and /admin/sync (120) at 1440px, and on all four (16) at 390px.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Comment |
Two conflicts, both the same collision: #39 reworked the standing footnote (wrapping its prose in a span so the hairline runs the full content column while the text stays capped at 68ch) while this branch removes the footnote entirely, having moved that text onto the CONTACTS column header. Resolved by taking this branch's deletion of the markup and main's improved .footnote CSS verbatim. The rule is now unused — kept, as noted in the PR, because PR 6 adds the closing-beat content to the empty lower page and is the likely consumer. Discarding main's refinement to a rule we are keeping would have thrown away work for no gain.
The shell bar took a `measure` prop and tracked whichever column the page under it used, so it was 960px on /account and /payouts/new and 1248px everywhere else. Every crossing moved the seal and the nav 144px sideways. That trade was made in #39 and priced as admin-only, on the premise that a member only ever sees /account. Payouts falsified it: /payouts is wide, /payouts/new is narrow, and a plain member walks that list -> form path in sequence, watching the chrome slide under them mid-task. Pin the bar to --measure-page on every route and drop the prop. The page measures are untouched: .page--narrow still holds the reading column at 60rem. The cost is that on the two narrow routes the seal sits 144px outboard of the H1, and the nav's right edge 144px outboard of the content's — symmetric, and less than chrome that won't hold still. e2e/shell.spec.ts asserts the bar's width and centring are identical on all seven routes that render one, measuring the rect rather than the absence of a class name.
1 participant
Aligns the header to the content grid, so every later screen's alignment is judged against a fixed edge rather than a moving one.
What changed
1. The header's contents now sit on the page measure. A
.shell__barwrapper inside<header class="shell">carries the layout. The bar's ground and bottom hairline stay full-bleed — the shell is the frame the page sits in, not a thing inside it — while only the contents are measured, with padding matching.page. The seal's left edge lands on the H1's left edge and the nav's right edge lands on the content column's right edge.2. The active nav tab is a gold underline, not an outlined box. The box was the only bordered element in the header, so it read as a button and competed with the page's one primary action — and DESIGN.md rations gold to one primary action per view plus the mark. A hairline under the label is the system's own structural device (it's what
.rule-headis built from) and spends 1px of gold instead of a whole outline. It sits on a pseudo-element rather than a border, so it can't shift the item's box.aria-current="page"and the focus ring are untouched.3. The gold wordmark on
/admin/syncwas a hover state, not an inconsistency. Measured computed colour is--inkon all four pages. The cause: the globala:hover(specificity 0-1-1) beats.shell__mark(0-1-0), but.shell__wordmark spansets--ink-fainton itself and blocks the second line — so half the mark flipped gold and half sat still. Both lines are now declared on.shell__mark:hoverso the mark responds as one object.4. Rule lengths audited. On
/accountthe footnote rule stopped short of the crew-manifest table's right edge, because.footnotecapped the whole block at--measure(68ch). The rule now runs the full column width; the prose keeps its 68ch cap on an inner<span>, since 68ch is a reading limit and not a structural one. That was the only mismatched hairline found.Deviation from the spec — per-route header measure
The spec asked for both "the seal's left edge lands on the H1's left edge" (verified on all four pages) and "pages using
.page--narrowkeep the header at the wider 78rem measure — do not make the header width vary by route." Those can't both hold on/account, whose column is 60rem: obeying the second rule leaves the seal 144px inside the H1.The first commit follows the explicit rule and aligns three of four pages. The second, after discussion, switches to the per-route measure so all four align.
SiteHeadertakes ameasure?: "wide" | "narrow"prop, and--measure-page-narrowis shared by.page--narrowand.shell__bar--narrowso the two can't drift.The measure is passed as a prop rather than derived with
:has(+ .page--narrow)for the same reasoncurrentis a prop:SiteHeaderstays a server component, and this avoids betting on Next not inserting a wrapper between<AdminNav />and{children}, which would fail silently on a framework upgrade. There's also no:has()precedent inglobals.css. The cost is that a future narrow page must remember the prop — noted in the component's doc comment.The accepted trade-off: the header shifts 144px when an admin crosses between
/accountand/admin/*. Members only ever see/account, so the frame is fixed for almost everyone, and admins get alignment on all four screens instead of three.Verification
Measured edges, authenticated as an admin:
markLeft === h1Lefton all four pages, at both viewports.npm run typecheck— clean, no output.npm run lint—✖ 4 problems (0 errors, 4 warnings). All four are pre-existing@next/next/no-img-elementwarnings on<img>tags this PR doesn't touch.npm run format:check—All matched files use Prettier code style!e2e — 8 passed (19.3s), all of
e2e/account.spec.tsande2e/admin.spec.ts.One caveat on that last run: it is not the plain
npm run test:e2einvocation. Port 3111 and theauthgd_testdatabase were both in use by a concurrent worktree, andreuseExistingServer: !CIsilently serves that worktree's code while itsresetDb()truncates the shared tables mid-run. I ran the same specs on port 3149 against a privateauthgd_test_shellaligndatabase — only the port and DB URL differ.Where to look
src/app/globals.cssfor the measure tokens and the.shell__bar/ active-tab rules;src/app/_components/ui.tsxfor the wrapper and the new prop. Note the spec namedsrc/app/layout.tsxandsrc/app/_components/admin-nav.tsx, butSiteHeaderactually lives inui.tsx—admin-nav.tsxis just the"use client"shim that readsusePathname, and is unchanged.Colours, type scale, spacing tokens, and the login page are untouched.
🤖 Generated with Claude Code