From c1dde39c6885b18706498624303fdbb84ae46400 Mon Sep 17 00:00:00 2001 From: Brian Love Date: Tue, 1 Sep 2026 15:30:44 -0700 Subject: [PATCH] fix(website): close the docs shell's nav-height and column-measure defects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Polish pass over the /docs single-pane reading experience, following the footer removal in #932. Three real defects, each measured rather than inferred, plus the parity gap on the one route that wore no chrome. --nav-h was wrong by 15px from 768px to 1023px. The nav has three heights, not two: padding steps at md (768) but the tall `hidden lg:flex` link row only appears at lg (1024), leaving a 66px nav in between. The variable jumped straight to 81px at md, so every offset in that band overshot — dead space above the docs column, and the mobile drawer (top: nav-h - 1px) hanging 14px below the nav it attaches to. This is the nav-height coupling the migration comment in pages.css deferred; it lived in chrome.css, not in the docs shell, and it is site-wide. The article's `overflow-x-hidden` is removed rather than relocated. It was redundant — global.css already clips the body — and it was the very mistake that rule's own comment warns about: `overflow-x: hidden` computes `overflow-y: auto`, so it made every docs article a scroll container. All 123 docs URLs were swept at 375px with it removed; none overflow. The breadcrumb/page-header block now shares the article's md:max-w-3xl measure, which the article and the prev/next rail already used. It had stretched to the full content width, floating PageActions ~500px right of its column (1272px against 768px at 1920). /docs/choosing-an-adapter gains the TOC rail. It carries as many headings as any library page but had no rail. It stays library-neutral, so it takes no breadcrumb or page header — both are keyed to a library it deliberately has not picked. Tests: e2e/nav-height.spec.ts pins --nav-h to the rendered nav at all six breakpoint edges (jsdom cannot measure layout, so a real browser is the only place these can be compared). e2e/docs-shell.spec.ts covers the TOC rail, which had no tests at all, and the shared column edge. The existing horizontal-overflow guard in website.spec.ts was vacuous: it measured documentElement.scrollWidth, which the global body clip pins to the viewport, so its 24 assertions could never fail. It now measures whether content escapes its own column, exempting content inside its own horizontal scroller. Every new and rewritten guard was mutation-tested against the defect it describes. Verified: 447 unit tests pass (unchanged), 72 e2e pass (59 pre-existing + 13 new), lint 0 errors, production build succeeds. Co-Authored-By: Claude Opus 5 --- apps/website/e2e/docs-shell.spec.ts | 93 +++++++++++++++++++ apps/website/e2e/nav-height.spec.ts | 81 ++++++++++++++++ apps/website/e2e/website.spec.ts | 34 ++++++- .../docs/[library]/[section]/[slug]/page.tsx | 9 +- .../src/app/docs/choosing-an-adapter/page.tsx | 10 +- apps/website/src/styles/chrome.css | 27 ++++-- apps/website/src/styles/docs.css | 5 +- apps/website/src/styles/pages.css | 19 +++- 8 files changed, 258 insertions(+), 20 deletions(-) create mode 100644 apps/website/e2e/docs-shell.spec.ts create mode 100644 apps/website/e2e/nav-height.spec.ts diff --git a/apps/website/e2e/docs-shell.spec.ts b/apps/website/e2e/docs-shell.spec.ts new file mode 100644 index 000000000..0939a345c --- /dev/null +++ b/apps/website/e2e/docs-shell.spec.ts @@ -0,0 +1,93 @@ +import { test, expect } from '@playwright/test'; + +const ARTICLE = '/docs/langgraph/getting-started/introduction'; + +/** + * The docs shell is one reading pane: a sticky control plane on the left, one + * prose column, and a sticky TOC rail on the right. These guard the parts of + * that whose failure mode is silence — a rail that stops tracking, a column + * that stops sharing its measure — and which jsdom cannot see. + */ + +test.describe('DocsTOC rail', () => { + test('tracks the reading position on a hard load', async ({ page }) => { + await page.setViewportSize({ width: 1440, height: 900 }); + await page.goto(ARTICLE); + await expect(page.locator('.docs-toc-link').first()).toBeVisible(); + + // Nothing is active at the top: the first heading is below the reading line. + await expect(page.locator('.docs-toc-link[data-active]')).toHaveCount(0); + + await page.evaluate(() => window.scrollTo({ top: 4000, behavior: 'instant' })); + await expect + .poll(() => + page + .locator('.docs-toc-link[data-active]') + .evaluateAll((els) => els.map((e) => e.getAttribute('href'))), + ) + .toEqual(['#connect-with-angular']); + + // ...and it follows the scroll rather than latching on the first match. + await page.evaluate(() => window.scrollTo({ top: 0, behavior: 'instant' })); + await expect.poll(() => page.locator('.docs-toc-link[data-active]').count()).toBe(0); + }); + + test('every rail link resolves to a heading in the article', async ({ page }) => { + await page.setViewportSize({ width: 1440, height: 900 }); + await page.goto(ARTICLE); + + const unresolved = await page.evaluate(() => + [...document.querySelectorAll('.docs-toc-link')] + .map((a) => (a as HTMLAnchorElement).getAttribute('href') ?? '') + .filter((href) => !document.getElementById(href.slice(1))), + ); + expect(unresolved).toEqual([]); + }); + + test('the library-neutral adapter page gets the same rail', async ({ page }) => { + await page.setViewportSize({ width: 1440, height: 900 }); + await page.goto('/docs/choosing-an-adapter'); + await expect(page.locator('.docs-toc')).toBeVisible(); + expect(await page.locator('.docs-toc-link').count()).toBeGreaterThan(3); + }); +}); + +test.describe('docs shell layout', () => { + test('the sticky rails hold through a full-page scroll', async ({ page }) => { + await page.setViewportSize({ width: 1440, height: 900 }); + await page.goto(ARTICLE); + + const navH = await page.evaluate(() => + parseFloat(getComputedStyle(document.documentElement).getPropertyValue('--nav-h')), + ); + const tops = async () => ({ + plane: await page.locator('.docs-control-plane').evaluate((el) => Math.round(el.getBoundingClientRect().top)), + toc: await page.locator('.docs-toc').evaluate((el) => Math.round(el.getBoundingClientRect().top)), + }); + + expect(await tops()).toEqual({ plane: navH, toc: navH }); + await page.evaluate(() => window.scrollTo({ top: 4000, behavior: 'instant' })); + expect(await tops()).toEqual({ plane: navH, toc: navH }); + await page.evaluate(() => window.scrollTo({ top: document.body.scrollHeight, behavior: 'instant' })); + expect(await tops()).toEqual({ plane: navH, toc: navH }); + }); + + test('breadcrumb, prose and prev/next share one right edge', async ({ page }) => { + // The header block used to stretch to the full content width while the + // article and the prev/next rail sat at max-w-3xl, so PageActions floated + // ~500px right of the column it belongs to. + await page.setViewportSize({ width: 1920, height: 1000 }); + await page.goto(ARTICLE); + + const right = (selector: string) => + page.locator(selector).first().evaluate((el) => Math.round(el.getBoundingClientRect().right)); + + const header = await right('.docs-page-header'); + const article = await right('article'); + const prevNext = await right('.docs-prevnext'); + + // The header and prev/next sit inside the article's horizontal padding. + expect(article - header).toBeLessThanOrEqual(48); + expect(header).toBe(prevNext); + }); +}); diff --git a/apps/website/e2e/nav-height.spec.ts b/apps/website/e2e/nav-height.spec.ts new file mode 100644 index 000000000..efb2eee54 --- /dev/null +++ b/apps/website/e2e/nav-height.spec.ts @@ -0,0 +1,81 @@ +import { test, expect } from '@playwright/test'; + +/** + * `--nav-h` (styles/chrome.css) is the single source of truth for every offset + * against the fixed nav: the docs shell's top padding, the sticky sidebar and + * TOC rails, the mobile drawer's `top`, and html's scroll-padding. + * + * Its value is measured from the rendered nav, not derived from the classes, so + * it silently drifts whenever Nav.tsx changes what it shows at a breakpoint — + * which is exactly how the 768–1023px band came to overshoot by 15px. jsdom + * cannot measure layout, so this is the only place the two can be compared. + * + * The tolerance is 1px, and deliberately not 0: the declared values round *up* + * off the measured height (58/66/81 against 57/65/81 in Chrome at dpr 1) so the + * offset always clears the nav rather than tucking content under it, and the + * sub-pixel height itself moves with font rendering. 1px is the rounding; the + * bug this guards against was fifteen. + */ +const STEPS = [ + { width: 375, note: 'phone — px-6 py-4' }, + { width: 767, note: 'phone — last px before md' }, + { width: 768, note: 'tablet — md padding, no lg link row' }, + { width: 1023, note: 'tablet — last px before lg' }, + { width: 1024, note: 'desktop — lg link row appears' }, + { width: 1440, note: 'desktop' }, +]; + +for (const step of STEPS) { + test(`--nav-h matches the rendered nav at ${step.width}px (${step.note})`, async ({ page }) => { + await page.setViewportSize({ width: step.width, height: 800 }); + await page.goto('/docs/langgraph/getting-started/introduction'); + + const nav = page.locator('nav').first(); + await expect(nav).toBeVisible(); + + const measured = await nav.evaluate((el) => el.getBoundingClientRect().height); + const variable = await page.evaluate(() => + parseFloat(getComputedStyle(document.documentElement).getPropertyValue('--nav-h')), + ); + + expect(variable).toBeGreaterThanOrEqual(measured); + expect(variable - measured).toBeLessThanOrEqual(1); + }); +} + +test('the docs column starts directly under the nav at a tablet width', async ({ page }) => { + // The 15px overshoot showed up here as dead space above the breadcrumb. + await page.setViewportSize({ width: 900, height: 800 }); + await page.goto('/docs/langgraph/getting-started/introduction'); + + const navBottom = await page + .locator('nav') + .first() + .evaluate((el) => el.getBoundingClientRect().bottom); + const shellTop = await page + .locator('.docs-shell-page') + .evaluate((el) => el.getBoundingClientRect().top + parseFloat(getComputedStyle(el).paddingTop)); + + expect(Math.abs(shellTop - navBottom)).toBeLessThanOrEqual(1); +}); + +test('the mobile drawer hangs flush off the nav on a tablet width', async ({ page }) => { + // The drawer is positioned at `top: calc(var(--nav-h) - 1px)`, so a wrong + // --nav-h shows up here as a visible gap between the nav and the panel. + await page.setViewportSize({ width: 900, height: 800 }); + await page.goto('/docs/langgraph/getting-started/introduction'); + + await page.locator('.nav-hamburger').click(); + const overlay = page.locator('.nav-mobile-overlay'); + await expect(overlay).toBeVisible(); + + const navBottom = await page + .locator('nav') + .first() + .evaluate((el) => el.getBoundingClientRect().bottom); + const overlayTop = await overlay.evaluate((el) => el.getBoundingClientRect().top); + + // Flush or overlapping the nav's bottom border — never a gap below it. + expect(overlayTop - navBottom).toBeLessThanOrEqual(0); + expect(overlayTop - navBottom).toBeGreaterThanOrEqual(-2); +}); diff --git a/apps/website/e2e/website.spec.ts b/apps/website/e2e/website.spec.ts index a57b6017e..12f0be82a 100644 --- a/apps/website/e2e/website.spec.ts +++ b/apps/website/e2e/website.spec.ts @@ -413,11 +413,37 @@ test('representative docs pages do not create page-level horizontal overflow', a for (const route of routes) { await page.goto(route); - const overflow = await page.evaluate(() => ( - document.documentElement.scrollWidth - document.documentElement.clientWidth - )); - expect(overflow, `${route} at ${width}px`).toBeLessThanOrEqual(1); + // NOT documentElement.scrollWidth. global.css clips the body + // (`overflow-x: clip`) precisely so overflow can never reach the layout + // viewport, which means that number is pinned to the viewport width and + // every assertion on it passed vacuously — confirmed by injecting a + // 2000px-wide element and watching it stay put. Ask the question the + // clip is hiding instead: does anything escape its own column? Content + // inside a horizontal scroller (code blocks, wide tables) is exempt — + // scrolling there is the intended containment. + const escaped = await page.evaluate(() => { + const column = document.querySelector('article') ?? document.querySelector('main'); + if (!column) return ['no column']; + const box = column.getBoundingClientRect(); + const inScroller = (el: Element) => { + let p = el.parentElement; + while (p && p !== column) { + const ox = getComputedStyle(p).overflowX; + if (ox === 'auto' || ox === 'scroll' || ox === 'hidden' || ox === 'clip') return true; + p = p.parentElement; + } + return false; + }; + return [...column.querySelectorAll('*')] + .filter((el) => { + const r = el.getBoundingClientRect(); + return r.width > 0 && r.right > box.right + 1 && !inScroller(el); + }) + .map((el) => `${el.tagName}.${String(el.className).slice(0, 40)}`); + }); + + expect(escaped, `${route} at ${width}px`).toEqual([]); } } }); diff --git a/apps/website/src/app/docs/[library]/[section]/[slug]/page.tsx b/apps/website/src/app/docs/[library]/[section]/[slug]/page.tsx index 5bc411490..eef5319c5 100644 --- a/apps/website/src/app/docs/[library]/[section]/[slug]/page.tsx +++ b/apps/website/src/app/docs/[library]/[section]/[slug]/page.tsx @@ -98,7 +98,12 @@ export default async function DocsPage({ params }: DocsRouteProps) { />
-
+ {/* Same measure as the article and the prev/next rail below it, so the + * whole column shares one right edge. Without md:max-w-3xl this + * block stretched to the full content width and PageActions floated + * ~500px right of the prose it belongs to (1272px vs 768px at + * 1920). */} +
} />
-
+
{section === 'api' && (() => { diff --git a/apps/website/src/app/docs/choosing-an-adapter/page.tsx b/apps/website/src/app/docs/choosing-an-adapter/page.tsx index 08f62b3d0..4291042c9 100644 --- a/apps/website/src/app/docs/choosing-an-adapter/page.tsx +++ b/apps/website/src/app/docs/choosing-an-adapter/page.tsx @@ -4,7 +4,9 @@ import { notFound } from 'next/navigation'; import { DocsControlPlane } from '../../../components/docs/DocsControlPlane'; import { DocsSearch } from '../../../components/docs/DocsSearch'; import { MdxRenderer } from '../../../components/docs/MdxRenderer'; +import { DocsTOC } from '../../../components/docs/DocsTOC'; import { createPageMetadata } from '../../../lib/site-metadata'; +import { extractHeadings } from '../../../lib/extract-headings'; import { stripFrontmatter } from '../../../lib/docs'; const PAGE_TITLE = 'Choosing an adapter'; @@ -32,6 +34,7 @@ export default function ChoosingAnAdapterPage() { if (!filePath) notFound(); const source = stripFrontmatter(fs.readFileSync(filePath, 'utf8')); + const headings = extractHeadings(source); return (
@@ -48,11 +51,16 @@ export default function ChoosingAnAdapterPage() {
+ {/* This page carries as many headings as any library page, so it gets + * the same rail. It stays library-neutral, so it takes no breadcrumb + * or page header — both are keyed to a library it deliberately + * has not picked. */} +
); diff --git a/apps/website/src/styles/chrome.css b/apps/website/src/styles/chrome.css index c30f16a4f..0ff3c22f6 100644 --- a/apps/website/src/styles/chrome.css +++ b/apps/website/src/styles/chrome.css @@ -10,16 +10,31 @@ * Fonts: unified onto the next/font vars — see the font note atop ui.css. */ -/* One nav height. The fixed nav measures 58px (px-6 py-4) and 81px at the md - * breakpoint (md:px-8 md:py-5, 1px border included). Everything that offsets - * against the nav — the docs shell's top padding, both sticky rails, the - * mobile overlay, and html's scroll-padding for anchor jumps — reads this one - * variable instead of hardcoding its own guess (the old hardcoded 80px left - * 22px of dead space on phones; anchors landed 81px under the nav). */ +/* One nav height. Everything that offsets against the nav — the docs shell's + * top padding, both sticky rails, the mobile overlay, and html's scroll-padding + * for anchor jumps — reads this one variable instead of hardcoding its own + * guess (the old hardcoded 80px left 22px of dead space on phones; anchors + * landed 81px under the nav). + * + * The nav has THREE heights, not two, because padding and content step at + * different breakpoints (Nav.tsx): the inner row is `px-6 py-4 md:px-8 md:py-5`, + * so padding grows at md (768px), but the tall `hidden lg:flex` link row only + * appears at lg (1024px). Between them the nav is 66px — a 25px logo in 40px of + * padding plus the 1px border. This variable used to jump straight to 81px at + * md, so from 768px to 1023px every offset overshot by 15px: dead space above + * the docs column, and the mobile drawer (top: nav-h - 1px) hung 14px below the + * nav it is supposed to be attached to. These are measured, not derived, so + * only a real browser can hold them honest: e2e/nav-height.spec.ts asserts + * nav.height === --nav-h at each of the three steps. */ :root { --nav-h: 58px; } @media (min-width: 768px) { + :root { + --nav-h: 66px; + } +} +@media (min-width: 1024px) { :root { --nav-h: 81px; } diff --git a/apps/website/src/styles/docs.css b/apps/website/src/styles/docs.css index 7da71bf0b..95e1875f7 100644 --- a/apps/website/src/styles/docs.css +++ b/apps/website/src/styles/docs.css @@ -978,9 +978,8 @@ font-weight: 500; } -/* DocsBreadcrumb — verbatim, including the inconsistent li/separator - * typography (crumb/sep were separate style-variable shapes; a later - * polish pass reconciles them, not this migration). */ +/* DocsBreadcrumb — the crumb/separator typography the migration left + * inconsistent is reconciled below, on the list rather than the links. */ .docs-crumb-nav { margin-bottom: 16px; } diff --git a/apps/website/src/styles/pages.css b/apps/website/src/styles/pages.css index ed455a865..b661a89b9 100644 --- a/apps/website/src/styles/pages.css +++ b/apps/website/src/styles/pages.css @@ -249,10 +249,21 @@ max-width: 760px; } -/* Docs shell — app/docs/[library]/[section]/[slug]/page.tsx - * paddingTop: 80 and the overflow-x-hidden utility are known defects - * (nav-height coupling) migrated VERBATIM here; a later project fixes them, - * not this migration. */ +/* Docs shell — shared by all three /docs routes. + * + * Both defects the migration flagged here are now closed. The hardcoded + * `paddingTop: 80` became `var(--nav-h)`, and the nav-height coupling behind it + * was fixed at the source in chrome.css (the variable was missing its + * 768–1023px step). `min-height: 100vh` and this padding do not stack, because + * global.css sets `box-sizing: border-box` on everything. + * + * The article's `overflow-x-hidden` utility is gone rather than moved here. It + * was redundant — global.css already clips the body — and it was the same + * mistake that rule's comment warns about: `overflow-x: hidden` computes + * `overflow-y: auto`, so it made every docs article a scroll container. All 123 + * docs URLs were measured at 375px with it removed; none overflow. Wide code + * blocks and tables scroll inside their own `overflow-x: auto` containers, + * which is where the containment belongs. */ .docs-shell-page { background: var(--color-canvas); padding-top: var(--nav-h);