Skip to content

Commit d820542

Browse files
authored
fix(21606): consider scroll option when using shallow routing (#24888)
## Bug - [x] Related issues linked using `fixes #number` - [ ] Integration tests added fixes [#21606](#21606) ### Description When using shallow routing and wanting to scroll to top by setting the `scroll` option to `true` it didn't work. This PR fixes this issue.
1 parent 5072518 commit d820542

File tree

4 files changed

+29
-14
lines changed

4 files changed

+29
-14
lines changed

packages/next/client/link.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ function linkClicked(
9494
e.preventDefault()
9595

9696
// avoid scroll for urls with anchor refs
97-
if (scroll == null) {
98-
scroll = as.indexOf('#') < 0
97+
if (scroll == null && as.indexOf('#') >= 0) {
98+
scroll = false
9999
}
100100

101101
// replace state instead of push if prop is present

packages/next/next-server/lib/router/router.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -816,11 +816,6 @@ export default class Router implements BaseRouter {
816816
this.isReady = true
817817
}
818818

819-
// Default to scroll reset behavior unless explicitly specified to be
820-
// `false`! This makes the behavior between using `Router#push` and a
821-
// `<Link />` consistent.
822-
options.scroll = !!(options.scroll ?? true)
823-
824819
let localeChange = options.locale !== this.locale
825820

826821
if (process.env.__NEXT_I18N_SUPPORT) {
@@ -1165,9 +1160,6 @@ export default class Router implements BaseRouter {
11651160
!(routeInfo.Component as any).getInitialProps
11661161
}
11671162

1168-
// shallow routing is only allowed for same page URL changes.
1169-
const isValidShallowRoute = options.shallow && this.route === route
1170-
11711163
if (
11721164
(options as any)._h &&
11731165
pathname === '/_error' &&
@@ -1179,14 +1171,18 @@ export default class Router implements BaseRouter {
11791171
props.pageProps.statusCode = 500
11801172
}
11811173

1174+
// shallow routing is only allowed for same page URL changes.
1175+
const isValidShallowRoute = options.shallow && this.route === route
1176+
1177+
const shouldScroll = options.scroll ?? !isValidShallowRoute
1178+
const resetScroll = shouldScroll ? { x: 0, y: 0 } : null
11821179
await this.set(
11831180
route,
11841181
pathname!,
11851182
query,
11861183
cleanedAs,
11871184
routeInfo,
1188-
forcedScroll ||
1189-
(isValidShallowRoute || !options.scroll ? null : { x: 0, y: 0 })
1185+
forcedScroll ?? resetScroll
11901186
).catch((e) => {
11911187
if (e.cancelled) error = error || e
11921188
else throw e

test/integration/client-navigation/pages/nav/shallow-routing.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ export default withRouter(
2222
return router.query.counter ? parseInt(router.query.counter) : 0
2323
}
2424

25-
increase() {
25+
increase(scroll) {
2626
const counter = this.getCurrentCounter()
2727
const href = `/nav/shallow-routing?counter=${counter + 1}`
28-
Router.push(href, href, { shallow: true })
28+
Router.push(href, href, { shallow: true, scroll })
2929
}
3030

3131
increaseNonShallow() {
@@ -56,6 +56,9 @@ export default withRouter(
5656
<button id="increase" onClick={() => this.increase()}>
5757
Increase
5858
</button>
59+
<button id="increaseWithScroll" onClick={() => this.increase(true)}>
60+
Increase with scroll
61+
</button>
5962
<button id="increase2" onClick={() => this.increaseNonShallow()}>
6063
Increase Non-Shallow
6164
</button>

test/integration/client-navigation/test/index.test.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -881,6 +881,22 @@ describe('Client Navigation', () => {
881881
})
882882
})
883883

884+
it('should scroll to top when the scroll option is set to true', async () => {
885+
const browser = await webdriver(context.appPort, '/nav/shallow-routing')
886+
await browser.eval(() =>
887+
document.querySelector('#increaseWithScroll').scrollIntoView()
888+
)
889+
const scrollPosition = await browser.eval('window.pageYOffset')
890+
891+
expect(scrollPosition).toBeGreaterThan(3000)
892+
893+
await browser.elementByCss('#increaseWithScroll').click()
894+
await check(async () => {
895+
const newScrollPosition = await browser.eval('window.pageYOffset')
896+
return newScrollPosition === 0 ? 'success' : 'fail'
897+
}, 'success')
898+
})
899+
884900
describe('with URL objects', () => {
885901
it('should work with <Link/>', async () => {
886902
const browser = await webdriver(context.appPort, '/nav')

0 commit comments

Comments
 (0)