refactor(website): render both nav panel surfaces from one body - #1084
Merged
blove merged 1 commit intoSep 9, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The desktop hover panel and the mobile drill-in stack each wrote the same shape by hand: map panel.columns to a column wrapper, map column.items through NavPanelItem, then render panel.footer with nav-panel-footer / nav-panel-footer-lead plus one more NavPanelItem. Individual items already funnelled through the shared NavPanelItem, which is what has kept the analytics ids from drifting, but the column and footer shape around them was duplicated and free to diverge. NavPanelBody now renders columns + footer once, parameterised by the two things that genuinely differ: the wrapper class names (the layouts are not the same, so desktop keeps nav-panel-cols / nav-panel-col and mobile keeps nav-mobile-panel / nav-mobile-group) and the optional onNavigate the mobile drawer uses to close itself. The caller still owns the outermost element, because that is the other real difference: desktop needs the panel id and data-columns, mobile needs neither. Desktop passes columnsClassName, mobile omits it, so the mobile stack keeps its columns as direct children and the scoped .nav-mobile-panel .nav-panel-footer override still matches. NavPanelItem and trackNavItem move into the new module with it. NavMobile had been importing NavPanelItem from NavDesktop, which pointed the dependency the wrong way between two sibling surfaces; both now import from a shared leaf and neither imports the other. Pure refactor — no rendered output changes. Verified by dumping the outerHTML of all three panels on both surfaces before and after: identical byte for byte, down to the useId-generated panel id. nx test website 1462 passed, nx build website green, and the nav e2e suites (nav-panels, nav-drawer, nav-surface, nav-height) 32 passed in a real browser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
blove
force-pushed
the
blove/nav-panel-shared-body
branch
from
September 9, 2026 03:31
05850d7 to
52eb171
Compare
Contributor
blove
added a commit
that referenced
this pull request
Sep 9, 2026
… a transparent hero surface, and a mobile drill-in stack (#1083) * docs(specs): navbar redesign design Single-row nav with hover-opened panels (Libraries / Docs / Solutions / Pricing), transparent-on-hero bar surface, a condensed docs height, and a mobile drill-in stack that replaces the Site/Docs tab strip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(specs): correct the navbar spec's copy-scan and nav-height risks public-copy.spec.ts has a second `renderedCopyFiles` AST scan covering every non-spec .ts/.tsx/.mjs under src/, so nav-config copy is guarded already; the spec had claimed only content/** was scanned. nav-height.spec.ts navigates every width step to /docs, so a docs-specific height moves those steps rather than merely adding cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): navbar redesign implementation plan Nine TDD tasks: IA as data, surface hook, two verbatim extractions, desktop panels, transparent bar, condensed docs height, mobile drill-in stack, and the deletion pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(website): describe the navbar IA as data Lands the four-trigger nav structure (Libraries, Docs, Solutions, Pricing) as a standalone data module with route-resolution and copy-quality tests. Nothing consumes it yet — Nav.tsx is unchanged; later tasks render both the desktop panels and the mobile drill-in from this same module so the two surfaces cannot drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): teach the nav-config test about the dynamic solutions route Task 1's test only resolved /docs/* dynamically, so the three /solutions/:slug deep links looked unresolvable and invited repointing them at the hub — which would have collapsed three distinct panel destinations into one. Adds solutionsHrefResolves against getAllSolutionSlugs, plus a mutation check so neither dynamic-route resolver can pass vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(website): deep-link the nav's solutions items and validate the dynamic route Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): note that nx swallows the vitest path filter Every per-task command runs the full website suite; the trailing path is ignored. Records the `cd apps/website && npx vitest run <path>` form for iterating. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): record the docs-structured-data timeout seen under full-suite load Passes in isolation and is untouched by this branch, so Task 9's full run will likely hit it. Noted rather than pre-excused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(website): derive the nav bar surface from route and scroll Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): defer the nav surface's optimistic-atTop decision to Task 5 jsdom cannot measure layout, so the flash trade-off can only be judged in a real browser. Records both failing cases, the getBoundingClientRect seed that resolves both, and that StrictMode's dev double-invoke means a green dev-server run is not sufficient evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(website): assert useNavSurface observes the sentinel it hands back Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(website): extract the desktop nav row into its own component Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): warn that some website specs are cwd-sensitive cockpit-retirement.spec.ts walks directories relative to process.cwd(), so it fails 5/9 under `npx vitest` from apps/website and passes under nx. Verified directly. A filtered local run is fine for the one spec being iterated on; suite-level claims must come from `npx nx test website`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): fix the nav CTA selector — Button emits no class Tasks 5 and 6 targeted `.btn[data-variant='primary']`, which matches nothing: Button stamps data-ui/data-variant/data-size and passes through only a caller- supplied className, which the nav's CTA does not supply. A CSS selector that matches nothing fails silently, so both the transparent-state CTA inversion and the docs CTA demotion would have looked implemented and done nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(website): render the desktop nav as four triggers with hover panels Replaces the flat link row and the hand-rolled Demo dropdown with the four triggers from NAV_TRIGGERS: three panel disclosures (Libraries, Docs, Solutions) and Pricing as a plain link. NavPanelItem and trackNavItem are exported so Task 8's mobile levels render the same items through the same component. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(website): lay the libraries panel across its own width Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): correct the e2e invocation and warn about orphaned dev servers A bare positional spec path fails on this Nx/Playwright executor with `unknown option '--_=…'`; it needs --testFiles=. Also records that an orphaned next-server can hold the web-server port, and that a stale one will happily serve an old bundle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): require nx build per task — test and lint do not typecheck Task 4 shipped a broken production build behind 1411 passing tests and a clean lint: vitest strips types and this eslint config is not type-aware, so a template literal widening to `string` against the CtaId union went unseen until review ran the build. Annotates the plan's own trackNavItem, and sets aria-controls only while the panel is actually rendered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(website): keep the nav's analytics id assignable to CtaId Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(specs): drop the shared morphing nav panel, on measurement All three panels are the same width by construction, and Docs and Solutions are the same height, so four of six transitions have nothing to animate. The morph buys one 67px tween and costs an inert/allow-discrete state machine on top of the a11y wiring, plus rewriting every .nav-panel locator. A 140ms entrance animation addresses the real complaint instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(specs): record the nav panel's gutter misalignment as a known gap The shell insets against the inner row's padding box, so the bar's px-8 gutter lands inside the panel: first item at x=24 against a logo at x=32. Polish, not a defect — recorded rather than silently dropped with the content-width claim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(website): put nav panels in tab order and make the panel grid generic - Render each panel shell in a Fragment right after its own trigger so tab order matches visual order; delete the trailing filter().map() block that put every panel's content after Docs, Solutions, Pricing, GitHub, and Talk to Us. - Make .nav-panel-cols generic (grid-auto-flow: column) so a fourth column in nav-config.ts degrades gracefully instead of stacking into a 949px wall; the single-column Libraries layout is now equally generic instead of hardcoding repeat(4, 1fr). - Delete 45 lines of dead .nav-demo-* CSS left over from the removed DemoDropdown component. - Give .nav-panel an entrance animation so it doesn't snap in while the caret glides, with a prefers-reduced-motion override. - Comment .nav-desktop's position: static so it isn't mistaken for a no-op and deleted later. - Add e2e coverage for the tab-order fix and for the hover-open grace period/dead-zone survival, the latter using a stepped mouse.move because a plain .hover() jumps straight to the target and never exercises the dead zone. - Stabilize the pre-existing "lays out side by side" geometry test against the new entrance animation by waiting for it to finish before sampling bounding boxes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): record two Playwright traps found in Task 4 .hover() teleports to the target centre, so any test whose subject is the path between two elements is vacuous when written with it — Task 4's hover-grace test passed with and without the behaviour it guarded, caught only because the mutation proof failed to fail. And an entrance animation makes sequential boundingBox sampling flaky; await getAnimations().finished, never a fixed sleep. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(website): render the nav transparent over the hero until scrolled The bar now carries two surfaces: transparent at rest on a HERO_ROUTES page, solid everywhere else and once scrolled. The drop shadow goes in both. Wiring useNavSurface into Nav.tsx also settled the design question its comment deferred. Measured per-frame in Chrome, the unconditional optimistic `setAtTop(true)` applied `transparent` for a frame whenever a hero route mounted already scrolled — a #hash deep link, or back-navigation with scroll restoration. Seeding from the sentinel's getBoundingClientRect() removes those frames and leaves the common fresh-load-at-scroll-0 case unchanged. The unit spec has to state the scroll position each case is about now that the hook reads layout, because jsdom reports every rect as zero. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(specs): record the nav's first-load solid flash and pin the contrast figures SSR ships `solid` because the first render has no layout to read, so the flip waits on hydration — ~250ms against next dev. Fixing it by server-rendering transparent trades the common case for the already-scrolled one and risks a hydration mismatch; judge it on the deployed preview, which is already a required gate. Contrast now computed rather than asserted: 8.33:1 and 15.37:1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(specs): correct the mobile section — the drawer is hidden on docs detail pages Only /docs/[library]/[section]/[slug] mounts WebsiteWorkspace, and docs.css hides the hamburger and overlay below 1024px on workspace pages; lg:hidden covers the rest. So the site drawer is unreachable at every width on detail docs — existing e2e already asserts it. The drill-in decision stands, but the pre-push reaches only the two library-neutral docs routes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(website): condense the nav to one height inside docs Inside /docs the bar is chrome around the reader's content, not the page's headline, so it condenses to a flat 58px at every breakpoint and the "Talk to Us" CTA demotes from a fill button to a text link. `--nav-h` becomes route-dependent. The marketing ladder (58/66/81) stays; `:root:has(.nav-bar[data-route='docs'])` overrides it, which wins over the media queries regardless of source order because `:has()` takes the specificity of its most specific argument. Nav.tsx stamps the route on the bar from the `isDocsPage` it already computes, server-rendered, so the value is right on first paint. Flatness comes from the CTA demotion, not the padding alone: Button's size=md carries a hard `height: 40px`, which is what pushes the marketing bar to 81px once the lg link row appears. With `height: auto` the 25px logo is the tallest thing in the row at every width, so a flat 16px of padding measures 58px from 375px up. e2e/nav-height.spec.ts now measures both ladders — its width steps moved to `/` so they stop measuring the docs bar against marketing values — and adds a case pinning that the docs nav does not grow with the breakpoint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(website): give the demoted docs CTA a real hit area and pin it Follow-ups from Task 6 of the navbar redesign (commit 7f649cde1): - The docs CTA demotion (fill button -> text link) left `height: auto` with `padding-inline: 0` and `line-height: 1`, collapsing the hit area to ~16px tall. Padded the box and pulled it back with an equal negative margin, the same trick `.nav-hamburger` already uses, so the clickable area grows without moving the visual layout or the flex row's height that --nav-h depends on. - Nothing asserted that the docs CTA is actually a text link rather than a fill, so a future change to Button's primary-variant styling could silently restyle it. Added a test to nav-surface.spec.ts pinning both halves of the contrast: marketing keeps a filled CTA, docs demotes it to a transparent background with the navy accent color. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(website): pin the declared nav heights per surface Every nav-height.spec.ts step only checked self-consistency (--nav-h tracks the rendered nav), which is blind to the marketing ladder and the rendered nav drifting together to the wrong value -- e.g. the ladder silently flattening to match docs. Add absolute assertions that pin the declared --nav-h per surface/width (marketing: 58/66/81, docs: 58 flat) alongside the existing tolerance check, plus the inverse of "the docs nav does not grow with the breakpoint": marketing does grow. Also amend the stale CTA-demotion comment in chrome.css -- a later commit added the padding-block/margin-block hit-area trick, which is what actually pins the CTA's margin box at 16px now; height: auto is belt-and-braces on top of it, not the sole reason for the flat 58px. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(website): say which kind of nav-height failure you are looking at The tolerance checks and the ladder pin fail with identical generic output, so a red CI run does not tell you whether --nav-h drifted from what rendered or whether the design changed. Each assertion now carries a message saying which. Behaviour-neutral: messages only. Typechecked with the workspace compiler and linted, but NOT run — the e2e suite starts a cockpit runtime server on the hardcoded port 4300, which another worktree is currently holding. Exercised by the next full e2e run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): warn Task 7 not to move the drawer inside <nav> Task 6 added `.nav-bar[data-route='docs'] > div` padding. The overlay is a sibling of <nav> today, so it is unaffected; moving it inside would silently give it docs-only padding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(website): extract the mobile nav drawer into its own component Task 7 of the navbar redesign plan: a pure move, no behavior changes. NavMobile.tsx now owns the hamburger trigger, the focus trap, scroll lock, inert management, desktop-breakpoint auto-close, focus restore via requestAnimationFrame, and the Cmd+K search handoff — verbatim from Nav.tsx, with `open`/`mobileTab` state and refs localized to the new component. Nav.tsx keeps the <nav> shell, path parsing, navRef, useNavSurface, and NavDesktop, and passes down isDocsPage, docsLibrary, activeSection, activeSlug, and navRef. The one deliberate addition: since NavMobile is invoked once from inside nav's flex row (where the hamburger button must stay, for flex layout), the overlay dialog is rendered via createPortal to document.body so it remains a sibling of <nav> in the DOM rather than becoming a second direct <div> child of <nav> — preserving both the original stacking-context fix and immunity from the Task 6 `.nav-bar[data-route='docs'] > div` padding rule. No props beyond the given NavMobileProps interface were needed for this. Verified: nx test website -- --run -> 140 files / 1413 tests passed, identical to baseline. nx build website exits 0. nx lint website -> 0 errors, 65 pre-existing warnings (no new ones). nx e2e website --testFiles=e2e/nav-height.spec.ts -> 16/16 passed (port 4300 was free). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): record cross-worktree e2e port contention and the BASE_URL escape A neighbouring worktree started a server mid-run during Task 7, producing 15 failures that were all ERR_CONNECTION_REFUSED with zero assertion failures. Records how to tell contention from a real failure, that another worktree's server must never be killed, and that BASE_URL bypasses the config's own webServers entirely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(website): replace the mobile tab strip with a drill-in stack The drawer's Site/Docs tab strip is gone. Depth carries what it said: the root level lists the four nav triggers as rows, tapping one pushes to that trigger's panel with a back row above it, and on a docs route the drawer opens pre-pushed to the docs tree. Escape retraces the way in — it pops a pushed level, and dismisses the drawer only from the level it opened at, so Escape on a docs route still closes the drawer in one press rather than stranding it at the root list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): widen Task 9's dead-CSS sweep to the audited orphan set Thirteen nav-* classes in chrome.css are referenced from no component, no other stylesheet and no dynamic className. Three were orphaned by Task 8's drill-in rewrite; the rest predate this branch. Records the list, requires re-verification before deletion, and asks that the pre-existing ones be called out separately so the diff is not read as redesign fallout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(website): state the drawer's Escape rule and restore its footer lead Escape's condition compared level against a hardcoded docs special case; express it instead as a comparison against initialLevel(isDocsPage), the level the drawer actually opened at, so the rule reads as what it is and survives any future route that pre-pushes to a different level. The mobile Libraries panel footer dropped the "Not sure which one?" lead that desktop renders before the Choosing-an-adapter link, a copy regression against desktop. Render it via the same nav-panel-footer / nav-panel-footer-lead classes, with a mobile-scoped CSS override to stack the lead above the link instead of wrapping onto its row at narrow width. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): inverting a condition does not prove it equivalent A "clarity" refactor of the drawer's Escape handler dropped a guard and made Escape a dead key at the root of a docs-route drawer. The equivalence proof (invert the condition, watch both Escape tests fail) passed, because those two tests covered two of five reachable states. Records the state list, and that nx is the only unit lane that actually runs Nav.spec.tsx. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(website): stop Escape becoming a dead key at the drawer root Restore the level.kind === 'panel' guard on the Escape handler's pop branch. Without it, a root reached via Back on a docs route (never pushed from, so not sameLevel as the docs opening level) hit the pop branch and called setLevel(rootLevel) on a level that was already root — a no-op that left closeMobileMenu() unreached. Escape became permanently dead until the reader used the Close button. Adds two regression tests covering the previously-unreached states: Escape from a root reached via Back on a docs route, and Escape popping a level pushed from that root. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(website): delete the superseded nav exports and dead drawer CSS The rebuilt navbar left three kinds of dead weight behind. `NavDesktop.tsx` still exported `links` (the old flat link array) and `trackNavLink` (the old per-label analytics helper). Both were kept alive only for the mobile drawer, which now reads `NAV_TRIGGERS` and tracks through `trackNavItem`. Nothing outside the file referenced either. The dead CSS in `chrome.css` falls into two groups, and the split matters when reading this diff: Orphaned by this branch's drill-in rewrite (Task 8) — `.nav-mtabs`, `.nav-mtab`, `.nav-mtab[data-active]`, `.nav-mobile-site-link`. Already dead before this branch started, leftovers from an older drawer — `.nav-msubtabs-wrap`, `.nav-msubtabs`, `.nav-msubtab`, `.nav-msubtab[data-active]`, `.nav-mobile-content-list`, `.nav-mobile-item`, `.nav-mobile-item[data-active]`, `.nav-mobile-item--strong`, `.nav-mobile-demo-link`, `.nav-mobile-section-toggle`, `.nav-mobile-chevron`, `.nav-mobile-chevron[data-open]`, `.nav-mobile-search`, `.nav-mobile-search:focus-visible`. Removing them is in scope — same drawer's leftovers — but they are not fallout from the redesign. Every class was re-verified unreferenced across `src/`, `e2e/`, the rest of `src/styles/`, `src/app/global.css` and the style contracts before deletion; none is built dynamically, and the drawer's live classes (`.nav-mobile-list`, `-overlay`, `-row`, `-back`, `-panel`, `-group`, `-github-link`, `-cta`, `-dialog-close`) are untouched. Finally, drill-in focus is now symmetric. Pushing a level focuses its "Back to menu" row through an explicit ref; popping fell through to `focusable()[0]`, which is not a designed destination. Both pop paths — the Back button and Escape — now route through `popToRoot`, which records the originating trigger id so the per-level focus effect can land on that row once root has re-rendered. The restore reads a ref map rather than the focus-trap query on purpose: jsdom's multi-clause `querySelectorAll` groups by clause instead of returning document order, so a test written against `focusable()[0]` would assert the Pricing anchor where a browser gives the row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plans): write down the three deferrals that were only in reviewers' heads The panel-shape duplication, the NavMobile -> NavDesktop import direction, and MobileLevel.id being an untyped string were all deliberate calls, but none was recorded. An undocumented deferral reads as an oversight later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(website): open the mobile drawer in a real browser Nav.spec.tsx covers the drill-in stack in jsdom, but jsdom has no CSS or layout engine, so nothing catches the overlay's top offset drifting, an overlay host swallowing row clicks, an lg:hidden regression exposing the hamburger at desktop, or a scroll lock that never engages/releases. Gives the mobile drawer the same real-browser treatment nav-panels.spec.ts gives the desktop panels. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(website): render both nav panel surfaces from one body (#1084) The desktop hover panel and the mobile drill-in stack each wrote the same shape by hand: map panel.columns to a column wrapper, map column.items through NavPanelItem, then render panel.footer with nav-panel-footer / nav-panel-footer-lead plus one more NavPanelItem. Individual items already funnelled through the shared NavPanelItem, which is what has kept the analytics ids from drifting, but the column and footer shape around them was duplicated and free to diverge. NavPanelBody now renders columns + footer once, parameterised by the two things that genuinely differ: the wrapper class names (the layouts are not the same, so desktop keeps nav-panel-cols / nav-panel-col and mobile keeps nav-mobile-panel / nav-mobile-group) and the optional onNavigate the mobile drawer uses to close itself. The caller still owns the outermost element, because that is the other real difference: desktop needs the panel id and data-columns, mobile needs neither. Desktop passes columnsClassName, mobile omits it, so the mobile stack keeps its columns as direct children and the scoped .nav-mobile-panel .nav-panel-footer override still matches. NavPanelItem and trackNavItem move into the new module with it. NavMobile had been importing NavPanelItem from NavDesktop, which pointed the dependency the wrong way between two sibling surfaces; both now import from a shared leaf and neither imports the other. Pure refactor — no rendered output changes. Verified by dumping the outerHTML of all three panels on both surfaces before and after: identical byte for byte, down to the useId-generated panel id. nx test website 1462 passed, nx build website green, and the nav e2e suites (nav-panels, nav-drawer, nav-surface, nav-height) 32 passed in a real browser. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1083 — base is
blove/navbar-design-brainstorm-2b6a7d, so this diff is the single commit on top of that branch.The duplication
NavDesktop'sPanelandNavMobile's inline panel JSX each wrote the same shape by hand: mappanel.columns→ a column wrapper →column.items.map(NavPanelItem), then conditionally renderpanel.footerwith the samenav-panel-footer/nav-panel-footer-leadclasses plus one moreNavPanelItem. Individual items already funnelled through the sharedNavPanelItem— which is what has kept the analytics ids from drifting — but the column and footer shape around them was written twice and free to diverge.The shape
NavPanelBodyrenders columns + footer once, parameterised by the two things that genuinely differ:nav-panel-cols/nav-panel-col, mobile keepsnav-mobile-panel/nav-mobile-grouponNavigate, which the mobile drawer uses to close itself and desktop omitsThe caller still owns the outermost element, because that is the other real difference: desktop needs the panel
idanddata-columns, mobile needs neither. Desktop passescolumnsClassName; mobile omits it, so the mobile stack keeps its columns as direct children of.nav-mobile-paneland the scoped.nav-mobile-panel .nav-panel-footeroverride that stacks the footer lead above the link at narrow widths still matches.NavPanelItemandtrackNavItemmove into the new module with it.NavMobilehad been importingNavPanelItemfromNavDesktop, pointing the dependency the wrong way between two sibling surfaces; both now import from a shared leaf and neither imports the other.Rendered output is unchanged
Pure refactor. Rather than trusting the unit suite — jsdom has no layout engine, and the panel geometry bugs on this branch were all invisible to it — the
outerHTMLof all three panels was dumped on both surfaces before and after (6 files, 612 lines).diff -rreports zero differences, down to theuseId-generatedid="_r_0_-libraries".No
cta_idchanges:trackNavItemmoved verbatim and both call sites pass the samesurface, so desktop still emitsnav_<ctaId>and mobilemobile_nav_<ctaId>.Verification
nx test website -- --runNav.spec.tsxcollected, 28 tests)nx build websitenx e2e website -- --testFiles=e2e/nav-panels.spec.tsnav-drawer+nav-surface+nav-heighte2enx lint websitenav-panels.spec.tsis desktop-only and this touches the mobile surface too, so the mobile drawer suites were run as well rather than left unverified in a real browser.🤖 Generated with Claude Code