Skip to content

Commit be7a578

Browse files
fix: scroll to top if no scroll restoration entry exists (#4786)
1 parent abad0a1 commit be7a578

File tree

5 files changed

+92
-5
lines changed

5 files changed

+92
-5
lines changed

e2e/react-router/basic-scroll-restoration/src/main.tsx

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,10 +202,65 @@ function ByElementComponent() {
202202
)
203203
}
204204

205+
const fooRoute = createRoute({
206+
getParentRoute: () => rootRoute,
207+
path: '/foo',
208+
loader: () => new Promise((r) => setTimeout(r, 500)),
209+
component: FooComponent,
210+
})
211+
212+
function FooComponent() {
213+
return (
214+
<div data-testid="foo-route-component" className="p-2">
215+
<h3 id="greeting">Hello from Foo!</h3>
216+
<Link to="/bar" data-testid="go-to-bar-link">
217+
Go to Bar
218+
</Link>
219+
<div className="space-y-2">
220+
{Array.from({ length: 50 }).map((_, i) => (
221+
<div
222+
key={i}
223+
className="h-[100px] p-2 rounded-lg bg-gray-200 dark:bg-gray-800 border"
224+
>
225+
Foo Item {i + 1}
226+
</div>
227+
))}
228+
</div>
229+
</div>
230+
)
231+
}
232+
233+
const barRoute = createRoute({
234+
getParentRoute: () => rootRoute,
235+
path: '/bar',
236+
loader: () => new Promise((r) => setTimeout(r, 500)),
237+
component: BarComponent,
238+
})
239+
240+
function BarComponent() {
241+
return (
242+
<div data-testid="bar-route-component" className="p-2">
243+
<h3 id="greeting">Hello from Bar!</h3>
244+
<div className="space-y-2">
245+
{Array.from({ length: 50 }).map((_, i) => (
246+
<div
247+
key={i}
248+
className="h-[100px] p-2 rounded-lg bg-gray-200 dark:bg-gray-800 border"
249+
>
250+
Bar Item {i + 1}
251+
</div>
252+
))}
253+
</div>
254+
</div>
255+
)
256+
}
257+
205258
const routeTree = rootRoute.addChildren([
206259
indexRoute,
207260
aboutRoute,
208261
byElementRoute,
262+
fooRoute,
263+
barRoute,
209264
])
210265

211266
const router = createRouter({

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ test('restore scroll positions by page, home pages top message should not displa
2121
expect(scrollPosition).toBe(targetScrollPosition)
2222

2323
await expect(page.locator('#top-message')).not.toBeInViewport()
24+
await page.waitForTimeout(1000)
2425

2526
// Step 3: Navigate to the about page
2627
await page.getByRole('link', { name: 'About', exact: true }).click()
@@ -80,3 +81,34 @@ test('restore scroll positions by element, first regular list item should not di
8081
// )
8182
// expect(restoredScrollPosition).toBe(targetScrollPosition)
8283
})
84+
85+
test('scroll to top when not scrolled, regression test for #4782', async ({
86+
page,
87+
}) => {
88+
await page.goto('/foo')
89+
90+
await expect(page.getByTestId('foo-route-component')).toBeVisible()
91+
92+
let scrollPosition = await page.evaluate(() => window.scrollY)
93+
expect(scrollPosition).toBe(0)
94+
95+
// do not scroll, just navigate to /bar
96+
await page.getByTestId('go-to-bar-link').click()
97+
await expect(page.getByTestId('bar-route-component')).toBeVisible()
98+
99+
const targetScrollPosition = 1000
100+
await page.evaluate(
101+
(scrollPos: number) => window.scrollTo(0, scrollPos),
102+
targetScrollPosition,
103+
)
104+
scrollPosition = await page.evaluate(() => window.scrollY)
105+
expect(scrollPosition).toBe(targetScrollPosition)
106+
107+
// navigate back to /foo
108+
await page.goBack()
109+
await expect(page.getByTestId('foo-route-component')).toBeVisible()
110+
111+
// check if scroll position is restored to 0 since we did not scroll on /foo
112+
const restoredScrollPosition = await page.evaluate(() => window.scrollY)
113+
expect(restoredScrollPosition).toBe(0)
114+
})

e2e/react-start/scroll-restoration/public/script.js

Lines changed: 0 additions & 2 deletions
This file was deleted.

e2e/react-start/scroll-restoration/public/script2.js

Lines changed: 0 additions & 2 deletions
This file was deleted.

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,11 @@ export function restoreScroll(
128128
;(() => {
129129
// If we have a cached entry for this location state,
130130
// we always need to prefer that over the hash scroll.
131-
if (shouldScrollRestoration && elementEntries) {
131+
if (
132+
shouldScrollRestoration &&
133+
elementEntries &&
134+
Object.keys(elementEntries).length > 0
135+
) {
132136
for (const elementSelector in elementEntries) {
133137
const entry = elementEntries[elementSelector]!
134138
if (elementSelector === 'window') {

0 commit comments

Comments
 (0)