From 34f18ae83ca130e02bcf504401a0b103dc0612a0 Mon Sep 17 00:00:00 2001
From: Bjorn Lu <34116392+bluwy@users.noreply.github.com>
Date: Fri, 5 Nov 2021 12:16:24 +0800
Subject: [PATCH] [fix] reset scroll when navigated from scrolled page (#2735)
---
.changeset/wet-papayas-live.md | 5 ++
packages/kit/src/runtime/client/renderer.js | 48 +++++++++++--------
.../anchor-with-manual-scroll/_tests.js | 10 ++--
.../apps/basics/src/routes/anchor/_tests.js | 23 ++++++---
.../basics/src/routes/anchor/index.svelte | 9 +++-
.../basics/src/routes/use-action/_tests.js | 10 ++--
packages/kit/test/test.js | 19 ++++++++
packages/kit/test/types.d.ts | 4 ++
8 files changed, 87 insertions(+), 41 deletions(-)
create mode 100644 .changeset/wet-papayas-live.md
diff --git a/.changeset/wet-papayas-live.md b/.changeset/wet-papayas-live.md
new file mode 100644
index 000000000000..cf93f3b4448c
--- /dev/null
+++ b/.changeset/wet-papayas-live.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[fix] reset scroll when navigated from scrolled page
diff --git a/packages/kit/src/runtime/client/renderer.js b/packages/kit/src/runtime/client/renderer.js
index b0990138ecf8..b0d8838b8115 100644
--- a/packages/kit/src/runtime/client/renderer.js
+++ b/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);
+ }
}
}
diff --git a/packages/kit/test/apps/basics/src/routes/anchor-with-manual-scroll/_tests.js b/packages/kit/test/apps/basics/src/routes/anchor-with-manual-scroll/_tests.js
index ebc787a52896..fd10490913da 100644
--- a/packages/kit/test/apps/basics/src/routes/anchor-with-manual-scroll/_tests.js
+++ b/packages/kit/test/apps/basics/src/routes/anchor-with-manual-scroll/_tests.js
@@ -9,10 +9,9 @@ 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'));
}
}
);
@@ -20,11 +19,10 @@ export default function (test) {
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'));
}
}
);
diff --git a/packages/kit/test/apps/basics/src/routes/anchor/_tests.js b/packages/kit/test/apps/basics/src/routes/anchor/_tests.js
index 660bf1457a14..3ddc93ea97f1 100644
--- a/packages/kit/test/apps/basics/src/routes/anchor/_tests.js
+++ b/packages/kit/test/apps/basics/src/routes/anchor/_tests.js
@@ -9,10 +9,9 @@ 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'));
}
}
);
@@ -20,11 +19,21 @@ export default function (test) {
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'));
}
}
);
diff --git a/packages/kit/test/apps/basics/src/routes/anchor/index.svelte b/packages/kit/test/apps/basics/src/routes/anchor/index.svelte
index b89b04ad2874..960cf6547774 100644
--- a/packages/kit/test/apps/basics/src/routes/anchor/index.svelte
+++ b/packages/kit/test/apps/basics/src/routes/anchor/index.svelte
@@ -1,5 +1,7 @@
Welcome to a test project
-Anchor demo
+Anchor demo
+Spacing
+Anchor demo below
diff --git a/packages/kit/test/apps/basics/src/routes/use-action/_tests.js b/packages/kit/test/apps/basics/src/routes/use-action/_tests.js
index 007ba55e55ef..520b6ecdb276 100644
--- a/packages/kit/test/apps/basics/src/routes/use-action/_tests.js
+++ b/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));
}
}
diff --git a/packages/kit/test/test.js b/packages/kit/test/test.js
index 1c89179c412b..06da8854aa01 100644
--- a/packages/kit/test/test.js
+++ b/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
diff --git a/packages/kit/test/types.d.ts b/packages/kit/test/types.d.ts
index f8c6c9fec627..c81d97ded2c5 100644
--- a/packages/kit/test/types.d.ts
+++ b/packages/kit/test/types.d.ts
@@ -17,6 +17,10 @@ export interface TestContext {
response: PlaywrightResponse;
clicknav(selector: string): Promise;
back(): Promise;
+ /**
+ * Only supported in js mode
+ */
+ is_intersecting_viewport(selector: string): Promise;
fetch(url: RequestInfo, opts?: RequestInit): Promise;
capture_requests(fn: () => Promise): Promise;
errors(): string;