Skip to content

Commit ec80698

Browse files
fix: scroll to top with hash history (#4795)
1 parent 3c6633a commit ec80698

File tree

4 files changed

+63
-23
lines changed

4 files changed

+63
-23
lines changed

e2e/react-router/scroll-restoration-sandbox-vite/tests/app.spec.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,30 @@ test('Smoke - Renders home', async ({ page }) => {
99
).toBeVisible()
1010
})
1111

12-
// Test for scroll related stuff
13-
;[
12+
const pages = [
1413
linkOptions({ to: '/normal-page' }),
1514
linkOptions({ to: '/lazy-page' }),
1615
linkOptions({ to: '/virtual-page' }),
1716
linkOptions({ to: '/lazy-with-loader-page' }),
1817
linkOptions({ to: '/page-with-search', search: { where: 'footer' } }),
19-
].forEach((options) => {
18+
] as const
19+
20+
pages.forEach((options, index) => {
21+
const from = index === 0 ? pages[1].to : pages[0].to
22+
test(`On navigate from ${from} to ${options.to} (from the footer), scroll should be at top`, async ({
23+
page,
24+
}) => {
25+
await page.goto(toRuntimePath(from))
26+
const link = page.getByRole('link', { name: `Foot-${options.to}` })
27+
await link.scrollIntoViewIfNeeded()
28+
await page.waitForTimeout(500)
29+
await link.click()
30+
await expect(page.getByTestId('at-the-top')).toBeInViewport()
31+
})
32+
})
33+
34+
// Test for scroll related stuff
35+
pages.forEach((options) => {
2036
test(`On navigate to ${options.to} (from the header), scroll should be at top`, async ({
2137
page,
2238
}) => {

packages/react-router/src/scroll-restoration.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,23 @@ export function ScrollRestoration() {
1414
const resolvedKey =
1515
userKey !== defaultGetScrollRestorationKey(router.latestLocation)
1616
? userKey
17-
: null
17+
: undefined
1818

1919
if (!router.isScrollRestoring || !router.isServer) {
2020
return null
2121
}
2222

23+
const restoreScrollOptions: Parameters<typeof restoreScroll>[0] = {
24+
storageKey,
25+
shouldScrollRestoration: true,
26+
}
27+
if (resolvedKey) {
28+
restoreScrollOptions.key = resolvedKey
29+
}
30+
2331
return (
2432
<ScriptOnce
25-
children={`(${restoreScroll.toString()})(${JSON.stringify(storageKey)},${JSON.stringify(resolvedKey)}, undefined, true)`}
33+
children={`(${restoreScroll.toString()})(${JSON.stringify(restoreScrollOptions)})`}
2634
/>
2735
)
2836
}

packages/router-core/src/scroll-restoration.ts

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { functionalUpdate } from './utils'
22
import type { AnyRouter } from './router'
33
import type { ParsedLocation } from './location'
44
import type { NonNullableUpdater } from './utils'
5+
import type { HistoryLocation } from '@tanstack/history'
56

67
export type ScrollRestorationEntry = { scrollX: number; scrollY: number }
78

@@ -100,15 +101,21 @@ let ignoreScroll = false
100101
// unless they are passed in as arguments. Why? Because we need to be able to
101102
// toString() it into a script tag to execute as early as possible in the browser
102103
// during SSR. Additionally, we also call it from within the router lifecycle
103-
export function restoreScroll(
104-
storageKey: string,
105-
key: string | undefined,
106-
behavior: ScrollToOptions['behavior'] | undefined,
107-
shouldScrollRestoration: boolean | undefined,
108-
scrollToTopSelectors:
109-
| Array<string | (() => Element | null | undefined)>
110-
| undefined,
111-
) {
104+
export function restoreScroll({
105+
storageKey,
106+
key,
107+
behavior,
108+
shouldScrollRestoration,
109+
scrollToTopSelectors,
110+
location,
111+
}: {
112+
storageKey: string
113+
key?: string
114+
behavior?: ScrollToOptions['behavior']
115+
shouldScrollRestoration?: boolean
116+
scrollToTopSelectors?: Array<string | (() => Element | null | undefined)>
117+
location?: HistoryLocation
118+
}) {
112119
let byKey: ScrollRestorationByKey
113120

114121
try {
@@ -157,7 +164,7 @@ export function restoreScroll(
157164
// Which means we've never seen this location before,
158165
// we need to check if there is a hash in the URL.
159166
// If there is, we need to scroll it's ID into view.
160-
const hash = window.location.hash.split('#')[1]
167+
const hash = (location ?? window.location).hash.split('#')[1]
161168

162169
if (hash) {
163170
const hashScrollIntoViewOptions =
@@ -325,13 +332,14 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) {
325332
return
326333
}
327334

328-
restoreScroll(
335+
restoreScroll({
329336
storageKey,
330-
cacheKey,
331-
router.options.scrollRestorationBehavior || undefined,
332-
router.isScrollRestoring || undefined,
333-
router.options.scrollToTopSelectors || undefined,
334-
)
337+
key: cacheKey,
338+
behavior: router.options.scrollRestorationBehavior,
339+
shouldScrollRestoration: router.isScrollRestoring,
340+
scrollToTopSelectors: router.options.scrollToTopSelectors,
341+
location: router.history.location,
342+
})
335343

336344
if (router.isScrollRestoring) {
337345
// Mark the location as having been seen

packages/solid-router/src/scroll-restoration.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,23 @@ export function ScrollRestoration() {
1414
const resolvedKey =
1515
userKey !== defaultGetScrollRestorationKey(router.latestLocation)
1616
? userKey
17-
: null
17+
: undefined
1818

1919
if (!router.isScrollRestoring || !router.isServer) {
2020
return null
2121
}
2222

23+
const restoreScrollOptions: Parameters<typeof restoreScroll>[0] = {
24+
storageKey,
25+
shouldScrollRestoration: true,
26+
}
27+
if (resolvedKey) {
28+
restoreScrollOptions.key = resolvedKey
29+
}
30+
2331
return (
2432
<ScriptOnce
25-
children={`(${restoreScroll.toString()})(${JSON.stringify(storageKey)},${JSON.stringify(resolvedKey)}, undefined, true)`}
33+
children={`(${restoreScroll.toString()})(${JSON.stringify(restoreScrollOptions)})`}
2634
/>
2735
)
2836
}

0 commit comments

Comments
 (0)