Skip to content

fix: desktop nav menu scrolling#1597

Merged
h4rkl merged 1 commit into
mainfrom
fix/nav-scroll
Jun 23, 2026
Merged

fix: desktop nav menu scrolling#1597
h4rkl merged 1 commit into
mainfrom
fix/nav-scroll

Conversation

@h4rkl

@h4rkl h4rkl commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • cap desktop nav dropdown height to the available viewport below the sticky header
  • make desktop dropdowns scrollable with contained overscroll
  • remove stale per-section 90vh overrides so all top-level nav menus use the shared behavior

Verification

  • Playwright check at 1280x360 confirmed all desktop nav dropdowns stay within the viewport and scroll when content overflows
  • pnpm --filter @solana-com/ui-chrome lint
  • pnpm --filter @solana-com/ui-chrome check-types

@h4rkl h4rkl requested review from ZYJLiu and jacobcreech as code owners June 23, 2026 01:18
@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
solana-com Ready Ready Preview, Comment Jun 23, 2026 1:22am
solana-com-accelerate Ready Ready Preview, Comment Jun 23, 2026 1:22am
solana-com-breakpoint-2 Ready Ready Preview, Comment Jun 23, 2026 1:22am
solana-com-docs Ready Ready Preview, Comment Jun 23, 2026 1:22am
solana-com-media Ready Ready Preview, Comment Jun 23, 2026 1:22am
templates Ready Ready Preview, Comment Jun 23, 2026 1:22am

Request Review

@greptile-apps

greptile-apps Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes desktop nav menu overflow by replacing two per-section 90vh height overrides with a single shared calc(100dvh-5rem) max-height applied to the base NavigationMenuContent component, along with overflow-y-auto and overscroll-contain to make all desktop dropdowns scrollable and self-contained.

  • nav-menu.tsx: Adds xl:max-h-[calc(100dvh-5rem)] xl:overflow-y-auto xl:overscroll-contain to the shared content class so every nav dropdown is capped to the available viewport below the header, scrollable, and won't chain-scroll the page.
  • header-section-metadata.ts: Removes the stale contentClassName values from "enterprise" and "products" sections; the optional field remains available for future per-section overrides if needed.

Confidence Score: 4/5

Safe to merge; the change is a targeted CSS-only fix that was Playwright-validated and does not touch any logic or data paths.

The two changed files only affect Tailwind class strings — no logic, routing, or data-fetching is involved. The dvh unit and overscroll-contain are well-supported on the targeted desktop breakpoint. The one open question is whether the 5rem header-height assumption will stay accurate over time; today it works, but it is not tied to any shared constant.

nav-menu.tsx — the 5rem magic number in calc(100dvh-5rem) deserves a second look if the header layout ever changes.

Important Files Changed

Filename Overview
packages/ui-chrome/src/nav-menu.tsx Adds xl:max-h-[calc(100dvh-5rem)] xl:overflow-y-auto xl:overscroll-contain to the shared NavigationMenuContent base class, centralising viewport-constrained scrolling for all desktop dropdowns; the 5rem offset is an implicit magic number for the header height.
packages/ui-chrome/src/header-section-metadata.ts Removes the per-section contentClassName overrides (xl:overflow-y-auto xl:max-h-[90vh]) from the enterprise and products entries; the field remains optional in the type so existing callers are unaffected.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User hovers nav trigger] --> B[NavigationMenuContent renders]
    B --> C{xl breakpoint?}
    C -- No --> D[Mobile dropdown no height constraint]
    C -- Yes --> E[Desktop styles applied max-h calc 100dvh minus 5rem overflow-y auto overscroll contain]
    E --> F{Content taller than max-height?}
    F -- No --> G[Renders at natural height]
    F -- Yes --> H[Dropdown scrolls internally Page scroll not triggered]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[User hovers nav trigger] --> B[NavigationMenuContent renders]
    B --> C{xl breakpoint?}
    C -- No --> D[Mobile dropdown no height constraint]
    C -- Yes --> E[Desktop styles applied max-h calc 100dvh minus 5rem overflow-y auto overscroll contain]
    E --> F{Content taller than max-height?}
    F -- No --> G[Renders at natural height]
    F -- Yes --> H[Dropdown scrolls internally Page scroll not triggered]
Loading

Reviews (1): Last reviewed commit: "fix: allow desktop nav menus to scroll" | Re-trigger Greptile

Comment thread packages/ui-chrome/src/nav-menu.tsx
@h4rkl h4rkl changed the title Fix desktop nav menu scrolling fix: desktop nav menu scrolling Jun 23, 2026
@h4rkl h4rkl merged commit daa9869 into main Jun 23, 2026
23 checks passed
@h4rkl h4rkl deleted the fix/nav-scroll branch June 23, 2026 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants