Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] revert #2819 and add code comment #2883

Merged
merged 4 commits into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lucky-phones-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[fix] revert #2819 and add code comment
76 changes: 41 additions & 35 deletions packages/kit/src/runtime/client/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,44 +275,50 @@ export class Renderer {
this._init(navigation_result);
}

const { hash, scroll, keepfocus } = opts || {};
// opts must be passed if we're navigating...
if (opts) {
const { hash, scroll, keepfocus } = opts;

if (!keepfocus) {
getSelection()?.removeAllRanges();
document.body.focus();
}
if (!keepfocus) {
getSelection()?.removeAllRanges();
document.body.focus();
}

const old_page_y_offset = Math.round(pageYOffset);
const old_max_page_y_offset = document.documentElement.scrollHeight - innerHeight;

await 0;

const new_page_y_offset = Math.round(pageYOffset);
const new_max_page_y_offset = document.documentElement.scrollHeight - innerHeight;

// After `await 0`, the `onMount()` function in the component executed.
// Check if no scrolling happened on mount.
const no_scroll_happened =
// In most cases, we can compare whether `pageYOffset` changed between navigation
new_page_y_offset === Math.min(old_page_y_offset, new_max_page_y_offset) ||
// But if the page is scrolled to/near the bottom, the browser would also scroll
// to/near the bottom of the new page on navigation. Since we can't detect when this
// behaviour happens, we naively compare by the y offset from the bottom of the page.
old_max_page_y_offset - old_page_y_offset === new_max_page_y_offset - new_page_y_offset;

// If there was no scrolling, we run on our custom scroll handling
if (no_scroll_happened) {
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);
const old_page_y_offset = Math.round(pageYOffset);
const old_max_page_y_offset = document.documentElement.scrollHeight - innerHeight;

await 0;

const new_page_y_offset = Math.round(pageYOffset);
const new_max_page_y_offset = document.documentElement.scrollHeight - innerHeight;

// After `await 0`, the `onMount()` function in the component executed.
// Check if no scrolling happened on mount.
const no_scroll_happened =
// In most cases, we can compare whether `pageYOffset` changed between navigation
new_page_y_offset === Math.min(old_page_y_offset, new_max_page_y_offset) ||
// But if the page is scrolled to/near the bottom, the browser would also scroll
// to/near the bottom of the new page on navigation. Since we can't detect when this
// behaviour happens, we naively compare by the y offset from the bottom of the page.
old_max_page_y_offset - old_page_y_offset === new_max_page_y_offset - new_page_y_offset;

// If there was no scrolling, we run on our custom scroll handling
if (no_scroll_happened) {
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);
}
}
} else {
// ...they will not be supplied if we're simply invalidating
await 0;
}

this.loading.promise = null;
Expand Down