Repository navigation
fix(docsite): measure the whole AppShell header for --appshell-header-height - #6856
Conversation
…-height AppShell stopped setting --appshell-header-height in #4944 (it now writes a private --_app-shell-header-height), so the docsite's variable has stayed at its 48px :root seed. That is the height of the nav alone, which only matches the header while no banner is showing. On canary the banner makes the header taller, so the home page's nav backdrop stays a 48px strip hidden behind the banner (the nav shows a white bar instead of the hero color), the pinned hero starts under the banner, and the docs table of contents sticks under it. SharedTopNav, which every page shell renders, now measures the AppShell header it sits in and publishes that height on the root element, following resizes such as dismissing the banner. The two rules that used the variable as an in-flow gap below the header (main's top padding and the narrow hero's top padding) now use --spacing-12, so they keep today's spacing instead of growing by the banner's height.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: No accessibility violations detected. Visual Regression14 of 420 shot(s) changed. A change here is a question, not a failure: check whether the after is the
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
There was a problem hiding this comment.
Approval recommended. This restores the docsite's header-height variable to the whole AppShell header (banner plus nav) that its hero, docs table of contents, and anchor offsets were written against, without reading the core's private property or changing any public API. The two in-flow paddings keep their current 48px spacing, so page content does not move, and production and banner-dismissed layouts keep today's values.
The new unit test covers measuring the header rather than the nav, following the banner-dismiss resize, restoring the stylesheet seed on unmount, and ignoring the nav copy outside the header; the browser regression and before/after screenshots cover the canary banner-open home and docs pages. No concrete defect was found in the remaining consumers of the variable.
[Automated review]
User impact
On the canary docs (the
maindeploy and every PR preview), the home page shows a white bar behind the top nav while the "You're viewing unreleased docs (Canary)" banner is open, and the hero wordmark sits jammed against the nav. Dismissing the banner hides the problem. On docs pages, the "On this page" list sticks partly under the banner. The latest (production) site has no banner and is unaffected.Problem and solution fit
--appshell-header-height. AppShell used to set that variable to the measured header height; since fix(core): AppShell audit: banner landmark on mobile, shared skip-link focus ring, corrected mobileNav doc examples #4944 it only writes a private--_app-shell-header-height, so the docsite's variable has stayed at its 48px:rootseed.The fix gives the variable its intended meaning again, the full header height, without reaching into AppShell's private property.
SharedTopNav, which all four page shells render, measures the AppShell header it sits in and publishes that height on the root element, re-measuring when the header resizes (banner dismissed, viewport change). Two rules used the variable as an in-flow gap below the header rather than as an offset from the top of the viewport; they now use--spacing-12, the value they effectively had, so page content does not move.Expected behavior and authority
Regression history: until #4944, AppShell set
--appshell-header-heightto the measured header height, and the docsite's consumers (HeroThemeReel's nav backdrop and pinned layers,HeroFloatingCards,DocPageLayout's table of contents,AnchorHeading,ThemeExplorerLayout, and the:rootseed inglobals.css) are written against that contract. The banner-dismissed state, where the header is the nav alone, shows the intended layout.Smallest restoration
src/lib/useAppShellHeaderHeight.ts(new): sets--appshell-header-heighton<html>to the height of the AppShell header containing the given element, follows it with aResizeObserver, and removes it on unmount, falling back to the:rootseed.SharedTopNav.tsx: passes a ref toTopNavand calls the hook.globals.cssand(site)/page.tsx: main's top padding and the narrow hero's top padding use--spacing-12instead of the variable; comments updated to match.No core change and no new public API. Before hydration, canary's first paint still uses the 48px seed, as it did before #4944.
Evidence
Production build in Chromium, 1440×900, banner open:
main--appshell-header-heighth1tope2e/header-offset.spec.tsfails onmain(variable 48 vs. header 132) and passes here.app-shell-header-height.test.tsxcovers measuring the header rather than the nav, following a resize, unmounting, and the mobile drawer's copy of the nav.main)Scope
Testing
pnpm -F @astryxdesign/docsite generate && pnpm -F @astryxdesign/docsite test: 44 files, 552 tests pass.pnpm -F @astryxdesign/docsite typecheck,pnpm check:repo, and ESLint and Prettier on the changed TypeScript files.pnpm -F @astryxdesign/docsite build, thenpnpm test:docsite-browser: all 14 specs pass, including the new one./docs/getting-startedat the top and scrolled.#5480 and #5542 rework the hero's pinning in the same files but don't change where this variable comes from; whichever lands second needs a small rebase in
page.tsx.