Fix App Router scroll padding visibility - #96308
Merged
Merged
Conversation
Contributor
Tests PassedCommit: 4748515 |
Contributor
Stats from current PR🟢 3 improvements
📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📝 Changed Files (2 files)Files with changes:
View diffspages-api-tu..time.prod.jsDiff too large to display pages-turbo...time.prod.jsDiff too large to display 📎 Tarball URLCommit: 4748515 |
DavidIlie
force-pushed
the
codex/fix-layout-router-scroll-padding-top-pr
branch
from
July 30, 2026 18:01
e763145 to
4748515
Compare
DavidIlie
marked this pull request as ready for review
July 30, 2026 18:44
ztanner
approved these changes
Aug 3, 2026
m-kawafuji
pushed a commit
to m-kawafuji/next.js
that referenced
this pull request
Aug 8, 2026
## Summary - Treat the root `scroll-padding-top` as the lower boundary of the usable viewport during App Router navigations, so content obscured by a sticky header is not incorrectly considered visible. - Preserve the merged Fragment scroll-ownership state machine: empty Fragments remain unavailable targets, while real targets are classified against the padding-aware viewport. - Resolve pixel and percentage values and use the same visibility rule in both the legacy element handler and the Fragment-ref handler. - Add regression coverage showing that Next scrolls when the destination is hidden inside the padding boundary, while preserving the current scroll position when the destination is genuinely visible below it. ## Performance The root computed style is resolved lazily, only after the candidate produces client rects. Empty Fragments and hash navigations do not perform this lookup. For a real route-scroll target, the resolved value is cached locally and reused by the second geometry check after `scrollTop = 0`. This does not add work to scroll events or every render, and keeping the value local allows responsive CSS, root classes, and custom properties to change between navigations. In a 50-iteration dirty-style Chromium probe, both the existing and updated paths reported 50 style recalculations and 50 layouts, indicating that the lookup reused the style/layout update already required by the handler in that test. ## Related work vercel#96342 is now merged into `canary` and handles empty Fragment scroll ownership. This PR composes with its `NoClientRects` / `InViewport` / `OutOfViewport` state machine by changing only the visible-region boundary for real targets. ## Verification - `HEADLESS=true pnpm test-dev-turbo test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts -t "scroll padding"` (3/3) - `HEADLESS=true __NEXT_EXPERIMENTAL_APP_NEW_SCROLL_HANDLER=false pnpm test-dev-turbo test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts -t "scroll padding"` (3/3) - `HEADLESS=true pnpm test-dev-turbo test/e2e/app-dir/parallel-routes-scroll-owner/parallel-routes-scroll-owner.test.ts` (8/8) - `HEADLESS=true pnpm test-dev-webpack test/e2e/app-dir/parallel-routes-scroll-owner/parallel-routes-scroll-owner.test.ts` (8/8) - `HEADLESS=true pnpm test-dev-turbo test/e2e/app-dir/navigation-focus/navigation-focus.test.ts` (5/5) - Manually compared the same sticky-header reproduction before and after the change. <!-- NEXT_JS_LLM -->
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
2 participants
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.
Summary
scroll-padding-topas the lower boundary of the usable viewport during App Router navigations, so content obscured by a sticky header is not incorrectly considered visible.Performance
The root computed style is resolved lazily, only after the candidate produces client rects. Empty Fragments and hash navigations do not perform this lookup. For a real route-scroll target, the resolved value is cached locally and reused by the second geometry check after
scrollTop = 0.This does not add work to scroll events or every render, and keeping the value local allows responsive CSS, root classes, and custom properties to change between navigations. In a 50-iteration dirty-style Chromium probe, both the existing and updated paths reported 50 style recalculations and 50 layouts, indicating that the lookup reused the style/layout update already required by the handler in that test.
Related work
#96342 is now merged into
canaryand handles empty Fragment scroll ownership. This PR composes with itsNoClientRects/InViewport/OutOfViewportstate machine by changing only the visible-region boundary for real targets.Verification
HEADLESS=true pnpm test-dev-turbo test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts -t "scroll padding"(3/3)HEADLESS=true __NEXT_EXPERIMENTAL_APP_NEW_SCROLL_HANDLER=false pnpm test-dev-turbo test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts -t "scroll padding"(3/3)HEADLESS=true pnpm test-dev-turbo test/e2e/app-dir/parallel-routes-scroll-owner/parallel-routes-scroll-owner.test.ts(8/8)HEADLESS=true pnpm test-dev-webpack test/e2e/app-dir/parallel-routes-scroll-owner/parallel-routes-scroll-owner.test.ts(8/8)HEADLESS=true pnpm test-dev-turbo test/e2e/app-dir/navigation-focus/navigation-focus.test.ts(5/5)