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)

Open
#2,810 0 comments 0 reactions 0 assignees View on GitHub
nextjs-tracking
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.