Skip to content

fix(docsite): measure the whole AppShell header for --appshell-header-height - #6856

Merged
kentonquatman merged 2 commits into
mainfrom
fix/docsite-canary-banner-header-offset
Oct 1, 2026
Merged

kentonquatman merged 2 commits into
mainfrom
fix/docsite-canary-banner-header-offset

Conversation

@kentonquatman

@kentonquatman kentonquatman commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

User impact

On the canary docs (the main deploy 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

  1. Why does it happen? The docsite positions page chrome against --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 :root seed.
  2. Why does that break the page? 48px is the nav alone. The canary banner lives in the same header, which is 132px tall at 1440px wide. The hero's nav backdrop (the fixed band that tints the transparent nav with the hero color) is 48px tall at the top of the viewport, so it sits behind the banner and the nav shows the shell's white surface. The pinned hero band starts 48px down, under the banner, and the docs table of contents sticks 72px down, also under it.
  3. Why does it matter? Canary is where unreleased docs and every PR preview get reviewed, and this is the first thing its home page shows.

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-height to 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 :root seed in globals.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-height on <html> to the height of the AppShell header containing the given element, follows it with a ResizeObserver, and removes it on unmount, falling back to the :root seed.
  • SharedTopNav.tsx: passes a ref to TopNav and calls the hook.
  • globals.css and (site)/page.tsx: main's top padding and the narrow hero's top padding use --spacing-12 instead 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 This PR
Header (banner + nav) 132px 132px
--appshell-header-height 48px 132px
Nav backdrop 0–48px, behind the banner 0–132px
Pinned hero band top 48px 132px
Docs table of contents sticky top 72px 156px
Docs h1 top 204px 204px (unchanged)
  • Reproduction before the change: the home page with the banner open, at 1440px and 390px, and a scrolled docs page (screenshots below).
  • Result after the change: the nav takes the hero color, the hero starts below the header, and the table of contents sticks below the banner.
  • Representative unchanged path: with the banner dismissed (and on production, which has no banner) the header is the nav alone, the variable is 48px, and the layout is identical before and after. With the banner open, docs page content and the hero at 800px and 390px keep their current positions.
  • Regression test: e2e/header-offset.spec.ts fails on main (variable 48 vs. header 132) and passes here. app-shell-header-height.test.tsx covers measuring the header rather than the nav, following a resize, unmounting, and the mobile drawer's copy of the nav.
Before (main) After
Home, 1440px Home page at 1440px with the canary banner open, before Home page at 1440px with the canary banner open, after
Home, 390px Home page at 390px with the canary banner open, before Home page at 390px with the canary banner open, after
Docs, scrolled, 1440px Getting started docs page scrolled, with the canary banner open, before Getting started docs page scrolled, with the canary banner open, after

Scope

  • One defect is restored.
  • Separable contradictory or unsettled tagalongs were removed or split.
  • No new public API, visual representation, default, or interaction model is hidden under the bug-fix label.
  • Public text and artifacts contain no internal Meta context.

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, then pnpm test:docsite-browser: all 14 specs pass, including the new one.
  • Real browser (Chromium, production build): the home page at 1440, 800, and 390px with the banner open and dismissed; /docs/getting-started at 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.

…-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.
@vercel

vercel Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Oct 1, 2026 4:36pm UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 1, 2026
@astracat-bot

astracat-bot Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Oct 1, 2026, 4:41 PM UTC

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR

🧪 Sandbox Preview

View Sandbox for this PR

No new or modified components detected.

Bundle Size Summary

No component packages changed.

Accessibility Audit

Status: No accessibility violations detected.

Visual Regression

14 of 420 shot(s) changed.

A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.

component story theme mode pixels
Chat Full AI Chat probe light 155,733
Chat Full AI Chat probe dark 155,005
Chat Default neutral dark 33,205
Chat Default neutral light 32,876
Chat Empty State probe light 23,531
Chat Empty State probe dark 22,622
ChatAutoScroll Scroll Behavior Comparison neutral dark 15,378
ChatAutoScroll Scroll Behavior Comparison neutral light 14,107
Chat With Attachments probe light 13,229
Chat With Attachments probe dark 11,065
Markdown Default probe dark 897
Markdown Default probe light 897
Markdown Default neutral dark 882
Markdown Default neutral light 858

Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

@kentonquatman
kentonquatman merged commit 737ec59 into main Oct 1, 2026
43 checks passed
@kentonquatman
kentonquatman deleted the fix/docsite-canary-banner-header-offset branch October 1, 2026 17:17

This branch was successfully deployed

1 active deployment
Preview — 4d397ad5 Deployed Oct 1, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant