cloudflare / cloudflare/vinext
App Router navigation scroll: respect root `scroll-padding-top` as viewport boundary (don't skip scroll when target is under sticky header)
- Dominant language
- TypeScript
- Stars
- 8.8k
- Forks
- 406
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 120
Description
## Next.js Change
**Commit:** [`da90782`](https://github.com/vercel/next.js/commit/da90782311ff578bb950f7ca6d98e7e9fbed5f41)
**PR:** [#96308](https://github.com/vercel/next.js/pull/96308)
## What changed
Fixes App Router scroll-on-navigation so that content hidden behind a sticky header (via root `scroll-padding-top`) is no longer treated as "already visible." Previously, if a navigation target's top edge fell within the `scroll-padding-top` band, Next.js considered it in-viewport and skipped scrolling, leaving the target obscured under the sticky header.
### Mechanism (from the diff)
`layout-router.tsx` now treats the root element's `scroll-padding-top` as the lower boundary of the usable viewport. A new helper resolves the padding value:
```ts
// packages/next/src/client/components/layout-router.tsx
function getScrollPaddingTopInPixels(
htmlElement: HTMLElement,
viewportHeight: number
): number {
const scrollPaddingTop = getComputedStyle(htmlElement).scrollPaddingTop
const value = Number.parseFloat(scrollPaddingTop)
if (!Number.isFinite(value) || value < 0) return 0
if (scrollPaddingTop.endsWith('px')) return value
if (scrollPaddingTop.endsWith('%')) return (value / 100) * viewportHeight
return 0
}
```
The viewport visibility check changed from `elementTop >= 0` to `elementTop >= getScrollPaddingTop()`:
```ts
return elementTop >= getScrollPaddingTop() && elementTop <= viewportHeight
? ScrollTargetState.InViewport
: ScrollTargetState.OutOfViewport
```
Key details:
- **Lazy + cached resolution.** `scroll-padding-top` is read via a `getScrollPaddingTop()` closure that computes on first call and caches locally. Empty Fragments and hash navigations do not trigger the computed-style read. The cached value is reused for the second geometry check (after `scrollTop = 0`).
- **Percentages vs pixels.** Computed lengths serialize as `px`; percentages remain relative to the scrollport, so `%` values are resolved against `viewportHeight`.
- **Both handlers updated.** The change applies to both the legacy element handler (`InnerScrollAndFocusHandlerOld`) and the newer Fragment-ref handler (`InnerScrollHandlerNew`), preserving the `NoClientRects` / `InViewport` / `OutOfViewport` state machine — only the visible-region lower boundary moves.
## Impact on vinext
vinext implements App Router auto-scroll on navigation. If vinext's scroll handler uses `elementTop >= 0` as the "in viewport" test, it will incorrectly skip scrolling when the target sits under a sticky header defined via `scroll-padding-top`, matching the pre-fix Next.js bug. On sites with a sticky header (very common), sidebar/link navigations can land with the target hidden.
What to check/do:
1. **Use `scroll-padding-top` as the visible-region lower boundary** in vinext's navigation scroll logic, mirroring `elementTop >= scrollPaddingTop`.
2. **Resolve both `px` and `%`** — computed values are usually `px`, but percentages must be resolved against the viewport height. Guard against non-finite/negative values (fall back to `0`).
3. **Resolve lazily and cache.** Only read `getComputedStyle().scrollPaddingTop` after the candidate produces client rects; skip it for empty Fragments and hash-only navigations. Cache the value and reuse it for the post-`scrollTop = 0` re-check to avoid extra reflows.
4. **Apply to whichever scroll handler(s) vinext has.** If vinext has both a legacy element path and a Fragment-based path, both need the boundary change.
5. **Port the regression tests.** Next.js added cases under `test/e2e/app-dir/router-autoscroll/` asserting that navigation scrolls when the destination is hidden inside the padding boundary, but preserves the current scroll position when the destination is genuinely visible below it.
### Notes
- Independent of Cache Components / Partial Prefetching; this is core App Router navigation scroll behavior.
- The change intentionally does not add work to scroll events or every render — keep the lazy/cached pattern to preserve dev/prod perf parity.
## Related
- (none — no existing scroll-behavior tracking issue)
Contributor guide
Research direction
Start by locating vinext's App Router navigation scroll handler and identify any legacy and Fragment-based paths that check whether a target is in view. Compare the behavior with packages/next/src/client/components/layout-router.tsx, then run or port the regression cases under test/e2e/app-dir/router-autoscroll/. Done means targets within root scroll-padding-top scroll into view while genuinely visible targets retain their position.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- next.js, typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100