Skip to content

Commit

Permalink
fix: app router hash scrolling should respect scroll-padding (#51268)
Browse files Browse the repository at this point in the history
When navigating to a route with a hash parameter, the layout router
jumps to the element by scrolling to the node's `offsetTop` value.
However, this will ignore `scroll-padding`, which deviates from browser
behavior.

It looks like this isn't an issue in the pages router which currently
makes use of
[`scrollIntoView`](https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/router.ts#L2262).

Closes NEXT-1171
Fixes #49612

---------
  • Loading branch information
ztanner committed Jun 14, 2023
1 parent 4ef3982 commit 3427d32
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 2 deletions.
5 changes: 3 additions & 2 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,10 @@ class InnerScrollAndFocusHandler extends React.Component<ScrollAndFocusHandlerPr

handleSmoothScroll(
() => {
// In case of hash scroll we need to scroll to the top of the element
// In case of hash scroll, we only need to scroll the element into view
if (hashFragment) {
window.scrollTo(0, (domNode as HTMLElement).offsetTop)
;(domNode as HTMLElement).scrollIntoView()

return
}
// Store the current viewport height because reading `clientHeight` causes a reflow,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
* {
margin: 0;
padding: 0;
box-sizing: border-box;
font-size: 14px;
line-height: 1;
scroll-padding-top: 20px;
}
41 changes: 41 additions & 0 deletions test/e2e/app-dir/navigation/app/hash-with-scroll-offset/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import Link from 'next/link'
import './global.css'

export default function HashPage() {
// Create list of 5000 items that all have unique id
const items = Array.from({ length: 5000 }, (_, i) => ({ id: i }))

return (
<div style={{ fontFamily: 'sans-serif', fontSize: '16px' }}>
<p>Hash Page</p>
<Link href="/hash#hash-6" id="link-to-6">
To 6
</Link>
<Link href="/hash#hash-50" id="link-to-50">
To 50
</Link>
<Link href="/hash#hash-160" id="link-to-160">
To 160
</Link>
<Link href="/hash#hash-300" id="link-to-300">
To 300
</Link>
<Link href="#hash-500" id="link-to-500">
To 500 (hash only)
</Link>
<Link href="/hash#top" id="link-to-top">
To Top
</Link>
<Link href="/hash#non-existent" id="link-to-non-existent">
To non-existent
</Link>
<div>
{items.map((item) => (
<div key={item.id}>
<div id={`hash-${item.id}`}>{item.id}</div>
</div>
))}
</div>
</div>
)
}
31 changes: 31 additions & 0 deletions test/e2e/app-dir/navigation/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,37 @@ createNextDescribe(
})
})

describe('hash-with-scroll-offset', () => {
it('should scroll to the specified hash', async () => {
const browser = await next.browser('/hash-with-scroll-offset')

const checkLink = async (
val: number | string,
expectedScroll: number
) => {
await browser.elementByCss(`#link-to-${val.toString()}`).click()
await check(
async () => {
const val = await browser.eval('window.pageYOffset')
return val.toString()
},
expectedScroll.toString(),
true,
// Try maximum of 15 seconds
15
)
}

await checkLink(6, 94)
await checkLink(50, 710)
await checkLink(160, 2250)
await checkLink(300, 4210)
await checkLink(500, 7010) // this one is hash only (`href="#hash-500"`)
await checkLink('top', 0)
await checkLink('non-existent', 0)
})
})

describe('hash-link-back-to-same-page', () => {
it('should scroll to the specified hash', async () => {
const browser = await next.browser('/hash-link-back-to-same-page')
Expand Down

0 comments on commit 3427d32

Please sign in to comment.