Skip to content

Commit

Permalink
fix: allow smooth scrolling if only hash changes (pages & app) (#52915)
Browse files Browse the repository at this point in the history
We were preventing smooth scrolling to avoid jarring UX when `scroll-behavior: smooth` was set and the user navigates to another route ([PR](#40642) and [related comment](#51721 (comment))). 

This updates both pages & app router to restore smooth scroll functionality if the only the route hash changes.

Fixes #51721
Closes NEXT-1406
  • Loading branch information
ztanner committed Jul 20, 2023
1 parent 4994a42 commit 8664ffe
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 23 deletions.
5 changes: 3 additions & 2 deletions packages/next/src/client/components/layout-router.tsx
Expand Up @@ -158,7 +158,7 @@ interface ScrollAndFocusHandlerProps {
segmentPath: FlightSegmentPath
}
class InnerScrollAndFocusHandler extends React.Component<ScrollAndFocusHandlerProps> {
handlePotentialScroll = () => {
handlePotentialScroll = (isUpdate?: boolean) => {
// Handle scroll and focus, it's only applied once in the first useEffect that triggers that changed.
const { focusAndScrollRef, segmentPath } = this.props

Expand Down Expand Up @@ -247,6 +247,7 @@ class InnerScrollAndFocusHandler extends React.Component<ScrollAndFocusHandlerPr
{
// We will force layout by querying domNode position
dontForceLayout: true,
onlyHashChange: !!isUpdate,
}
)

Expand All @@ -262,7 +263,7 @@ class InnerScrollAndFocusHandler extends React.Component<ScrollAndFocusHandlerPr
componentDidUpdate() {
// Because this property is overwritten in handlePotentialScroll it's fine to always run it when true as it'll be set to false for subsequent renders.
if (this.props.focusAndScrollRef.apply) {
this.handlePotentialScroll()
this.handlePotentialScroll(true)
}
}

Expand Down
48 changes: 28 additions & 20 deletions packages/next/src/shared/lib/router/router.ts
Expand Up @@ -2247,27 +2247,35 @@ export default class Router implements BaseRouter {

scrollToHash(as: string): void {
const [, hash = ''] = as.split('#')
// Scroll to top if the hash is just `#` with no value or `#top`
// To mirror browsers
if (hash === '' || hash === 'top') {
handleSmoothScroll(() => window.scrollTo(0, 0))
return
}

// Decode hash to make non-latin anchor works.
const rawHash = decodeURIComponent(hash)
// First we check if the element by id is found
const idEl = document.getElementById(rawHash)
if (idEl) {
handleSmoothScroll(() => idEl.scrollIntoView())
return
}
// If there's no element with the id, we check the `name` property
// To mirror browsers
const nameEl = document.getElementsByName(rawHash)[0]
if (nameEl) {
handleSmoothScroll(() => nameEl.scrollIntoView())
}
handleSmoothScroll(
() => {
// Scroll to top if the hash is just `#` with no value or `#top`
// To mirror browsers
if (hash === '' || hash === 'top') {
window.scrollTo(0, 0)
return
}

// Decode hash to make non-latin anchor works.
const rawHash = decodeURIComponent(hash)
// First we check if the element by id is found
const idEl = document.getElementById(rawHash)
if (idEl) {
idEl.scrollIntoView()
return
}
// If there's no element with the id, we check the `name` property
// To mirror browsers
const nameEl = document.getElementsByName(rawHash)[0]
if (nameEl) {
nameEl.scrollIntoView()
}
},
{
onlyHashChange: this.onlyAHashChange(as),
}
)
}

urlIsNew(asPath: string): boolean {
Expand Down
Expand Up @@ -4,8 +4,14 @@
*/
export function handleSmoothScroll(
fn: () => void,
options: { dontForceLayout?: boolean } = {}
options: { dontForceLayout?: boolean; onlyHashChange?: boolean } = {}
) {
// if only the hash is changed, we don't need to disable smooth scrolling
// we only care to prevent smooth scrolling when navigating to a new page to avoid jarring UX
if (options.onlyHashChange) {
fn()
return
}
const htmlElement = document.documentElement
const existing = htmlElement.style.scrollBehavior
htmlElement.style.scrollBehavior = 'auto'
Expand Down

0 comments on commit 8664ffe

Please sign in to comment.