[fix] reset scroll when navigated from scrolled page (#2735)

.changeset/wet-papayas-live.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[fix] reset scroll when navigated from scrolled page

packages/kit/src/runtime/client/renderer.js

@@ -275,28 +275,34 @@ export class Renderer {
 			this._init(navigation_result);
 		}
 
-		if (!opts?.keepfocus) {
-			document.body.focus();
-		}
+		if (!opts) {
+			await 0;
+		} else {
+			const { hash, scroll, keepfocus } = opts;
 
-		await 0;
-
-		// After `await 0`, the onMount() function in the component executed.
-		// If there was no scrolling happening (checked via pageYOffset),
-		// continue on our custom scroll handling
-		if (pageYOffset === 0 && opts) {
-			const { hash, scroll } = opts;
-
-			const deep_linked = hash && document.getElementById(hash.slice(1));
-			if (scroll) {
-				scrollTo(scroll.x, scroll.y);
-			} else if (deep_linked) {
-				// Here we use `scrollIntoView` on the element instead of `scrollTo`
-				// because it natively supports the `scroll-margin` and `scroll-behavior`
-				// CSS properties.
-				deep_linked.scrollIntoView();
-			} else {
-				scrollTo(0, 0);
+			if (!keepfocus) {
+				document.body.focus();
+			}
+
+			const oldPageYOffset = pageYOffset;
+			await 0;
+			const maxPageYOffset = document.body.scrollHeight - innerHeight;
+
+			// After `await 0`, the `onMount()` function in the component executed.
+			// If there was no scrolling happening (checked via `pageYOffset`),
+			// continue on our custom scroll handling
+			if (pageYOffset === Math.min(oldPageYOffset, maxPageYOffset)) {
+				const deep_linked = hash && document.getElementById(hash.slice(1));
+				if (scroll) {
+					scrollTo(scroll.x, scroll.y);
+				} else if (deep_linked) {
+					// Here we use `scrollIntoView` on the element instead of `scrollTo`
+					// because it natively supports the `scroll-margin` and `scroll-behavior`
+					// CSS properties.
+					deep_linked.scrollIntoView();
+				} else {
+					scrollTo(0, 0);
+				}
 			}
 		}
 

packages/kit/test/apps/basics/src/routes/anchor-with-manual-scroll/_tests.js

@@ -9,22 +9,20 @@ export default function (test) {
 	test(
 		'url-supplied anchor is ignored with onMount() scrolling on direct page load',
 		'/anchor-with-manual-scroll/anchor#go-to-element',
-		async ({ page, js }) => {
+		async ({ is_intersecting_viewport, js }) => {
 			if (js) {
-				const p = await page.$('#abcde');
-				assert.ok(p && (await p.isVisible()));
+				assert.ok(is_intersecting_viewport('#abcde'));
 			}
 		}
 	);
 
 	test(
 		'url-supplied anchor is ignored with onMount() scrolling on navigation to page',
 		'/anchor-with-manual-scroll',
-		async ({ page, clicknav, js }) => {
+		async ({ clicknav, is_intersecting_viewport, js }) => {
 			await clicknav('[href="/anchor-with-manual-scroll/anchor#go-to-element"]');
 			if (js) {
-				const p = await page.$('#abcde');
-				assert.ok(p && (await p.isVisible()));
+				assert.ok(is_intersecting_viewport('#abcde'));
 			}
 		}
 	);

packages/kit/test/apps/basics/src/routes/anchor/_tests.js

@@ -9,22 +9,31 @@ export default function (test) {
 	test(
 		'url-supplied anchor works on direct page load',
 		'/anchor/anchor#go-to-element',
-		async ({ page, js }) => {
+		async ({ is_intersecting_viewport, js }) => {
 			if (js) {
-				const p = await page.$('#go-to-element');
-				assert.ok(p && (await p.isVisible()));
+				assert.ok(is_intersecting_viewport('#go-to-element'));
 			}
 		}
 	);
 
 	test(
 		'url-supplied anchor works on navigation to page',
 		'/anchor',
-		async ({ page, clicknav, js }) => {
-			await clicknav('[href="/anchor/anchor#go-to-element"]');
+		async ({ clicknav, is_intersecting_viewport, js }) => {
+			await clicknav('#first-anchor');
 			if (js) {
-				const p = await page.$('#go-to-element');
-				assert.ok(p && (await p.isVisible()));
+				assert.ok(is_intersecting_viewport('#go-to-element'));
+			}
+		}
+	);
+
+	test(
+		'url-supplied anchor works when navigated from scrolled page',
+		'/anchor',
+		async ({ clicknav, is_intersecting_viewport, js }) => {
+			await clicknav('#second-anchor');
+			if (js) {
+				assert.ok(is_intersecting_viewport('#go-to-element'));
 			}
 		}
 	);

packages/kit/test/apps/basics/src/routes/anchor/index.svelte

@@ -1,5 +1,7 @@
 <h1>Welcome to a test project</h1>
-<a href="/anchor/anchor#go-to-element">Anchor demo</a>
+<a id="first-anchor" href="/anchor/anchor#go-to-element">Anchor demo</a>
+<div>Spacing</div>
+<a id="second-anchor" href="/anchor/anchor#go-to-element">Anchor demo below</a>
 
 <style>
 	:global(body) {
@@ -12,4 +14,9 @@
 		display: block;
 		margin: 20px;
 	}
+
+	div {
+		background-color: hotpink;
+		height: 180vh;
+	}
 </style>

packages/kit/test/apps/basics/src/routes/use-action/_tests.js

@@ -9,10 +9,9 @@ export default function (test) {
 	test(
 		'app-supplied scroll and focus work on direct page load',
 		'/use-action/focus-and-scroll',
-		async ({ page, js }) => {
+		async ({ page, is_intersecting_viewport, js }) => {
 			if (js) {
-				const input = await page.$('#input');
-				assert.ok(input && (await input.isVisible()));
+				assert.ok(await is_intersecting_viewport('#input'));
 				assert.ok(await page.$eval('#input', (el) => el === document.activeElement));
 			}
 		}
@@ -21,11 +20,10 @@ export default function (test) {
 	test(
 		'app-supplied scroll and focus work on navigation to page',
 		'/use-action',
-		async ({ page, clicknav, js }) => {
+		async ({ page, clicknav, is_intersecting_viewport, js }) => {
 			await clicknav('[href="/use-action/focus-and-scroll"]');
 			if (js) {
-				const input = await page.$('#input');
-				assert.ok(input && (await input.isVisible()));
+				assert.ok(await is_intersecting_viewport('#input'));
 				assert.ok(await page.$eval('#input', (el) => el === document.activeElement));
 			}
 		}

packages/kit/test/test.js

@@ -181,6 +181,10 @@ function duplicate(test_fn, config, is_build) {
 					page: context.pages.nojs,
 					clicknav: (selector) => context.pages.nojs.click(selector),
 					back: () => context.pages.nojs.goBack().then(() => void 0),
+					is_intersecting_viewport: async () => {
+						console.warn('is_intersecting_viewport is not supported in nojs mode');
+						return false;
+					},
 					// @ts-expect-error
 					response,
 					js: false
@@ -254,6 +258,21 @@ function duplicate(test_fn, config, is_build) {
 							context.pages.js.evaluate(() => window.navigated)
 						]);
 					},
+					// Reference from Puppeteer: https://github.com/puppeteer/puppeteer/blob/943477cc1eb4b129870142873b3554737d5ef252/experimental/puppeteer-firefox/lib/JSHandle.js#L190-L204
+					is_intersecting_viewport: async (selector) => {
+						return await context.pages.js.$eval(selector, async (element) => {
+							const visibleRatio = await new Promise((resolve) => {
+								const observer = new IntersectionObserver((entries) => {
+									resolve(entries[0].intersectionRatio);
+									observer.disconnect();
+								});
+								observer.observe(element);
+								// Firefox doesn't call IntersectionObserver callback unless there are rafs
+								requestAnimationFrame(() => {});
+							});
+							return visibleRatio > 0;
+						});
+					},
 					js: true,
 					// @ts-expect-error
 					response

packages/kit/test/types.d.ts

@@ -17,6 +17,10 @@ export interface TestContext {
 	response: PlaywrightResponse;
 	clicknav(selector: string): Promise<void>;
 	back(): Promise<void>;
+	/**
+	 * Only supported in js mode
+	 */
+	is_intersecting_viewport(selector: string): Promise<boolean>;
 	fetch(url: RequestInfo, opts?: RequestInit): Promise<NodeFetchResponse>;
 	capture_requests(fn: () => Promise<void>): Promise<string[]>;
 	errors(): string;